From da531a0c07e53764fe7fedbd5b99009f9c16d6f5 Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Fri, 17 Apr 2026 04:04:19 -0700 Subject: [PATCH 1/4] Update DESIGN to match current workspace - Correct crate count (9 -> 10) and add sshenc-tpm-bridge to the crate table. - Expand the sshenc-cli subcommand list (config, openssh, uninstall, identity, default, ssh, completions) to match main.rs. - Replace the stale "socat + npiperelay" WSL bridge description with the current JSON-RPC bridge (sshenc-tpm-bridge) on both the platform backends and platform support tables. - Document the Windows named pipe DACL and the Unix socket/directory permissions. - Document that the Linux software fallback is wrapped by an OS keyring key via enclaveapp-keyring, not raw P-256 on disk. - Add sshenc-tpm-bridge to the binaries list. --- DESIGN.md | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 45c4757..7360ce8 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -8,18 +8,19 @@ Private key material never leaves the hardware security module. ## Architecture -Rust workspace with 9 crates under `crates/`: +Rust workspace with 10 crates under `crates/`: | Crate | Purpose | |---|---| -| `sshenc-core` | Domain models, SSH public key encoding, fingerprints, config | +| `sshenc-core` | Domain models, SSH public key encoding, fingerprints, config, trusted binary discovery, transactional backup/rollback | | `sshenc-se` | Hardware key backend via `KeyBackend` trait (macOS, Windows, Linux) | | `sshenc-agent-proto` | SSH agent protocol: message parsing, DER-to-SSH signature conversion | | `sshenc-agent` | Async SSH agent daemon (tokio), Unix socket / named pipe server | -| `sshenc-cli` | Main CLI (`sshenc`): keygen, list, inspect, delete, export-pub, agent, install | +| `sshenc-cli` | Main CLI (`sshenc`): keygen, list, inspect, delete, export-pub, agent, config, openssh, install, uninstall, identity, default, ssh, completions | | `sshenc-keygen-cli` | Standalone `sshenc-keygen` binary | | `sshenc-gitenc` | `gitenc` binary for git+SSH with per-key identity selection and commit signing | | `sshenc-pkcs11` | PKCS#11 provider (cdylib) for agent auto-start via SSH's PKCS11Provider | +| `sshenc-tpm-bridge` | Windows-side JSON-RPC bridge that serves TPM operations to the WSL agent | | `sshenc-test-support` | Mock key backend for testing without hardware | ### Platform Backends @@ -33,8 +34,8 @@ the `signing` feature of each platform crate: | macOS | `enclaveapp-apple` | Secure Enclave (CryptoKit) | | Windows | `enclaveapp-windows` | TPM 2.0 (CNG) | | Linux | `enclaveapp-linux-tpm` | TPM 2.0 (tss-esapi) | -| Linux (no TPM) | `enclaveapp-software` | Software P-256 (fallback) | -| WSL | `enclaveapp-wsl` + bridge | Windows TPM via socat/npiperelay | +| Linux (no TPM) | `enclaveapp-software` + `enclaveapp-keyring` | Software P-256 wrapped by an OS keyring key | +| WSL | `enclaveapp-wsl` + `sshenc-tpm-bridge` | Windows TPM via JSON-RPC over stdio | The `KeyBackend` trait in `sshenc-se` wraps libenclaveapp's `EnclaveSigner` into sshenc's domain model. Platform selection happens at runtime based on @@ -62,8 +63,22 @@ implements the SSH agent protocol for identity enumeration and signing. The PKCS#11 provider acts as a lightweight launcher -- if SSH loads it and the agent isn't running, it starts the agent automatically. +On Windows the named pipe is created with an explicit DACL +(`D:P(A;;GA;;;OW)(A;;GA;;;SY)`) via +`ConvertStringSecurityDescriptorToSecurityDescriptorW` so that only the pipe +owner and SYSTEM can open it. On Unix the socket directory is restricted to +`0700` and the socket to `0600`. + `sshenc install` configures `~/.ssh/config` with `IdentityAgent` and -`PKCS11Provider` directives. +`PKCS11Provider` directives. `sshenc uninstall` removes them. + +### WSL Bridge + +On WSL, the agent cannot talk to the Windows TPM directly. Instead, the Linux +side spawns `sshenc-tpm-bridge.exe` (installed on the Windows host) and +communicates with it over stdin/stdout using the JSON-RPC protocol defined in +`enclaveapp-tpm-bridge`. The bridge binary is discovered via a fixed allowlist +of installation directories (no `$PATH` lookup). ### Git Integration @@ -89,9 +104,9 @@ See [THREAT_MODEL.md](THREAT_MODEL.md) for detailed analysis. | macOS (Apple Silicon / T2) | Full support | CryptoKit Secure Enclave | | Windows (native) | Full support | TPM 2.0 via CNG, named pipe agent | | Windows (Git Bash) | Full support | GIT_SSH_COMMAND bypass for MINGW SSH | -| WSL | Full support | socat + npiperelay bridge to Windows agent | +| WSL | Full support | JSON-RPC bridge (`sshenc-tpm-bridge.exe`) to Windows TPM | | Linux (with TPM) | Full support | TPM 2.0 via tss-esapi | -| Linux (no TPM) | Software fallback | P-256 keys on disk, one-time warning | +| Linux (no TPM) | Software fallback | P-256 keys on disk, wrapped by OS keyring key via `enclaveapp-keyring` | ## Binaries @@ -100,3 +115,4 @@ See [THREAT_MODEL.md](THREAT_MODEL.md) for detailed analysis. 3. `sshenc-agent` -- ssh-agent-compatible daemon 4. `gitenc` -- git wrapper with per-key identity and commit signing 5. `libsshenc_pkcs11.dylib` -- PKCS#11 provider (agent auto-start) +6. `sshenc-tpm-bridge` -- Windows-side JSON-RPC bridge invoked by the WSL agent From a04b3ec4000c7b8518d9c1da984ec5e891dfbc81 Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Fri, 17 Apr 2026 09:54:05 -0700 Subject: [PATCH 2/4] agent: O_NOFOLLOW + create_new on the daemonize ready file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signal_ready now opens the ready file with OpenOptions::new().write(true).create_new(true).custom_flags(libc::O_NOFOLLOW).mode(0o600) on Unix. If the path already exists the open fails with EEXIST; if it's a symlink the open fails with ELOOP. Either way the write never lands on an attacker-chosen target, and the error surfaces to the parent process via the daemonize handshake instead of being silently followed. mode(0o600) at open time is load-bearing on umask-permissive systems; the explicit set_permissions(0o600) after write is belt-and-suspenders. The parent (daemonize in main.rs) is responsible for removing the ready-file path before and after spawning the child, so signal_ready always starts from a clean slate and can safely demand create_new semantics without a separate unlink. New test signal_ready_refuses_preplanted_symlink pre-plants a symlink at the ready path and asserts that: 1. signal_ready returns an error (ELOOP / EEXIST / File exists), 2. the symlink's target file is NOT created — so the attack is blocked rather than silently followed. --- crates/sshenc-agent/src/server.rs | 73 ++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/crates/sshenc-agent/src/server.rs b/crates/sshenc-agent/src/server.rs index 21e1dbe..f9c6d22 100644 --- a/crates/sshenc-agent/src/server.rs +++ b/crates/sshenc-agent/src/server.rs @@ -383,12 +383,44 @@ fn signal_ready(path: Option<&Path>) -> Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - std::fs::write(path, b"ready\n")?; + // Use create-new + O_NOFOLLOW so a pre-planted symlink at `path` + // (shared `/tmp`, multi-user system, etc.) cannot redirect the + // "ready" write to an attacker-chosen target. If the path already + // exists the open fails with EEXIST; if it's a symlink the open + // fails with ELOOP. Either way we surface the error rather than + // silently following the link. See sshenc THREAT_MODEL.md + // "Ready-File Symlink in $TMPDIR". + // + // The parent process (`daemonize` in `main.rs`) is responsible + // for unlinking the ready-file path before and after spawning + // the agent, so we always start from a clean slate here and can + // safely demand create_new semantics. #[cfg(unix)] { - use std::os::unix::fs::PermissionsExt; + use std::io::Write; + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .custom_flags(libc::O_NOFOLLOW) + .mode(0o600) + .open(path)?; + file.write_all(b"ready\n")?; + file.sync_all()?; + // `mode(0o600)` at open time is load-bearing on umask-0000 systems; + // the explicit set_permissions is belt-and-suspenders for the + // umask-permissive case. std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; } + #[cfg(not(unix))] + { + use std::io::Write; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(path)?; + file.write_all(b"ready\n")?; + } Ok(()) } @@ -1385,6 +1417,43 @@ mod tests { std::fs::remove_file(&path).unwrap(); } + #[cfg(unix)] + #[test] + fn signal_ready_refuses_preplanted_symlink() { + use std::os::unix::fs::symlink; + + let path = signal_ready_test_path("symlink-refused"); + let decoy = signal_ready_test_path("symlink-refused-target"); + let _unused = std::fs::remove_file(&path); + let _unused = std::fs::remove_file(&decoy); + + // Pre-plant a symlink at `path` pointing at a decoy file an + // attacker wants us to write to. The decoy need not exist — + // O_NOFOLLOW refuses the open before we ever dereference. + symlink(&decoy, &path).unwrap(); + + let err = signal_ready(Some(&path)).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("Too many levels of symbolic links") + || msg.contains("symbolic link") + || msg.contains("ELOOP") + || msg.contains("File exists") + || msg.contains("EEXIST") + || msg.to_lowercase().contains("symlink"), + "unexpected error message for symlink refusal: {msg}" + ); + + // The decoy file must NOT have been created. + assert!( + !decoy.exists(), + "signal_ready followed the symlink — decoy was written" + ); + + let _unused = std::fs::remove_file(&path); + let _unused = std::fs::remove_file(&decoy); + } + #[test] fn signal_ready_creates_parent_dirs() { let base = signal_ready_test_path("nested-parent"); From fe7b46ee853796de1c09d166960578512963c536 Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Fri, 17 Apr 2026 09:54:13 -0700 Subject: [PATCH 3/4] workspace: repoint libenclaveapp path deps to ../libenclaveapp/crates/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates on libenclaveapp's own crates/ as the single source of truth. The former ../crates/ location (top-level /enclaveapps/crates/) is gone — it was a divergent copy that periodically drifted from libenclaveapp/crates/. sshenc builds against the canonical tree now. --- Cargo.lock | 7 +++++++ Cargo.toml | 18 +++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 98270b4..3334f61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -428,12 +428,15 @@ dependencies = [ name = "enclaveapp-apple" version = "0.1.0" dependencies = [ + "aes-gcm", "base64 0.22.1", "dirs", "enclaveapp-core", "libc", + "rand 0.9.4", "serde", "serde_json", + "tracing", ] [[package]] @@ -462,9 +465,11 @@ dependencies = [ "libc", "serde", "serde_json", + "sha2", "thiserror", "toml 0.8.23", "tracing", + "windows", ] [[package]] @@ -481,6 +486,7 @@ dependencies = [ "serde", "serde_json", "sha2", + "zeroize", ] [[package]] @@ -1368,6 +1374,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "windows", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index eaac4ab..0c36668 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,15 +29,15 @@ sshenc-agent = { path = "crates/sshenc-agent" } sshenc-test-support = { path = "crates/sshenc-test-support" } # libenclaveapp (shared hardware-backed key management) -enclaveapp-core = { path = "../crates/enclaveapp-core" } -enclaveapp-apple = { path = "../crates/enclaveapp-apple", features = ["signing"] } -enclaveapp-windows = { path = "../crates/enclaveapp-windows", features = ["signing"] } -enclaveapp-keyring = { path = "../crates/enclaveapp-keyring", features = ["signing"] } -enclaveapp-linux-tpm = { path = "../crates/enclaveapp-linux-tpm", features = ["signing"] } -enclaveapp-wsl = { path = "../crates/enclaveapp-wsl" } -enclaveapp-app-storage = { path = "../crates/enclaveapp-app-storage" } -enclaveapp-build-support = { path = "../crates/enclaveapp-build-support" } -enclaveapp-tpm-bridge = { path = "../crates/enclaveapp-tpm-bridge" } +enclaveapp-core = { path = "../libenclaveapp/crates/enclaveapp-core" } +enclaveapp-apple = { path = "../libenclaveapp/crates/enclaveapp-apple", features = ["signing"] } +enclaveapp-windows = { path = "../libenclaveapp/crates/enclaveapp-windows", features = ["signing"] } +enclaveapp-keyring = { path = "../libenclaveapp/crates/enclaveapp-keyring", features = ["signing"] } +enclaveapp-linux-tpm = { path = "../libenclaveapp/crates/enclaveapp-linux-tpm", features = ["signing"] } +enclaveapp-wsl = { path = "../libenclaveapp/crates/enclaveapp-wsl" } +enclaveapp-app-storage = { path = "../libenclaveapp/crates/enclaveapp-app-storage" } +enclaveapp-build-support = { path = "../libenclaveapp/crates/enclaveapp-build-support" } +enclaveapp-tpm-bridge = { path = "../libenclaveapp/crates/enclaveapp-tpm-bridge" } # Serialization serde = { version = "1", features = ["derive"] } From e41f83810c80fb364de676880f7e660e124a059c Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Fri, 17 Apr 2026 09:54:26 -0700 Subject: [PATCH 4/4] docs: refresh THREAT_MODEL for ready-file, named-pipe, meta HMAC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 'Ready-File Symlink in $TMPDIR' rewritten around the new O_NOFOLLOW + create_new hardening; residual risk reduced from 'theoretically exploitable' to 'none for the symlink-redirect vector.' - 'Windows Named-Pipe Hijack' notes the explicit DACL now granting full control only to creator-owner + SYSTEM. - 'Metadata File Tamper (.meta)' notes the HMAC sidecar now written by the Linux keyring backend via enclaveapp-keyring::meta_hmac_key — .meta tamper without keyring access is now caught with a hard meta_hmac_verify error. --- THREAT_MODEL.md | 58 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 4bc2c42..fb306a9 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -242,12 +242,15 @@ descriptor) can accept SSH clients' signing requests. before starting `sshenc-agent`, so clients connect to `sshenc`. - `sshenc-agent` uses `ServerOptions::first_pipe_instance(true)` and refuses to attach to an existing pipe. +- The named pipe is created with an explicit DACL + (`ConvertStringSecurityDescriptorToSecurityDescriptorW`) that grants + full control only to the creator-owner (the current user) and + `SYSTEM`, cutting off `Administrators` and `Everyone` who would + otherwise have default read/write access (`sshenc-agent/src/server.rs` + `SecurityDescriptor`). - The CLI surfaces an actionable error when the pipe is in use. **Residual risk**: -- The named pipe is currently created with the default security - descriptor. A best-practice hardening is an explicit DACL that grants - only the owner user and `SYSTEM`; that work is not yet in place. - An attacker with admin rights can always create the pipe first; admin rights on Windows already implies full control over the TPM. @@ -263,20 +266,31 @@ fields. the hardware's enforcement — Touch ID / Windows Hello still fires on sign regardless of what the metadata file claims. - Metadata files are written 0600 via `atomic_write`. +- On the Linux keyring / software backend, `.meta` now has an HMAC + sidecar `