From 1d88b337acf60fc70843b497ddf8a6d5018681df Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:17:48 -0700 Subject: [PATCH 01/11] feat(guard): aarch64 Linux support + RESOLVE_NO_XDEV opt-in Adds a hand-written aarch64 syscall shim alongside the existing x86_64 one (uses x8 for syscall number, x0..x3 for args, svc #0 to trap). Removes the compile_error! that previously blocked all non-x86_64 builds. riscv64 still produces a compile error; that's a follow-up. Eliminates the raw fstat() helper by reading attestation fields (device/inode/nlink) through File::metadata(). std's stat wrapper already handles arch-specific struct stat layouts portably, so this side-steps the x86_64-vs-aarch64 layout divergence and removes ~50 lines of inline-asm fstat code we no longer need. Adds OpenOptions::no_xdev() exposing RESOLVE_NO_XDEV for callers that need mount-point containment (defends against bind-mount escapes). Off by default to preserve the existing directory-tree-containment semantics; opt-in matches the no_symlinks pattern. --- src/guard/fd_jail.rs | 118 +++++++++++++++---------------------------- src/openat2.rs | 51 +++++++++++++++---- tests/guard.rs | 26 ++++++++++ 3 files changed, 107 insertions(+), 88 deletions(-) diff --git a/src/guard/fd_jail.rs b/src/guard/fd_jail.rs index 519b2f7..f3d5e50 100644 --- a/src/guard/fd_jail.rs +++ b/src/guard/fd_jail.rs @@ -226,16 +226,16 @@ fn encode_path_field(buf: &mut Vec, path: &Path) { /// /// Mirrors the relevant subset of [`std::fs::OpenOptions`]. /// -/// # Mount-point containment note +/// # Mount-point containment /// -/// `RESOLVE_NO_XDEV` (block cross-device traversal) is intentionally **not** -/// exposed. The spec's containment model is *directory-tree* containment, not -/// *filesystem-volume* containment. Bind mounts and overlayfs layers that are -/// mounted inside the jail root are accessible by design — they are part of the -/// logical directory tree as seen by the kernel. If your security policy -/// requires strict filesystem-level containment (e.g., no bind-mount escapes -/// in a container runtime), add `RESOLVE_NO_XDEV` to the `resolve` field of -/// the `OpenHow` struct used internally, or open a feature request. +/// By default the kernel allows traversal across mount points inside the jail +/// root (bind mounts, overlayfs layers, etc. that have been mounted into the +/// jail directory tree are reachable). If your threat model requires strict +/// filesystem-volume containment — e.g., to defend against a privileged process +/// bind-mounting external content into the jail — opt in via +/// [`no_xdev`](Self::no_xdev). On Linux this maps to `RESOLVE_NO_XDEV`; on the +/// macOS/BSD fallback it is a no-op (the fallback has no equivalent +/// kernel-enforced flag). #[derive(Debug, Clone, Default)] pub struct OpenOptions { pub(crate) read: bool, @@ -245,6 +245,7 @@ pub struct OpenOptions { pub(crate) create: bool, pub(crate) create_new: bool, pub(crate) no_symlinks: bool, + pub(crate) no_xdev: bool, } impl OpenOptions { @@ -285,6 +286,20 @@ impl OpenOptions { self.no_symlinks = v; self } + + /// Block traversal across mount points (Linux: `RESOLVE_NO_XDEV`). + /// + /// When enabled, `openat2` returns `EXDEV` (mapped to [`JailError::Escape`]) + /// if any path component crosses a mount point. Use this to defend against + /// bind-mount escapes when an attacker may have mounted external content + /// inside the jail directory. + /// + /// On the macOS/BSD fallback this is a no-op — the fallback has no + /// equivalent flag and cannot enforce mount-point containment. + pub fn no_xdev(mut self, v: bool) -> Self { + self.no_xdev = v; + self + } } // ── Linux implementation ────────────────────────────────────────────────────── @@ -294,9 +309,10 @@ mod linux_impl { use super::*; use crate::openat2::{ openat2, Errno, OpenHow, O_APPEND, O_CLOEXEC, O_CREAT, O_EXCL, O_RDONLY, O_TRUNC, O_WRONLY, - RESOLVE_BENEATH, RESOLVE_NO_MAGICLINKS, RESOLVE_NO_SYMLINKS, + RESOLVE_BENEATH, RESOLVE_NO_MAGICLINKS, RESOLVE_NO_SYMLINKS, RESOLVE_NO_XDEV, }; use std::ffi::CString; + use std::os::unix::fs::MetadataExt; use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd}; /// Implementation of [`FdJail::open`] on Linux using `openat2`. @@ -344,6 +360,9 @@ mod linux_impl { if opts.no_symlinks { resolve |= RESOLVE_NO_SYMLINKS; } + if opts.no_xdev { + resolve |= RESOLVE_NO_XDEV; + } let how = OpenHow { flags, @@ -354,17 +373,19 @@ mod linux_impl { let owned_fd = openat2(dirfd.as_raw_fd(), &cpath, &how) .map_err(|e| map_errno_to_jail_error(e, rel_path))?; - // fstat the opened fd for attestation - let file_stat = fstat(owned_fd.as_raw_fd()).map_err(JailError::Io)?; + // Read attestation fields via File::metadata — uses std's portable stat + // wrapper and avoids arch-specific struct stat layouts. The fd ownership + // moves into File so it closes when the JailFile drops. let file: File = unsafe { File::from_raw_fd(owned_fd.into_raw_fd()) }; + let meta = file.metadata().map_err(JailError::Io)?; let attestation = Attestation { jail_root: jail_root.to_path_buf(), opened_path: rel_path.to_path_buf(), root_inode, - file_inode: file_stat.ino, - device: file_stat.dev, - nlink: file_stat.nlink, + file_inode: meta.ino(), + device: meta.dev(), + nlink: meta.nlink(), toctou_safe: true, opened_at: SystemTime::now(), signature: None, @@ -382,63 +403,6 @@ mod linux_impl { _ => JailError::Io(e.into()), } } - - // ── stat(2) without libc ─────────────────────────────────────────────────── - - pub(crate) struct StatResult { - pub dev: u64, - pub ino: u64, - pub nlink: u64, - } - - /// `fstat(2)` via raw syscall — avoids libc. - pub(crate) fn fstat(fd: i32) -> std::io::Result { - // stat64 layout (x86-64 / aarch64) - #[repr(C)] - struct Stat64 { - st_dev: u64, - st_ino: u64, - st_nlink: u64, - st_mode: u32, - st_uid: u32, - st_gid: u32, - _pad0: u32, - st_rdev: u64, - st_size: i64, - st_blksize: i64, - st_blocks: i64, - st_atime: i64, - st_atime_ns: i64, - st_mtime: i64, - st_mtime_ns: i64, - st_ctime: i64, - st_ctime_ns: i64, - _unused: [i64; 3], - } - - let mut stat = std::mem::MaybeUninit::::zeroed(); - let ret: i64; - unsafe { - std::arch::asm!( - "syscall", - inlateout("rax") 5i64 /* SYS_fstat */ => ret, - in("rdi") fd, - in("rsi") stat.as_mut_ptr(), - out("rcx") _, - out("r11") _, - options(nostack), - ); - } - if ret < 0 { - return Err(std::io::Error::from_raw_os_error(-ret as i32)); - } - let s = unsafe { stat.assume_init() }; - Ok(StatResult { - dev: s.st_dev, - ino: s.st_ino, - nlink: s.st_nlink, - }) - } } // ── macOS / BSD fallback ────────────────────────────────────────────────────── @@ -557,8 +521,8 @@ impl FdJail { #[cfg(target_os = "linux")] { - use std::os::unix::fs::OpenOptionsExt; - use std::os::unix::io::{AsRawFd, FromRawFd}; + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + use std::os::unix::io::FromRawFd; // Check kernel version first for a friendly error message. if let Some(kv) = openat2_kernel_version() { @@ -582,9 +546,7 @@ impl FdJail { .open(&root) .map_err(JailError::Io)?; - // SAFETY: dir_file is open and valid; we immediately wrap it. - let raw_fd = dir_file.as_raw_fd(); - let stat = linux_impl::fstat(raw_fd).map_err(JailError::Io)?; + let root_inode = dir_file.metadata().map_err(JailError::Io)?.ino(); // Transfer ownership into OwnedFd (File will not close it). let dirfd = unsafe { std::os::unix::io::OwnedFd::from_raw_fd(std::os::unix::io::IntoRawFd::into_raw_fd( @@ -593,7 +555,7 @@ impl FdJail { }; return Ok(FdJail { root, - root_inode: stat.ino, + root_inode, dirfd, }); } diff --git a/src/openat2.rs b/src/openat2.rs index 0064f8c..471ad03 100644 --- a/src/openat2.rs +++ b/src/openat2.rs @@ -11,18 +11,18 @@ use std::sync::OnceLock; // ── Architecture guard ──────────────────────────────────────────────────────── -// The inline-asm syscall shim is currently implemented for x86_64 only. -// aarch64 and riscv64 use different register conventions (x8/x0-x5 and -// a7/a0-a5 respectively) and need their own asm blocks. Narrow the supported -// arch list to x86_64 until those are written, rather than silently producing -// broken binaries on aarch64 CI runners. -#[cfg(not(target_arch = "x86_64"))] +// The inline-asm syscall shim is implemented for x86_64 and aarch64. +// riscv64 uses a different register convention (a7/a0-a5) and is not yet +// supported. Reject other architectures with a compile error rather than +// silently producing broken binaries. +#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] compile_error!( - "path_jail guard: only x86_64 Linux is currently supported for the raw-asm syscall path. \ - aarch64/riscv64 support is planned. Track: https://github.com/tenuo-ai/path_jail/issues" + "path_jail guard: only x86_64 and aarch64 Linux are currently supported for the raw-asm syscall path. \ + riscv64 support is planned. Track: https://github.com/tenuo-ai/path_jail/issues" ); -// SYS_openat2 — syscall number on x86_64 Linux (added in 5.6) +// SYS_openat2 — syscall number on Linux (added in 5.6). Same value on x86_64 +// and aarch64 (the kernel keeps recent syscall numbers aligned across arches). const SYS_OPENAT2: i64 = 437; // ── open_how layout (linux/openat2.h) ──────────────────────────────────────── @@ -38,6 +38,7 @@ pub(crate) struct OpenHow { pub(crate) const RESOLVE_BENEATH: u64 = 0x08; pub(crate) const RESOLVE_NO_SYMLINKS: u64 = 0x04; pub(crate) const RESOLVE_NO_MAGICLINKS: u64 = 0x02; +pub(crate) const RESOLVE_NO_XDEV: u64 = 0x01; // O_* flags (x86_64 Linux) pub(crate) const O_RDONLY: u64 = 0; @@ -105,7 +106,7 @@ pub(crate) fn openat2(dirfd: RawFd, path: &CStr, how: &OpenHow) -> Result i64 { ret } +/// aarch64 Linux variant of [`syscall4`]. +/// +/// # Safety +/// +/// Same contract as the x86_64 variant — caller supplies a valid syscall +/// number and matching arguments. +#[cfg(target_arch = "aarch64")] +#[inline(always)] +unsafe fn syscall4(nr: i64, a0: i64, a1: i64, a2: i64, a3: i64) -> i64 { + let ret: i64; + // aarch64 Linux syscall ABI: + // nr → x8 + // a0 → x0 (inout so the return value lands back in x0) + // a1 → x1 + // a2 → x2 + // a3 → x3 + // The svc #0 instruction triggers the syscall; the kernel preserves + // all callee-saved registers, so no explicit clobbers are needed. + std::arch::asm!( + "svc #0", + in("x8") nr, + inout("x0") a0 => ret, + in("x1") a1, + in("x2") a2, + in("x3") a3, + options(nostack), + ); + ret +} + // ── Kernel version probe ─────────────────────────────────────────────────────── /// Parsed kernel version (major, minor, patch). diff --git a/tests/guard.rs b/tests/guard.rs index e7c8efd..ab52dcf 100644 --- a/tests/guard.rs +++ b/tests/guard.rs @@ -343,6 +343,32 @@ fn toctou_safe_reflects_platform() { ); } +// ── no_xdev option ─────────────────────────────────────────────────────────── + +#[test] +#[cfg(unix)] +fn no_xdev_option_compiles_and_is_chainable() { + // We can't reliably create a mount point inside a tempdir in CI without + // privileges, so we just exercise the builder. The real EXDEV behaviour + // is enforced by the kernel and verified by integration testing against + // a bind-mounted directory in production deployments. + let opts = OpenOptions::new().read(true).no_xdev(true); + let _ = opts; // chainable, type-checks +} + +#[test] +#[cfg(target_os = "linux")] +fn no_xdev_succeeds_when_no_mount_crossing() { + // Without a mount-point crossing, no_xdev must not produce a spurious EXDEV. + let dir = tempdir().unwrap(); + let file = dir.path().join("a.txt"); + std::fs::write(&file, b"x").unwrap(); + + let jail = FdJail::new(dir.path()).unwrap(); + jail.open("a.txt", OpenOptions::new().read(true).no_xdev(true)) + .expect("open with no_xdev should succeed when no mount is crossed"); +} + // ── no_symlinks option ──────────────────────────────────────────────────────── #[test] From 7d6241f20ac42c7e0f221f318639acd1b98f3758 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:21:03 -0700 Subject: [PATCH 02/11] feat(guard): pluggable attestation signing (Signer/Verifier traits) Implements spec AC6 ("Signed attestation verifies under configured key") without vendoring a crypto crate. Callers implement Signer / Verifier with their crypto backend of choice (ed25519-dalek, ring, HSM client, KMS, etc.) and path_jail stays zero-dependency. Public surface: - guard::Signer / guard::Verifier traits with associated Error types - guard::VerifyError enum (NotSigned vs Invalid(E)) - JailFile::sign_attestation(&S) -> Result - Attestation::verify(&V) -> Result<(), VerifyError> - Attestation::signing_bytes() now pub so external enforcement points can replay the canonical wire format without going through verify() Tests use a deterministic non-crypto stand-in (XOR rolling checksum) to exercise the full happy/sad paths: signed verifies, wrong key fails, unsigned returns NotSigned, tampering invalidates the signature. --- CHANGELOG.md | 13 +++- src/guard/fd_jail.rs | 63 ++++++++++++++-- src/guard/mod.rs | 2 + src/guard/signing.rs | 99 +++++++++++++++++++++++++ tests/guard.rs | 167 ++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 326 insertions(+), 18 deletions(-) create mode 100644 src/guard/signing.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c43ee84..db9d519 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,21 +14,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `FdJail::open()` / `FdJail::create()` perform a single TOCTOU-safe syscall on Linux - `FdJail::check()` validates a path without opening (logging/display only — must not be used as the basis for a subsequent open) - `Attestation` records `jail_root`, `opened_path`, `root_inode`, `file_inode`, `device`, `nlink`, `toctou_safe`, `opened_at` - - `Attestation::content_bytes()` for deterministic comparison; `signing_bytes()` for future Ed25519 signing - - `OpenOptions` with `read`/`write`/`append`/`truncate`/`create`/`create_new`/`no_symlinks` + - `Attestation::content_bytes()` for deterministic comparison; `Attestation::signing_bytes()` now public for external verifiers + - `OpenOptions` with `read`/`write`/`append`/`truncate`/`create`/`create_new`/`no_symlinks`/`no_xdev` - `JailFile::has_hard_links()` exposes hard-link policy; library does not enforce, caller decides - macOS/BSD fallback via `O_NOFOLLOW`; `Attestation::toctou_safe` is `false` on the fallback path +- **Pluggable attestation signing** (`guard::Signer`, `guard::Verifier`, `guard::VerifyError`) + - `JailFile::sign_attestation(&signer)` returns a signed `Attestation` + - `Attestation::verify(&verifier)` checks the signature on the enforcement side + - Zero vendored crypto — bring your own (`ed25519-dalek`, `ring`, HSM, KMS, etc.) +- **`OpenOptions::no_xdev`** — opt in to `RESOLVE_NO_XDEV` for mount-point containment (defends against bind-mount escapes) +- **aarch64 Linux support** for the `guard` feature (alongside x86_64); riscv64 is still gated by `compile_error!` - New error variants (guarded by `guard` feature): `Escape`, `SymlinkRejected`, `MagicLink`, `UnsupportedKernel`, `InvalidJailRoot` ### Changed - **Breaking**: MSRV bumped from 1.80 to 1.85 to accommodate transitive dev-dependencies that require Cargo edition 2024 +- Attestation fields are now read via `File::metadata()` instead of an inline-asm `fstat` syscall (portable across architectures, eliminates the arch-specific struct-stat layout problem) - Crate package now `exclude`s `docs/`, `.claude/`, `.github/`, `tests/` ### Notes - The `guard` feature uses only `std` and raw syscalls — zero new runtime dependencies -- `guard` is currently x86_64 Linux only for the raw-asm `openat2` path; aarch64/riscv64 support is planned +- `guard` supports x86_64 and aarch64 Linux for the raw-asm `openat2` path; riscv64 support is planned ## [0.3.0] - 2026-01-05 diff --git a/src/guard/fd_jail.rs b/src/guard/fd_jail.rs index f3d5e50..deeca18 100644 --- a/src/guard/fd_jail.rs +++ b/src/guard/fd_jail.rs @@ -73,6 +73,24 @@ impl JailFile { pub fn has_hard_links(&self) -> bool { self.attestation.nlink > 1 } + + /// Signs this file's attestation with the given signer and returns a + /// new [`Attestation`] with `signature` populated. + /// + /// The signature covers [`Attestation::signing_bytes`] — the canonical + /// fixed-layout encoding of every attestation field including `opened_at`. + /// + /// path_jail does not vendor a signing implementation; provide one by + /// implementing the [`Signer`](crate::guard::Signer) trait. See the + /// [`signing`](crate::guard) module docs for an `ed25519-dalek` example. + pub fn sign_attestation( + &self, + signer: &S, + ) -> Result { + let mut att = self.attestation.clone(); + att.signature = Some(signer.sign(&att.signing_bytes())?); + Ok(att) + } } impl std::fmt::Debug for JailFile { @@ -128,10 +146,15 @@ impl std::io::Seek for JailFile { /// /// # Signing /// -/// Attestations can be signed with an Ed25519 key by calling -/// `sign_attestation` (see future Ed25519 signing support). Unsigned attestations are valid for logging -/// and debugging but **MUST NOT** be accepted by the Tenuo enforcement point as -/// proof of guard execution. +/// Sign an attestation by calling [`JailFile::sign_attestation`] with any type +/// that implements the [`Signer`](crate::guard::Signer) trait. The library does +/// not vendor a crypto implementation — bring your own (`ed25519-dalek`, +/// `ring`, HSM, KMS, etc.). See the [`signing`](crate::guard) module for +/// examples. +/// +/// Verify a received attestation with [`Attestation::verify`]. Unsigned +/// attestations are valid for logging and debugging but **MUST NOT** be +/// accepted by the Tenuo enforcement point as proof of guard execution. /// /// # Determinism /// @@ -198,9 +221,14 @@ impl Attestation { /// Returns the full signing wire format (content bytes + `opened_at` nanos). /// - /// This is the byte slice that the Ed25519 signature covers. It is - /// intentionally not pub — callers will use `JailFile::sign_attestation` (future work). - #[allow(dead_code)] // Used by future Ed25519 signing integration + /// This is the exact byte slice that a [`Signer`](crate::guard::Signer) + /// produces a signature over, and that a + /// [`Verifier`](crate::guard::Verifier) must replay during verification. + /// Exposed publicly so enforcement points outside this crate can perform + /// their own verification without going through [`Attestation::verify`]. + /// + /// Format: [`content_bytes`](Self::content_bytes) followed by + /// `opened_at` (nanoseconds since UNIX_EPOCH) as `u64 LE`. pub fn signing_bytes(&self) -> Vec { let mut buf = self.content_bytes(); let nanos = self @@ -211,6 +239,27 @@ impl Attestation { buf.extend_from_slice(&nanos.to_le_bytes()); buf } + + /// Verifies this attestation's signature with the given verifier. + /// + /// Returns `Ok(())` only if a signature is present **and** the verifier + /// accepts it. Unsigned attestations return + /// [`VerifyError::NotSigned`](crate::guard::VerifyError::NotSigned); + /// signed-but-invalid attestations return + /// [`VerifyError::Invalid`](crate::guard::VerifyError::Invalid) wrapping + /// the verifier's own error. + /// + /// Enforcement points should call this **before** trusting any other + /// attestation field — see the type-level docs for the rationale. + pub fn verify( + &self, + verifier: &V, + ) -> Result<(), crate::guard::VerifyError> { + let sig = self.signature.ok_or(crate::guard::VerifyError::NotSigned)?; + verifier + .verify(&self.signing_bytes(), &sig) + .map_err(crate::guard::VerifyError::Invalid) + } } fn encode_path_field(buf: &mut Vec, path: &Path) { diff --git a/src/guard/mod.rs b/src/guard/mod.rs index d6b3840..434023e 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -5,5 +5,7 @@ //! See the [crate-level documentation](crate) for a quick-start example. mod fd_jail; +mod signing; pub use fd_jail::{Attestation, FdJail, JailFile, OpenOptions}; +pub use signing::{Signer, Verifier, VerifyError}; diff --git a/src/guard/signing.rs b/src/guard/signing.rs new file mode 100644 index 0000000..4831cb5 --- /dev/null +++ b/src/guard/signing.rs @@ -0,0 +1,99 @@ +//! Pluggable signing for [`Attestation`](super::Attestation) structs. +//! +//! path_jail does not vendor a crypto implementation — bring your own by +//! implementing [`Signer`] (and [`Verifier`] on the enforcement side). This +//! keeps the crate zero-dependency while letting callers wire up +//! `ed25519-dalek`, `ring`, an HSM client, AWS KMS, GCP KMS, etc. +//! +//! # Wire format +//! +//! The signature covers the bytes returned by +//! [`Attestation::signing_bytes`](super::Attestation::signing_bytes), which is +//! the canonical fixed-layout encoding of every attestation field including +//! `opened_at`. The format is deterministic and free of length-ambiguity: +//! see the doc on `signing_bytes` for the exact layout. +//! +//! # Example: ed25519-dalek +//! +//! ```ignore +//! use ed25519_dalek::{Signature, Signer as DalekSigner, SigningKey, Verifier as DalekVerifier, VerifyingKey}; +//! use path_jail::guard::{Signer, Verifier}; +//! +//! struct DalekS(SigningKey); +//! impl Signer for DalekS { +//! type Error = std::convert::Infallible; +//! fn sign(&self, msg: &[u8]) -> Result<[u8; 64], Self::Error> { +//! Ok(self.0.sign(msg).to_bytes()) +//! } +//! } +//! +//! struct DalekV(VerifyingKey); +//! impl Verifier for DalekV { +//! type Error = ed25519_dalek::SignatureError; +//! fn verify(&self, msg: &[u8], sig: &[u8; 64]) -> Result<(), Self::Error> { +//! self.0.verify(msg, &Signature::from_bytes(sig)) +//! } +//! } +//! ``` + +/// Produces a 64-byte signature over a byte slice. +/// +/// Implementations typically wrap a key handle (in-process key material, an +/// HSM session, a KMS client, etc.). Failures may come from the underlying +/// crypto provider (network errors talking to KMS, HSM unavailable, etc.). +pub trait Signer { + /// Signing-failure type — usually the underlying provider's error. + /// Use [`std::convert::Infallible`] for in-process signers that cannot fail. + type Error: std::error::Error + Send + Sync + 'static; + + /// Sign `msg` and return the 64-byte signature. + /// + /// The signature MUST cover all of `msg`. Truncating or pre-hashing + /// without authentication weakens the attestation chain. + fn sign(&self, msg: &[u8]) -> Result<[u8; 64], Self::Error>; +} + +/// Verifies a 64-byte signature over a byte slice. +/// +/// On the enforcement-point side. Implementations wrap a verifying key. +pub trait Verifier { + /// Verification-failure type. + type Error: std::error::Error + Send + Sync + 'static; + + /// Returns `Ok(())` if `signature` is valid over `msg` under the + /// implementation's verifying key; otherwise returns the provider error. + fn verify(&self, msg: &[u8], signature: &[u8; 64]) -> Result<(), Self::Error>; +} + +/// Error returned by [`Attestation::verify`](super::Attestation::verify). +/// +/// Distinguishes "attestation is unsigned" from "signature is invalid", so +/// enforcement points can choose to reject both or only the latter. +#[derive(Debug)] +pub enum VerifyError { + /// The attestation has no signature attached. + /// + /// Per the spec, enforcement points MUST NOT accept unsigned attestations + /// as proof of guard execution. + NotSigned, + /// The signature was present but failed verification. + Invalid(E), +} + +impl std::fmt::Display for VerifyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotSigned => write!(f, "attestation has no signature"), + Self::Invalid(e) => write!(f, "signature verification failed: {}", e), + } + } +} + +impl std::error::Error for VerifyError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Invalid(e) => Some(e), + Self::NotSigned => None, + } + } +} diff --git a/tests/guard.rs b/tests/guard.rs index ab52dcf..de80ce8 100644 --- a/tests/guard.rs +++ b/tests/guard.rs @@ -5,11 +5,73 @@ #![cfg(feature = "guard")] -use path_jail::guard::{FdJail, OpenOptions}; +use path_jail::guard::{FdJail, OpenOptions, Signer, Verifier, VerifyError}; use path_jail::JailError; use std::io::{Read, Write}; use tempfile::tempdir; +// ── Test signer/verifier ───────────────────────────────────────────────────── +// A deterministic stand-in for an Ed25519 signer. NOT cryptographically secure; +// only used to verify the trait surface end-to-end without pulling in a +// crypto dep. Real callers wire up ed25519-dalek / ring / KMS. + +#[derive(Debug)] +struct TestSigner { + key: [u8; 32], +} + +#[derive(Debug)] +struct TestVerifier { + key: [u8; 32], +} + +#[derive(Debug)] +struct BadSignature; +impl std::fmt::Display for BadSignature { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "test verifier rejected signature") + } +} +impl std::error::Error for BadSignature {} + +fn test_signature(key: &[u8; 32], msg: &[u8]) -> [u8; 64] { + // First 32 bytes: key XOR rolling-checksum of msg. + // Last 32 bytes: msg length, repeated. + let mut sig = [0u8; 64]; + let mut acc: u8 = 0; + for (i, b) in msg.iter().enumerate() { + acc = acc.wrapping_add(*b).wrapping_add(i as u8); + } + for i in 0..32 { + sig[i] = key[i] ^ acc.wrapping_add(i as u8); + } + let len = msg.len() as u64; + let len_bytes = len.to_le_bytes(); + for i in 0..32 { + sig[32 + i] = len_bytes[i % 8]; + } + sig +} + +impl Signer for TestSigner { + type Error = std::convert::Infallible; + fn sign(&self, msg: &[u8]) -> Result<[u8; 64], Self::Error> { + Ok(test_signature(&self.key, msg)) + } +} + +impl Verifier for TestVerifier { + type Error = BadSignature; + fn verify(&self, msg: &[u8], signature: &[u8; 64]) -> Result<(), Self::Error> { + let expected = test_signature(&self.key, msg); + if expected == *signature { + Ok(()) + } else { + Err(BadSignature) + } + } +} + // ── Criterion 1 ────────────────────────────────────────────────────────────── // jail.open("../../etc/passwd") → JailError::Escape // (strace would show one openat2 syscall, no file open) @@ -206,16 +268,12 @@ fn ac5_content_bytes_deterministic() { ); } -// ── Criterion 6 (partial) ──────────────────────────────────────────────────── +// ── Criterion 6 ────────────────────────────────────────────────────────────── // Spec AC6: "Signed attestation verifies under configured key." -// Ed25519 signing requires an external key and is future work (tracked separately). -// This test covers the prerequisite: the wire format encodes fields correctly -// and signature is None when no key is configured. -// TODO(ac6): add signing verification once the Ed25519 feature is implemented. #[test] #[cfg(unix)] -fn ac6_partial_attestation_wire_format_and_unsigned() { +fn ac6_attestation_wire_format() { let dir = tempdir().unwrap(); let file = dir.path().join("data.bin"); std::fs::write(&file, b"bytes").unwrap(); @@ -241,10 +299,103 @@ fn ac6_partial_attestation_wire_format_and_unsigned() { assert_eq!(path_len, path_bytes.len()); assert_eq!(&cb[off + 4..off + 4 + path_len], path_bytes); - // Signature is None (no key configured — full AC6 is pending Ed25519 feature) + // Unsigned by default assert!(att.signature.is_none()); } +#[test] +#[cfg(unix)] +fn ac6_signed_attestation_verifies_under_configured_key() { + let dir = tempdir().unwrap(); + let file = dir.path().join("payload.bin"); + std::fs::write(&file, b"x").unwrap(); + + let jail = FdJail::new(dir.path()).unwrap(); + let jf = jail + .open("payload.bin", OpenOptions::new().read(true)) + .unwrap(); + + let key = [7u8; 32]; + let signer = TestSigner { key }; + let verifier = TestVerifier { key }; + + // Sign produces a signature populated attestation. + let signed = jf.sign_attestation(&signer).expect("signer infallible"); + assert!(signed.signature.is_some()); + + // Verify under the same key succeeds. + signed + .verify(&verifier) + .expect("signature must verify under matching key"); +} + +#[test] +#[cfg(unix)] +fn ac6_signature_rejected_under_wrong_key() { + let dir = tempdir().unwrap(); + let file = dir.path().join("payload.bin"); + std::fs::write(&file, b"x").unwrap(); + + let jail = FdJail::new(dir.path()).unwrap(); + let jf = jail + .open("payload.bin", OpenOptions::new().read(true)) + .unwrap(); + + let signer = TestSigner { key: [1u8; 32] }; + let wrong_verifier = TestVerifier { key: [2u8; 32] }; + + let signed = jf.sign_attestation(&signer).unwrap(); + let err = signed + .verify(&wrong_verifier) + .expect_err("verification under a different key must fail"); + assert!(matches!(err, VerifyError::Invalid(_))); +} + +#[test] +#[cfg(unix)] +fn ac6_unsigned_attestation_verify_returns_notsigned() { + let dir = tempdir().unwrap(); + let file = dir.path().join("payload.bin"); + std::fs::write(&file, b"x").unwrap(); + + let jail = FdJail::new(dir.path()).unwrap(); + let jf = jail + .open("payload.bin", OpenOptions::new().read(true)) + .unwrap(); + + let verifier = TestVerifier { key: [0u8; 32] }; + let err = jf + .attestation() + .verify(&verifier) + .expect_err("unsigned attestation must not verify"); + assert!(matches!(err, VerifyError::NotSigned)); +} + +#[test] +#[cfg(unix)] +fn ac6_signature_rejected_on_tampered_field() { + let dir = tempdir().unwrap(); + let file = dir.path().join("payload.bin"); + std::fs::write(&file, b"x").unwrap(); + + let jail = FdJail::new(dir.path()).unwrap(); + let jf = jail + .open("payload.bin", OpenOptions::new().read(true)) + .unwrap(); + + let key = [42u8; 32]; + let signer = TestSigner { key }; + let verifier = TestVerifier { key }; + + let mut signed = jf.sign_attestation(&signer).unwrap(); + // Tamper after signing. + signed.file_inode = signed.file_inode.wrapping_add(1); + let err = signed + .verify(&verifier) + .expect_err("tampered attestation must fail verification"); + assert!(matches!(err, VerifyError::Invalid(_))); +} + // ── Criterion 7 ────────────────────────────────────────────────────────────── // Kernel < 5.6, fallback feature disabled → Jail::new returns UnsupportedKernel // We verify: (a) FdJail::new succeeds on this machine (proving kernel >= 5.6), From 7fe93c6b38d0ea50b3cad760592ea06514938976 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:22:38 -0700 Subject: [PATCH 03/11] chore(ci): supply-chain & release hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds cargo-deny config and a deny job (advisories, licenses, dep bans, sources) — main signal for a security crate that a transitive dep brought in an advisory or a non-permissive license. Adds a semver-checks job that runs on PRs (`|| true` for now since this is the first release with the guard surface — drop the suffix after a baseline ships). Extends test/clippy/docs/MSRV jobs to exercise --all-features so the guard and secure-open feature code is actually linted and doc-built, not just compiled with default features. Hardens release.yml: - Verifies the git tag matches Cargo.toml version before doing anything else (catches "tagged but forgot to bump Cargo.toml" before publish) - Runs `cargo publish --dry-run` before the real publish so packaging failures land in CI logs, not on crates.io --- .github/workflows/ci.yml | 39 ++++++++++++++++++--- .github/workflows/release.yml | 25 ++++++++++++- deny.toml | 66 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 deny.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24967ce..42b22a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,9 @@ jobs: toolchain: ${{ matrix.rust }} - run: cargo build --verbose - run: cargo test --verbose + # Exercise every feature combo so guard/secure-open compile-only paths get linted. + - run: cargo build --all-features --verbose + - run: cargo test --all-features --verbose # Verify we compile on the Minimum Supported Rust Version msrv: @@ -38,8 +41,8 @@ jobs: - uses: dtolnay/rust-toolchain@f7ccc83f9ed1e5b9c81d8a67d7ad1a747e22a561 # master with: toolchain: "1.85" - - run: cargo build --verbose - - run: cargo test --verbose + - run: cargo build --all-features --verbose + - run: cargo test --all-features --verbose # Cross-compile and test on aarch64-linux (ARM servers, Raspberry Pi, etc.) cross: @@ -67,7 +70,7 @@ jobs: - uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable with: components: clippy - - run: cargo clippy -- -D warnings + - run: cargo clippy --all-features --all-targets -- -D warnings fmt: runs-on: ubuntu-latest @@ -83,5 +86,33 @@ jobs: steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable - - run: cargo doc --no-deps + - run: cargo doc --all-features --no-deps + + # Supply-chain check: advisories, licenses, dep bans, sources. + # Config lives in deny.toml. + deny: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable + - name: Install cargo-deny + run: cargo install --locked cargo-deny + - name: Run cargo-deny + run: cargo deny --all-features check + + # Semver discipline: catch breaking changes between commits and the + # last-published version on crates.io. Runs on PRs only; main is allowed to + # bump major/minor explicitly. + semver-checks: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable + - name: Install cargo-semver-checks + run: cargo install --locked cargo-semver-checks + - name: Run cargo-semver-checks + run: cargo semver-checks check-release --all-features || true + # `|| true` for now: 0.4.0 is the first release with the guard surface + # so a baseline doesn't exist. Remove the suffix once a baseline ships. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e2a4930..0970fd1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,6 +9,9 @@ on: permissions: contents: read +env: + CARGO_TERM_COLOR: always + jobs: publish: name: Publish to crates.io @@ -19,11 +22,31 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable + # Sanity gate: the git tag and Cargo.toml version MUST match. + # Catches the "tagged v0.5.0 but forgot to bump Cargo.toml" mistake before + # we ship a confusing release. + - name: Verify tag matches Cargo.toml version + run: | + set -euo pipefail + TAG_VERSION="${GITHUB_REF_NAME#v}" + CARGO_VERSION=$(awk -F'"' '/^version *=/ { print $2; exit }' Cargo.toml) + if [ "$TAG_VERSION" != "$CARGO_VERSION" ]; then + echo "::error::Tag version '$TAG_VERSION' does not match Cargo.toml version '$CARGO_VERSION'" + exit 1 + fi + echo "tag and Cargo.toml agree on version $CARGO_VERSION" + - name: Run tests run: cargo test --all-features + # Pre-flight: validate the publish package without actually pushing it. + # Catches packaging mistakes (missing files, unbuildable crate as + # shipped, license errors) before a real publish — `cargo publish` + # has no rollback story. + - name: Dry-run publish + run: cargo publish --dry-run + - name: Publish run: cargo publish env: CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} - diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..938fdc9 --- /dev/null +++ b/deny.toml @@ -0,0 +1,66 @@ +# cargo-deny configuration for path_jail +# +# Run locally: +# cargo install cargo-deny +# cargo deny check +# +# Enforced in CI by .github/workflows/ci.yml (job: deny). + +[graph] +all-features = true +no-default-features = false + +[output] +feature-depth = 1 + +# ── Security advisories ────────────────────────────────────────────────────── +# RustSec advisory database. New advisories MUST be triaged within the SLA +# documented in SECURITY.md. + +[advisories] +db-path = "~/.cargo/advisory-db" +db-urls = ["https://github.com/rustsec/advisory-db"] +yanked = "deny" +ignore = [] + +# ── Licenses ──────────────────────────────────────────────────────────────── +# Permissive licenses only. If a transitive dep brings in something else, +# the build fails until it's reviewed. + +[licenses] +confidence-threshold = 0.93 +allow = [ + "MIT", + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "BSD-2-Clause", + "BSD-3-Clause", + "ISC", + "Unicode-3.0", + "Unicode-DFS-2016", + "Unlicense", + "Zlib", + "CC0-1.0", +] + +# ── Dependency bans ───────────────────────────────────────────────────────── +# path_jail is a security library — duplicate versions and wildcards are +# both signals worth flagging. + +[bans] +multiple-versions = "warn" +wildcards = "deny" +highlight = "all" +# Empty by default; populate when a specific crate must be banned. +deny = [] + +# ── Source restrictions ───────────────────────────────────────────────────── +# Crates must come from crates.io. No git deps, no private registries. +# (When this changes — e.g., to consume a private Tenuo crate — explicitly +# allowlist the registry/git URL here so the decision is reviewable.) + +[sources] +unknown-registry = "deny" +unknown-git = "deny" +allow-registry = ["https://github.com/rust-lang/crates.io-index"] +allow-git = [] From 8f4d4881e2d938978008a282b937cb48378693ef Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:23:56 -0700 Subject: [PATCH 04/11] docs: add SECURITY.md threat model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the attacker model, per-API guarantees (Jail vs secure-open vs guard) in a single comparison table, out-of-scope threats, an API-selection flowchart, the versioning/MSRV policy, and a vulnerability reporting channel. Calls out explicitly that the default path-based Jail API is not TOCTOU-safe — this is documented today but scattered across README subsections, and it's the thing most likely to bite a downstream user who follows the quick-start verbatim. --- SECURITY.md | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..4de0f29 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,184 @@ +# Security Policy + +This document is the threat model and security contract for `path_jail`. It +exists because every security library needs one: callers can only use the +library correctly if they know what it defends against and what it doesn't. + +If you find a vulnerability, see [Reporting a vulnerability](#reporting-a-vulnerability) below. + +--- + +## Attacker model + +`path_jail` is designed to defend against an attacker who supplies path +strings to your application — for example: + +- A web client uploading a file with a chosen filename +- A user-controlled config value naming a path inside a sandbox directory +- A workflow step naming a file inside a CI working directory + +The attacker can supply: + +- Arbitrary bytes in the path (including `..`, leading `/`, null bytes, magic + link prefixes like `/proc/self/fd/N`) +- A pre-existing symlink inside the jail that points outside the jail +- A pre-existing hard link inside the jail that points to sensitive content +- Concurrent filesystem activity attempting to swap paths between validation + and open (TOCTOU) + +The attacker is assumed to **not** have: + +- Privileges to mount filesystems, run as root inside the jail's filesystem, + call `ptrace`, or otherwise escape the OS sandbox the application runs in +- The ability to modify the running process's memory +- A working kernel exploit + +If your attacker can do any of the above, no userspace library can help — +you need a process-level sandbox (`seccomp`, `landlock`, containers, VMs). + +--- + +## What each API defends against + +`path_jail` ships three API layers with different security/ergonomics +tradeoffs. Pick the strongest one your environment supports. + +| Threat | `Jail` (default) | `secure-open` | `guard` (Linux 5.6+) | +|-----------------------------------------------|:---:|:---:|:---:| +| Path traversal via `..` | ✅ | ✅ | ✅ | +| Absolute path injection (`/etc/passwd`) | ✅ | ✅ | ✅ | +| Null-byte injection | ✅ | ✅ | ✅ | +| Symlink target outside the jail | ✅ | ✅ | ✅ | +| Broken symlinks (cannot verify target) | ✅ | ✅ | ✅ | +| Symlink swap on final component (TOCTOU) | ❌ | ✅ | ✅ | +| Symlink swap on intermediate directories | ❌ | ❌ | ✅ | +| Concurrent rename of jail root mid-operation | ❌ | ❌ | ✅¹ | +| Magic links (`/proc/self/fd`, `/proc/self/root`) | ❌ | ❌ | ✅ | +| Hard link to sensitive content (detect) | ❌² | ❌² | ✅³ | +| Bind-mount escape (opt-in) | ❌ | ❌ | ✅⁴ | +| Atomic open with kernel-enforced containment | ❌ | ❌ | ✅ | +| Signed attestation of the open event | ❌ | ❌ | ✅⁵ | + +Footnotes: + +1. `guard::FdJail` pins an `O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC` fd to the + jail root at construction time. Subsequent renames or replacements of the + root *path* do not affect the jail — all operations remain scoped to the + original directory inode. +2. Hard links are not detectable in user space before open. The path-based + APIs do not stat the opened file and so cannot surface `nlink`. +3. `guard::JailFile::has_hard_links()` exposes `nlink > 1` from the post-open + `fstat`. **Policy is the caller's responsibility** — a content-addressed + store may legitimately use hard links. If your policy rejects hard links, + check `has_hard_links()` **before** reading or writing. +4. Opt in with `OpenOptions::no_xdev(true)` (maps to `RESOLVE_NO_XDEV`). + Off by default to preserve directory-tree containment semantics, which + are what most callers want. +5. Opt in by implementing the `Signer` trait. `path_jail` ships no crypto; + bring your own (`ed25519-dalek`, `ring`, HSM client, KMS, etc.). + +--- + +## Out of scope + +These threats are documented as **not defended** by any API: + +- **Privileged local attackers.** A process with root or `CAP_SYS_ADMIN` on + the host can mount, bind-mount, or `ptrace` around any user-space check. +- **Kernel and filesystem exploits.** A kernel-level bug, a FUSE filesystem + misbehaving, or an `openat2` semantic regression in a specific kernel + version are outside our control. We pin to documented kernel ABI. +- **Side-channel attacks.** Timing, cache, filesystem-metadata leaks. +- **Directory iteration (`read_dir`) and recursive walks.** Iterating jail + contents has its own TOCTOU surface (rename-during-walk) that we do not + currently address. If you walk a directory tree, treat it as untrusted + input on every iteration. +- **Windows.** No Windows-specific protections are implemented. `Jail` and + `secure-open` compile on Windows but provide no defenses beyond the + cross-platform path-string checks; `guard` is Linux-only. +- **Unicode normalization.** Paths are accepted byte-for-byte. We do not + normalize NFC/NFD on macOS or fold case on Windows/macOS. If your storage + layer is case-insensitive, treat `Report.PDF` and `report.pdf` as + potentially the same file. +- **Resource exhaustion.** Very long paths, deep symlink chains, etc. are + rejected by the kernel (`ENAMETOOLONG`, `ELOOP`) but `path_jail` does not + impose its own limits. + +--- + +## Choosing the right API + +```text + ┌──────────────────────────────────┐ +You only need a validated │ Use `Jail::join` / `join_typed`. │ +path (e.g., for logging) → │ Cheap, portable. │ + └──────────────────────────────────┘ + + ┌──────────────────────────────────┐ +You open the file in │ Use `secure-open`. │ +process, on Unix, and need │ Protects final-component swaps. │ +final-component TOCTOU → └──────────────────────────────────┘ + + ┌──────────────────────────────────┐ +Security-critical opens on │ Use `guard` (Linux 5.6+). │ +Linux, attestation needed, │ Kernel-enforced; signable. │ +or hostile multi-tenant → └──────────────────────────────────┘ +``` + +`guard` is the strongest. Use it on Linux where you can. + +--- + +## Versioning & supported releases + +- We follow semver. While we are pre-1.0 (`0.y.z`), minor-version bumps + (`0.4 → 0.5`) may include breaking changes; patch bumps (`0.4.0 → 0.4.1`) + will not. +- Security fixes are issued on the latest minor line. We do not currently + backport to older 0.x lines. +- The MSRV (currently 1.85) may be bumped in any minor release. We treat MSRV + bumps as breaking. + +--- + +## Reporting a vulnerability + +**Do not** open a public GitHub issue for security bugs. + +Use **GitHub Private Vulnerability Reporting** on the +[`tenuo-ai/path_jail` repository](https://github.com/tenuo-ai/path_jail) +(Security tab → "Report a vulnerability"), or email **security@tenuo.ai** +with: + +- A minimal reproducer (Rust code that demonstrates the issue) +- The affected `path_jail` version and feature flags +- The platform (OS, kernel version, architecture) +- Your assessment of the impact (information disclosure / write outside jail + / etc.) + +We aim to acknowledge within 5 business days and to ship a fix within 30 +days for high-severity findings (escape from a documented containment +guarantee). Lower-severity findings (e.g., a missing defense for a +documented out-of-scope threat) will be triaged on the open repository. + +--- + +## A note on the `Jail` default API + +The README quick-start uses `Jail::new` and `Jail::join` — the path-based +API. That API is **not TOCTOU-safe**. It validates a path string and returns +a `PathBuf`; whatever the caller does with that `PathBuf` is a separate +operation with its own race window. + +This is documented but easy to miss. If you operate in any of these +environments, **strongly prefer `guard`** over the path-based API: + +- Multi-tenant systems where another local process can manipulate the + filesystem +- File-upload paths where the same directory is also writable by other + workers +- Anywhere "the file you validated" and "the file you opened" need to be + the same file with certainty + +We may make `guard` the default in a future major release. For now, choose +explicitly. From 09a6b65462bbc408e22dc4c180c5fd71eb99e290 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:34:22 -0700 Subject: [PATCH 05/11] fix(guard): openat2 mode=0, Windows cfg, clippy needless_return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs surfaced by --all-features CI on the v0.5 branch: 1. openat2 returned EINVAL on every call. The kernel requires `how.mode == 0` unless `O_CREAT` or `O_TMPFILE` is in `how.flags`, but we hardcoded `mode: 0o666`. Tests never caught this because previous CI didn't run --all-features on Linux, so the guard integration tests never executed. Now mode is conditional on O_CREAT being set. 2. `--features guard` on Windows failed to compile: the fallback path uses Unix-only items (MetadataExt, custom_flags, OwnedFd). Windows is out of scope per SECURITY.md, so gate `pub mod guard` on cfg(unix) — Windows users with the feature flag get an absent module rather than a build error. 3. Clippy `needless_return` in FdJail::new. The two cfg-gated arms can both be tail expressions; only one compiles per target. --- src/guard/fd_jail.rs | 10 +++++++--- src/lib.rs | 9 ++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/guard/fd_jail.rs b/src/guard/fd_jail.rs index deeca18..46c27e2 100644 --- a/src/guard/fd_jail.rs +++ b/src/guard/fd_jail.rs @@ -413,9 +413,13 @@ mod linux_impl { resolve |= RESOLVE_NO_XDEV; } + // Per openat2(2): `mode` MUST be 0 unless O_CREAT or O_TMPFILE is set, + // otherwise the kernel returns EINVAL. We do not use O_TMPFILE. + let mode: u64 = if flags & O_CREAT != 0 { 0o666 } else { 0 }; + let how = OpenHow { flags, - mode: 0o666, + mode, resolve, }; @@ -602,11 +606,11 @@ impl FdJail { dir_file, )) }; - return Ok(FdJail { + Ok(FdJail { root, root_inode, dirfd, - }); + }) } #[cfg(not(target_os = "linux"))] diff --git a/src/lib.rs b/src/lib.rs index 4063f4f..dda0fb3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,11 +89,14 @@ mod jailed_path; #[cfg(feature = "secure-open")] mod open; -#[cfg(feature = "guard")] -#[cfg(target_os = "linux")] +#[cfg(all(feature = "guard", target_os = "linux"))] pub(crate) mod openat2; -#[cfg(feature = "guard")] +// `guard` is Unix-only — the implementation uses `OwnedFd`, `MetadataExt`, +// `OpenOptionsExt::custom_flags`, etc., which only exist on `cfg(unix)`. +// Windows is explicitly out of scope (see SECURITY.md). Compiling +// `--features guard` on Windows is a no-op rather than a build failure. +#[cfg(all(feature = "guard", unix))] pub mod guard; use std::path::{Path, PathBuf}; From b14733695ae2dc77237b8780de9ad52bd0c68311 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:38:07 -0700 Subject: [PATCH 06/11] chore: deny duplicate versions in cargo-deny; TODO for real no_xdev test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cargo-deny: dep tree is currently clean (cargo tree --duplicates --all-features --target all returns nothing), so flip multiple-versions from "warn" to "deny". For a security crate, duplicates are where advisories hide — one copy gets patched, another doesn't. A future upstream split that forces a duplicate will now fail the build and surface deliberately, either via a cargo update / [patch] resolution or a documented `skip` entry, rather than drifting silently. no_xdev test: replace the comment on the builder test with a concrete TODO describing the privileged-test setup needed for real EXDEV assertion (CAP_SYS_ADMIN + bind mount). The builder test on its own is weak signal and would rot once people stop remembering why it's "just a type-check". --- deny.toml | 13 ++++++++++--- tests/guard.rs | 18 +++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/deny.toml b/deny.toml index 938fdc9..024d299 100644 --- a/deny.toml +++ b/deny.toml @@ -44,15 +44,22 @@ allow = [ ] # ── Dependency bans ───────────────────────────────────────────────────────── -# path_jail is a security library — duplicate versions and wildcards are -# both signals worth flagging. +# path_jail is a security library. We deny both wildcards and duplicate +# versions: duplicates are where advisories hide (one copy patched, another +# isn't) and they bloat binaries. If a transitive dep forces a new duplicate +# in a future bump, the deny build fails and we resolve it deliberately +# (cargo update, [patch], or a documented `skip = [...]` here) rather than +# letting it drift. [bans] -multiple-versions = "warn" +multiple-versions = "deny" wildcards = "deny" highlight = "all" # Empty by default; populate when a specific crate must be banned. deny = [] +# Add entries here ONLY with a justification when an upstream split forces +# a temporary duplicate we can't fix immediately. +skip = [] # ── Source restrictions ───────────────────────────────────────────────────── # Crates must come from crates.io. No git deps, no private registries. diff --git a/tests/guard.rs b/tests/guard.rs index de80ce8..bdb98f2 100644 --- a/tests/guard.rs +++ b/tests/guard.rs @@ -496,15 +496,23 @@ fn toctou_safe_reflects_platform() { // ── no_xdev option ─────────────────────────────────────────────────────────── +// TODO(no_xdev): the test below only proves the builder type-checks. The +// real EXDEV-on-mount-crossing assertion needs a bind mount, which requires +// CAP_SYS_ADMIN and is not available in the default GitHub Actions runner. +// Plan: add a separate workflow (e.g. .github/workflows/privileged-tests.yml) +// that runs under `sudo unshare -m` or a privileged container and includes a +// `#[ignore]`d test marked `#[cfg(target_os = "linux")]` that: +// 1. mkdir jail/mnt && mkdir external +// 2. mount --bind external jail/mnt +// 3. assert FdJail::new(jail).open("mnt/foo", OpenOptions::new().read(true) +// .no_xdev(true)) returns JailError::Escape +// 4. umount jail/mnt +// Until that workflow exists, this builder test is the only signal we have. #[test] #[cfg(unix)] fn no_xdev_option_compiles_and_is_chainable() { - // We can't reliably create a mount point inside a tempdir in CI without - // privileges, so we just exercise the builder. The real EXDEV behaviour - // is enforced by the kernel and verified by integration testing against - // a bind-mounted directory in production deployments. let opts = OpenOptions::new().read(true).no_xdev(true); - let _ = opts; // chainable, type-checks + let _ = opts; } #[test] From 9099e8de4ad841c3b11a9893a53143bf6a72ba97 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:41:34 -0700 Subject: [PATCH 07/11] fix(guard): magic link maps to SymlinkRejected (kernel collapses errno) openat2(2) returns ELOOP for both RESOLVE_NO_MAGICLINKS and RESOLVE_NO_SYMLINKS rejections; userspace cannot tell them apart from the errno. The previous code checked `errno == 105` as "ENOLINK on some kernels" but 105 is actually ENOBUFS, and the real ENOLINK (67) is never produced by openat2. That code path was unreachable. - Drop the bogus ENOLINK branch in map_errno_to_jail_error - Document in JailError::MagicLink that the variant is currently unreachable on Linux; magic-link rejections surface as SymlinkRejected until a future kernel ABI separates the errnos - Relax AC3 to accept SymlinkRejected (the actual production value) in addition to MagicLink and Escape The MagicLink variant is preserved (not deprecated) so callers can already match it for forward-compat with a future kernel change. --- src/error.rs | 9 +++++++++ src/guard/fd_jail.rs | 17 +++++++++++++---- tests/guard.rs | 15 ++++++++++++--- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/error.rs b/src/error.rs index 99c4b47..37c139a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -32,6 +32,15 @@ pub enum JailError { /// A `/proc/self/fd`-style magic link was detected (`RESOLVE_NO_MAGICLINKS`). /// These links can escape the jail regardless of `RESOLVE_BENEATH`. + /// + /// # Currently unreachable + /// + /// The Linux kernel returns the same errno (`ELOOP`) for both + /// `RESOLVE_NO_MAGICLINKS` and `RESOLVE_NO_SYMLINKS` rejections, and + /// userspace cannot tell them apart. As of v0.5, magic-link rejections + /// surface as [`Self::SymlinkRejected`] rather than this variant. The + /// variant is preserved (and not yet deprecated) so callers can match on + /// it if a future kernel ABI separates the two errnos. #[cfg(feature = "guard")] MagicLink { requested: PathBuf }, diff --git a/src/guard/fd_jail.rs b/src/guard/fd_jail.rs index 46c27e2..cec8dbe 100644 --- a/src/guard/fd_jail.rs +++ b/src/guard/fd_jail.rs @@ -448,11 +448,20 @@ mod linux_impl { } fn map_errno_to_jail_error(e: Errno, path: &Path) -> JailError { + // openat2(2) consolidates magic-link rejection (RESOLVE_NO_MAGICLINKS) + // and symlink rejection (RESOLVE_NO_SYMLINKS / symlink loop) into the + // SAME errno: ELOOP. Userspace cannot distinguish a magic-link + // rejection from an ordinary symlink rejection from the errno alone. + // We therefore map ELOOP to `SymlinkRejected` uniformly; the + // `MagicLink` variant is reserved for a future kernel ABI change that + // separates the two (e.g., a distinct ENOLINK or new errno). match e { - Errno::EXDEV => JailError::Escape { requested: path.to_path_buf() }, - Errno::ELOOP => JailError::SymlinkRejected { requested: path.to_path_buf() }, - _ if e.raw() == 105 /* ENOLINK — magic link on some kernels */ => - JailError::MagicLink { requested: path.to_path_buf() }, + Errno::EXDEV => JailError::Escape { + requested: path.to_path_buf(), + }, + Errno::ELOOP => JailError::SymlinkRejected { + requested: path.to_path_buf(), + }, _ => JailError::Io(e.into()), } } diff --git a/tests/guard.rs b/tests/guard.rs index bdb98f2..2e73df9 100644 --- a/tests/guard.rs +++ b/tests/guard.rs @@ -169,10 +169,19 @@ fn ac3_magic_link_blocked() { .open("self/root/etc/passwd", OpenOptions::new().read(true)) .unwrap_err(); - // Either MagicLink (RESOLVE_NO_MAGICLINKS) or Escape (RESOLVE_BENEATH catches the root link) + // The kernel returns ELOOP for RESOLVE_NO_MAGICLINKS rejections, which + // we map to SymlinkRejected — userspace cannot distinguish a magic-link + // rejection from a regular symlink rejection (see JailError::MagicLink + // docs). Escape (EXDEV) is also acceptable if the kernel resolved + // /proc/self/root as a regular link before noticing the cross-mount. assert!( - matches!(err, JailError::MagicLink { .. } | JailError::Escape { .. }), - "expected MagicLink or Escape for /proc/self/root, got: {:?}", + matches!( + err, + JailError::MagicLink { .. } + | JailError::Escape { .. } + | JailError::SymlinkRejected { .. } + ), + "expected MagicLink, Escape, or SymlinkRejected for /proc/self/root, got: {:?}", err ); } From 37692d2e345472841f3db4aee94517a1886f681c Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:42:38 -0700 Subject: [PATCH 08/11] fix(guard): drop unused Errno::raw() helper Last caller was the dead ENOLINK branch removed in 9099e8d. Nothing else uses it; rustc/clippy with -D warnings catches it as dead code. --- src/openat2.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/openat2.rs b/src/openat2.rs index 471ad03..7640489 100644 --- a/src/openat2.rs +++ b/src/openat2.rs @@ -57,10 +57,6 @@ pub(crate) const O_CLOEXEC: u64 = 0o2000000; pub(crate) struct Errno(pub i32); impl Errno { - pub fn raw(self) -> i32 { - self.0 - } - // Errno constants we care about pub const EXDEV: Errno = Errno(18); // Cross-device link / escape attempt pub const ELOOP: Errno = Errno(40); // Too many symlinks / RESOLVE_NO_SYMLINKS From 301db16fb18c08f6cae6e449da040568866dcb36 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:44:26 -0700 Subject: [PATCH 09/11] fix(secure-open): gate to cfg(unix) on Windows Same pattern as the guard fix in 09a6b65. src/open.rs has an inner #![cfg(all(feature = "secure-open", unix))] attribute that empties the file on Windows, but lib.rs declared mod open and re-exported JailedFile gated only on the feature, not on unix. Result: Windows --all-features build saw 'mod open' with no contents and the re-export failed. Add the unix gate to both. --- src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index dda0fb3..33fee30 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,7 +86,9 @@ mod error; mod jail; mod jailed_path; -#[cfg(feature = "secure-open")] +// `secure-open` is Unix-only (uses `OpenOptionsExt::custom_flags`). +// On Windows, enabling the feature is a no-op rather than a build error. +#[cfg(all(feature = "secure-open", unix))] mod open; #[cfg(all(feature = "guard", target_os = "linux"))] @@ -105,7 +107,7 @@ pub use error::JailError; pub use jail::Jail; pub use jailed_path::JailedPath; -#[cfg(feature = "secure-open")] +#[cfg(all(feature = "secure-open", unix))] pub use open::JailedFile; /// Validate a path in one shot. From d46cf58e5b668e0bc6cdf098b3c6585882bcee7d Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:47:00 -0700 Subject: [PATCH 10/11] fix(tests): gate Unix-only test files for Windows --all-features tests/guard.rs now gates on cfg(unix) in addition to feature="guard", matching the module gate in lib.rs. Without this, Windows builds with --all-features fail because the file imports path_jail::guard but the module is cfg-gated away. Also fixes a warning in tests/security.rs:handles_control_characters where `let jail` is unused on Windows (the only usage is inside a cfg(unix) block). Move the binding inside the cfg gate. --- tests/guard.rs | 2 +- tests/security.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/guard.rs b/tests/guard.rs index 2e73df9..a69f1ba 100644 --- a/tests/guard.rs +++ b/tests/guard.rs @@ -3,7 +3,7 @@ //! These tests correspond 1:1 to the spec's acceptance criteria table. //! Run with: `cargo test --features guard` -#![cfg(feature = "guard")] +#![cfg(all(feature = "guard", unix))] use path_jail::guard::{FdJail, OpenOptions, Signer, Verifier, VerifyError}; use path_jail::JailError; diff --git a/tests/security.rs b/tests/security.rs index a737521..8377a35 100644 --- a/tests/security.rs +++ b/tests/security.rs @@ -421,16 +421,18 @@ fn backslash_is_valid_filename_on_unix() { #[test] fn handles_control_characters() { let dir = tempdir().unwrap(); - let jail = Jail::new(dir.path()).unwrap(); // Control characters are technically valid in filenames on Unix // (except null and slash). This is a logging/display issue, not security. #[cfg(unix)] { + let jail = Jail::new(dir.path()).unwrap(); // These should work (though they're ugly) let _ = jail.join("file\n.txt"); // Newline let _ = jail.join("file\t.txt"); // Tab } + #[cfg(not(unix))] + let _ = dir; } #[test] From 0a9333543624377147fbd1ca2946883748ba93a0 Mon Sep 17 00:00:00 2001 From: aimable100 <129232709+aimable100@users.noreply.github.com> Date: Wed, 13 May 2026 11:51:02 -0700 Subject: [PATCH 11/11] fix(docs): gate guard quick-start doctest on cfg(unix) The doctest in lib.rs was gated on feature = "guard" only. On Windows with --all-features the feature is set but the guard module is cfg(unix)-gated away (see 09a6b65 / 301db16), so the doctest body failed to resolve path_jail::guard. Other guard-related doctests are inside src/guard/, which is already cfg(unix)-gated at the module level, so they don't compile on Windows and don't need this fix. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 33fee30..92f75fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,7 +35,7 @@ //! ``` //! //! ```no_run -//! # #[cfg(feature = "guard")] { +//! # #[cfg(all(feature = "guard", unix))] { //! use path_jail::guard::{FdJail, OpenOptions}; //! //! let jail = FdJail::new("/var/uploads")?;