Merge pull request #27 from fastrevmd-lab/security-remediation

security: remediate RNC-SEC-001..006 + CI hardening

Author: fastrevmd-lab

.cargo/audit.toml

@@ -0,0 +1,28 @@
+# cargo-audit configuration.
+#
+# This file lists RustSec advisories that are tracked and explicitly
+# risk-accepted (or pending upstream fixes). Each ignore entry must
+# include a rationale and a review date so it does not become invisible.
+
+[advisories]
+ignore = [
+    # RUSTSEC-2023-0071 — Marvin Attack timing side-channel in `rsa`.
+    #
+    # Reachability: `rsa 0.10.0-rc.16` is pulled in through
+    #   russh -> internal-russh-forked-ssh-key -> rsa.
+    # The dependency exists unconditionally inside the forked ssh-key
+    # crate; russh has no opt-out feature for it. Upstream `cargo audit`
+    # reports "No fixed upgrade is available!"
+    #
+    # Exposure: only matters when an RSA SSH key is used. The library
+    # default and CLI templates do not generate or use RSA keys, and
+    # rustnetconf supports ed25519/ECDSA paths through SSH agent and
+    # key_file auth.
+    #
+    # Mitigation: README/security documentation recommends Ed25519 or
+    # ECDSA SSH keys until a fixed upstream release lands.
+    #
+    # Review date: 2026-08-01 (re-check russh + ssh-key for a fixed rsa
+    # release; remove this ignore once upstream resolves it).
+    "RUSTSEC-2023-0071",
+]

.github/workflows/ci.yml

@@ -0,0 +1,88 @@
+name: CI
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+env:
+  CARGO_TERM_COLOR: always
+  RUSTFLAGS: -D warnings
+
+jobs:
+  test:
+    name: Test (workspace, all features)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y cmake
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Cache cargo registry and target
+        uses: Swatinem/rust-cache@v2
+
+      - name: Build
+        run: cargo build --workspace --all-features --verbose
+
+      - name: Test
+        run: cargo test --workspace --all-features --verbose
+        env:
+          SKIP_INTEGRATION: "1"
+
+  clippy:
+    name: Clippy (workspace, all features)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y cmake
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy
+
+      - name: Cache cargo registry and target
+        uses: Swatinem/rust-cache@v2
+
+      - name: Clippy
+        run: cargo clippy --workspace --all-targets --all-features -- -D warnings
+
+  fmt:
+    name: Rustfmt
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          components: rustfmt
+
+      - name: Check formatting
+        run: cargo fmt --all -- --check
+
+  audit:
+    name: cargo audit
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Install cargo-audit
+        run: cargo install --locked cargo-audit
+
+      - name: Run cargo audit
+        run: cargo audit

Cargo.lock

@@ -98,6 +98,12 @@ key_file = "~/.ssh/id_ed25519"
 # vendor auto-detected from device hello
 ```
 
+**Secrets:** `inventory.toml` may contain plaintext passwords. Prefer
+`key_file` or SSH-agent auth where possible. If you must use inline
+passwords, protect the file with `chmod 600 inventory.toml` and add it
+to `.gitignore`. Passwords are stored in zeroizing memory and redacted
+from `Debug` output, but the on-disk file itself is plaintext.
+
 ## Library — Quick Start
 
 ```toml
