v0.5 production hardening: aarch64, attestation signing, supply-chain CI, threat model#2
Merged
Conversation
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.
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<E> enum (NotSigned vs Invalid(E))
- JailFile::sign_attestation(&S) -> Result<Attestation, S::Error>
- Attestation::verify(&V) -> Result<(), VerifyError<V::Error>>
- 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.
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
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.
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.
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".
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.
Last caller was the dead ENOLINK branch removed in 9099e8d. Nothing else uses it; rustc/clippy with -D warnings catches it as dead code.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
1d88b33openat2syscall + opt-inRESOLVE_NO_XDEV(mount-point containment). Refactors away the rawfstat(which had arch-specific struct-stat layouts) in favor ofFile::metadata().7d6241fSigner/Verifiertraits +JailFile::sign_attestation+Attestation::verify. Implements spec AC6 (signed-attestation verification) without vendoring crypto — callers wire uped25519-dalek,ring, KMS, etc.7fe93c6cargo-denyconfig + job (advisories, licenses, dep bans, sources),cargo-semver-checksPR job,--all-featureseverywhere (so guard/secure-open code is actually linted),release.ymltag/version match +--dry-rungate.8f4d488SECURITY.mdthreat model — attacker model, per-API defense table, out-of-scope list, API-selection flowchart, MSRV/versioning policy, vulnerability reporting channel.Test plan
cargo test --all-featurespasses locally (48 security + 13 secure_open + 13 guard + 14 doctests)cargo clippy --all-features --all-targets -- -D warningscleancargo fmt --checkcleancargo-denyjob picks up no advisories or license issuesaarch64-unknown-linux-gnusucceeds (replaces previouscompile_error!)