@@ -391,8 +397,25 @@ match result {
 - **Mock transport tests** — session state machine, CommitUnknown detection, lock recovery
 - **Integration tests** — 32 tests against a live Juniper vSRX including full edit-config round trips, vendor auto-detection, connection pooling, and concurrent sessions
 
+### Prerequisites
+
+The `rustnetconf-yang` subcrate builds `libyang2` from source via `yang2`'s `bundled` feature, which requires `cmake`. Install it before running workspace-wide tests or clippy:
+
 ```bash
-cargo test --workspace                    # Run all tests
+# Debian/Ubuntu
+sudo apt-get install cmake
+
+# macOS
+brew install cmake
+
+# Fedora/RHEL
+sudo dnf install cmake
+```
+
+The core `rustnetconf` and `rustnetconf-cli` crates do not require `cmake`; `cargo test -p rustnetconf` works without it.
+
+```bash
+cargo test --workspace                    # Run all tests (requires cmake)
 cargo test --test integration_vsrx        # Run vSRX integration tests only
 SKIP_INTEGRATION=1 cargo test             # Skip tests requiring a device
 ```
@@ -408,7 +431,7 @@ SKIP_INTEGRATION=1 cargo test             # Skip tests requiring a device
 ### Security Features
 
 - **Credential zeroization** — Passwords and key passphrases use `Zeroizing<String>` (via the `zeroize` crate) and are securely erased from memory on drop.
-- **SSH host key verification** — `HostKeyVerification` must be set explicitly (no `Default` impl). Use `Fingerprint("SHA256:...")` to pin host keys in production. `AcceptAll` is available for lab use but emits a `tracing::warn!`.
+- **SSH host key verification** — `HostKeyVerification` must be set explicitly. The `ClientBuilder` default is `RejectAll` (fail closed): the SSH handshake fails until the caller pins a fingerprint via `Fingerprint("SHA256:...")` or explicitly opts in to `AcceptAll` for lab use (logs a `tracing::warn!`). `ProxyJump` hops parsed from `~/.ssh/config` likewise default to `RejectAll` and must be individually configured. In the CLI, set `host_key_fingerprint` per device in `inventory.toml`, or pass `--insecure-accept-host-key` for lab use only.
 - **Shell-escaped ProxyCommand** — `%h` and `%p` substitutions are shell-escaped to prevent command injection via malicious hostnames.
 - **XML fragment validation** — All user-provided RPC content is validated for well-formedness before insertion, preventing XML injection.
 - **XML attribute escaping** — All message-id values are escaped to prevent XML attribute injection.
@@ -419,10 +442,21 @@ SKIP_INTEGRATION=1 cargo test             # Skip tests requiring a device
 - **Typed error hierarchy** — Structured error types (`ChannelClosed`, `SessionExpired`, `MessageIdMismatch`) enable precise error handling without string matching.
 - **No unsafe code** — The entire codebase uses safe Rust.
 
+### Known advisories
+
+- **RUSTSEC-2023-0071** (Marvin Attack, `rsa` crate timing side-channel)
+  is present in the dependency graph via
+  `russh → internal-russh-forked-ssh-key → rsa 0.10.0-rc.16`. No fixed
+  upstream release is available yet. The advisory is risk-accepted with
+  rationale in `.cargo/audit.toml` and CI re-checks on every run. It only
+  matters when an **RSA** SSH key is used for authentication — Ed25519
+  and ECDSA paths are unaffected. Use the mitigation in the next section.
+
 ### Security Best Practices
 
-- Use Ed25519 SSH keys (not RSA) for device authentication
-- Set `host_key_verification(HostKeyVerification::Fingerprint(...))` in production — `HostKeyVerification` has no default, so you must choose explicitly
+- Use Ed25519 SSH keys (not RSA) for device authentication (also mitigates
+  RUSTSEC-2023-0071 above)
+- Set `host_key_verification(HostKeyVerification::Fingerprint(...))` in production — the default is `RejectAll` (fail closed), so the connection will refuse to complete until you choose a policy. For the CLI, set `host_key_fingerprint = "SHA256:..."` per device in `inventory.toml`.
 - Set `.rpc_timeout(Duration::from_secs(30))` to prevent hanging on unresponsive devices
 - Prefer SSH agent auth over inline passwords
 - Store credentials in inventory.toml with restricted file permissions (`chmod 600`)

README.md

@@ -84,7 +84,9 @@ async fn main() {
     if let Some(caps) = client.capabilities() {
         eprintln!(
             "Connected (session-id: {})",
-            caps.session_id().map(|id| id.to_string()).unwrap_or_else(|| "unknown".into()),
+            caps.session_id()
+                .map(|id| id.to_string())
+                .unwrap_or_else(|| "unknown".into()),
         );
     }
 

examples/edit_config.rs

@@ -75,7 +75,9 @@ async fn main() {
     if let Some(caps) = client.capabilities() {
         eprintln!(
             "Connected (session-id: {}, capabilities: {})",
-            caps.session_id().map(|id| id.to_string()).unwrap_or_else(|| "unknown".into()),
+            caps.session_id()
+                .map(|id| id.to_string())
+                .unwrap_or_else(|| "unknown".into()),
             caps.all_uris().len()
         );
     }

examples/get_config.rs

@@ -61,7 +61,10 @@ async fn main() {
         process::exit(1);
     }
 
-    eprintln!("Fetching config from {} devices concurrently...", hosts.len());
+    eprintln!(
+        "Fetching config from {} devices concurrently...",
+        hosts.len()
+    );
     let start = Instant::now();
 
     // Spawn concurrent tasks for each device
@@ -90,7 +93,11 @@ async fn main() {
             let result = match client.get_config(Datastore::Running).await {
                 Ok(config) => {
                     let elapsed = device_start.elapsed();
-                    Ok(format!("{} bytes in {:.1}s", config.len(), elapsed.as_secs_f64()))
+                    Ok(format!(
+                        "{} bytes in {:.1}s",
+                        config.len(),
+                        elapsed.as_secs_f64()
+                    ))
                 }
                 Err(e) => Err(format!("get-config failed: {e}")),
             };

examples/multi_device.rs

@@ -22,3 +22,7 @@ serde_json = "1"
 quick-xml = "0.37"
 tokio = { version = "1", features = ["full"] }
 dialoguer = "0.11"
+zeroize = "1"
+
+[dev-dependencies]
+tempfile = "3"