security: harden integrity ffi
ober
405305e04138b403d857f78fb5ac741be821a706
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -41,9 +41,9 @@ Latest summary from `tools/ffi-audit-report.ss`: (c-file-count 5) (no-mangle-exports 196) (unsafe-sites 439) - (unsafe-sites-with-nearby-safety-comment 137) - (unsafe-sites-without-nearby-safety-comment 302) - (unsafe-comment-review-sites 302) + (unsafe-sites-with-nearby-safety-comment 143) + (unsafe-sites-without-nearby-safety-comment 296) + (unsafe-comment-review-sites 296) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -92,8 +92,11 @@ documents parser output/writev C ABI contracts and rejects a null second `embed_crypto.rs` now documents PBKDF2, AEAD, random-fill, and passphrase TTY unsafe invariants, wraps the `usize` encryption ABI in panic containment, and rejects null nonempty plaintext and output-length overflow before creating FFI -slices. The generated inventory now reports 137 annotated native unsafe sites -and 302 remaining unsafe review sites. +slices. `integrity.rs` now documents self-hash, signature-verification, file +region hashing, and FreeBSD `sysctl` unsafe invariants, checks exclusion-region +arithmetic before slicing, and streams caller-sized file regions instead of +allocating a caller-controlled `u64` length. The generated inventory now reports +143 annotated native unsafe sites and 296 remaining unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -635,10 +635,14 @@ not started." `writev` buffer with a nonzero length. `embed_crypto.rs` now documents PBKDF2, AEAD, random-fill, and passphrase TTY unsafe invariants, wraps the `usize` encryption ABI in panic containment, and rejects null nonempty - plaintext and output-length overflow before creating FFI slices. The generated - report now shows 137 annotated native unsafe sites and 302 remaining unsafe - review sites. Remaining work: continue unsafe invariant comments across the - rest of `jerboa-native-rs`. + plaintext and output-length overflow before creating FFI slices. + `integrity.rs` now documents self-hash, signature-verification, file-region + hashing, and FreeBSD `sysctl` unsafe invariants, checks exclusion-region + arithmetic before slicing, and streams caller-sized file regions instead of + allocating a caller-controlled `u64` length. The generated report now shows + 143 annotated native unsafe sites and 296 remaining unsafe review sites. + Remaining work: continue unsafe invariant comments across the rest of + `jerboa-native-rs`. ### K3-P1-02 — TOCTOU-safe filesystem capability checks **Serves:** G2. **Effort:** 1 week. @@ -1126,7 +1130,7 @@ Track these in `docs/status.md` per release: | Confined worker exists | facade, audit lifecycle, output caps, deadline, process-group kill, memory rlimit pre-exec path, explicit sandbox-axis refusal, egress proxy env wiring, and Linux syscall/ptrace seccomp pre-exec setup landed; Landlock path/net and Seatbelt/Capsicum parity pending | yes, tested (P0-02) | | Unclassified raw `read`/FASL/`load` sites | scanner-driven manifest classification gate closed 2026-07-27; trusted build, compiler, REPL/dev, and test surfaces are centrally justified by exact source line | 0 | | FFI bindings audited | scanner inventory and provisional verdicts in [ffi-audit.md](ffi-audit.md); native exports gated and justified | 100% with final verdicts | -| Un-annotated Rust `unsafe` blocks | 302 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 296 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | | Fuzz corpora / crash regressions | 0 / 0 | per-parser corpora + every crash a test | | Exploit-shaped regression tests | ~0 | ≥ 1 per historical finding | | Sandboxed-by-default app entry | no | yes (P0-06) | --- a/docs/reviews/2026-07-27-native-export-review.sexp +++ b/docs/reviews/2026-07-27-native-export-review.sexp @@ -72,11 +72,11 @@ (export (symbol "jerboa_inotify_init") (file "jerboa-native-rs/src/inotify_native.rs") (line 6) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 63)))) (export (symbol "jerboa_inotify_read") (file "jerboa-native-rs/src/inotify_native.rs") (line 68) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 69)))) (export (symbol "jerboa_inotify_rm_watch") (file "jerboa-native-rs/src/inotify_native.rs") (line 49) (scheme-callers 1 (caller (file "lib/std/os/inotify-native.ss") (line 67)))) - (export (symbol "jerboa_integrity_hash_file") (file "jerboa-native-rs/src/integrity.rs") (line 272) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 63)))) - (export (symbol "jerboa_integrity_hash_region") (file "jerboa-native-rs/src/integrity.rs") (line 194) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 60)))) - (export (symbol "jerboa_integrity_hash_self") (file "jerboa-native-rs/src/integrity.rs") (line 65) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 53)))) - (export (symbol "jerboa_integrity_sign_verify") (file "jerboa-native-rs/src/integrity.rs") (line 134) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 57)))) - (export (symbol "jerboa_integrity_verify_hash") (file "jerboa-native-rs/src/integrity.rs") (line 96) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 55)))) + (export (symbol "jerboa_integrity_hash_file") (file "jerboa-native-rs/src/integrity.rs") (line 322) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 63)))) + (export (symbol "jerboa_integrity_hash_region") (file "jerboa-native-rs/src/integrity.rs") (line 224) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 60)))) + (export (symbol "jerboa_integrity_hash_self") (file "jerboa-native-rs/src/integrity.rs") (line 68) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 53)))) + (export (symbol "jerboa_integrity_sign_verify") (file "jerboa-native-rs/src/integrity.rs") (line 141) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 57)))) + (export (symbol "jerboa_integrity_verify_hash") (file "jerboa-native-rs/src/integrity.rs") (line 101) (scheme-callers 1 (caller (file "lib/std/os/integrity.ss") (line 55)))) (export (symbol "jerboa_landlock_abi_version") (file "jerboa-native-rs/src/landlock.rs") (line 65) (scheme-callers 6 (caller (file "lib/std/os/landlock-native.ss") (line 78)) (caller (file "lib/std/os/landlock.ss") (line 12)) (caller (file "lib/std/os/landlock.ss") (line 56)) (caller (file "lib/std/os/landlock.ss") (line 57)) (caller (file "lib/std/os/limits/sandbox.ss") (line 455)) (caller (file "lib/std/os/sandbox.ss") (line 75)))) (export (symbol "jerboa_landlock_add_net_rule") (file "jerboa-native-rs/src/landlock.rs") (line 201) (scheme-callers 1 (caller (file "lib/std/os/landlock-native.ss") (line 86)))) (export (symbol "jerboa_landlock_add_path_rule") (file "jerboa-native-rs/src/landlock.rs") (line 120) (scheme-callers 1 (caller (file "lib/std/os/landlock-native.ss") (line 83)))) --- a/docs/status.md +++ b/docs/status.md @@ -25,7 +25,7 @@ release artifacts are built as Jerboa multicall binaries with `jerboa`, | Area | Current state | Remaining work | |---|---|---| | Kimi security handoff | [kimi3-security-recommmendations.md](kimi3-security-recommmendations.md) is the backlog. Dated evidence and review manifests live under [reviews/](reviews/). | Keep new security evidence in dated review records and summarize the current release state here. | -| FFI audit phase 5 | [ffi-audit.md](ffi-audit.md) records the scanner output, provisional Scheme binding verdicts, and native Rust export inventory. `make native-export-review-check` gates native export decisions. The worker-launch native path in `aproc.rs`/`seccomp.rs`, crypto FFI buffer path in `crypto.rs`, secure-memory region lifecycle in `secure_mem.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, and embed-crypto ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 302. | Continue adding `SAFETY:` invariant comments near the remaining Rust unsafe sites. | +| FFI audit phase 5 | [ffi-audit.md](ffi-audit.md) records the scanner output, provisional Scheme binding verdicts, and native Rust export inventory. `make native-export-review-check` gates native export decisions. The worker-launch native path in `aproc.rs`/`seccomp.rs`, crypto FFI buffer path in `crypto.rs`, secure-memory region lifecycle in `secure_mem.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, and integrity ABI now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 296. | Continue adding `SAFETY:` invariant comments near the remaining Rust unsafe sites. | | Native Rust exports | The native export review now has 190 exported functions: 183 tracked Scheme references and 7 retained standalone C/binary helpers. The previous 35 no-Scheme-reference removal candidates no longer have C ABI export markers. | Re-run `make native-export-review-check` whenever adding or removing native exports. | | Confined worker | `(std security worker)` provides the facade, audit lifecycle, output caps, deadlines, process-group kill, memory rlimit pre-exec setup, Linux syscall/ptrace seccomp pre-exec setup for requested axes, explicit sandbox-axis refusal, and egress proxy env wiring. | Install native Landlock path/net rules in the worker pre-exec path and keep Linux/macOS/FreeBSD parity tests current. | | Safe surface | Direct scripts default to the safe prelude; raw access requires `--unsafe-prelude` or `(jerboa prelude unsafe)`. | Continue moving risky APIs behind explicit unsafe imports as new modules land. | --- a/jerboa-native-rs/src/integrity.rs +++ b/jerboa-native-rs/src/integrity.rs @@ -33,6 +33,9 @@ fn read_self_exe() -> Result<Vec<u8>, String> { 12, /* KERN_PROC_PATHNAME */ -1, ]; + // SAFETY: sysctl receives a fixed four-integer MIB, a writable buffer + // with its length slot, and null new-value pointers because this is a + // read-only query for the current process pathname. let rc = unsafe { sysctl( mib.as_ptr(), @@ -82,6 +85,8 @@ pub extern "C" fn jerboa_integrity_hash_self(output: *mut u8, output_len: usize) }; let hash = digest::digest(&digest::SHA256, &binary); + // SAFETY: output is non-null above, output_len >= 32, and hash.as_ref() + // is exactly a 32-byte SHA-256 digest. unsafe { std::ptr::copy_nonoverlapping(hash.as_ref().as_ptr(), output, 32); } @@ -113,6 +118,8 @@ pub extern "C" fn jerboa_integrity_verify_hash(expected: *const u8, expected_len }; let hash = digest::digest(&digest::SHA256, &binary); + // SAFETY: expected is non-null above, expected_len is exactly 32, and + // callers provide a readable expected hash buffer. let exp = unsafe { std::slice::from_raw_parts(expected, 32) }; #[allow(deprecated)] @@ -162,17 +169,40 @@ pub extern "C" fn jerboa_integrity_sign_verify( }; // Zero out the exclusion region (where the signature itself lives) - let exc_off = exclude_offset as usize; - let exc_len = exclude_len as usize; + let exc_off = match usize::try_from(exclude_offset) { + Ok(n) => n, + Err(_) => { + set_last_error("exclusion offset exceeds platform size".to_string()); + return -1; + } + }; + let exc_len = match usize::try_from(exclude_len) { + Ok(n) => n, + Err(_) => { + set_last_error("exclusion length exceeds platform size".to_string()); + return -1; + } + }; if exc_len > 0 { - if exc_off + exc_len > binary.len() { + let exc_end = match exc_off.checked_add(exc_len) { + Some(n) => n, + None => { + set_last_error("exclusion region overflows platform size".to_string()); + return -1; + } + }; + if exc_end > binary.len() { set_last_error("exclusion region exceeds binary size".to_string()); return -1; } - binary[exc_off..exc_off + exc_len].fill(0); + binary[exc_off..exc_end].fill(0); } + // SAFETY: pubkey is non-null above, pubkey_len is exactly 32, and + // callers provide a readable Ed25519 public key buffer. let pk = unsafe { std::slice::from_raw_parts(pubkey, 32) }; + // SAFETY: sig is non-null above, sig_len is exactly 64, and callers + // provide a readable Ed25519 signature buffer. let s = unsafe { std::slice::from_raw_parts(sig, 64) }; let verify_key = signature::UnparsedPublicKey::new(&signature::ED25519, pk); @@ -209,6 +239,8 @@ pub extern "C" fn jerboa_integrity_hash_region( return -1; } + // SAFETY: path is non-null above and the C ABI contract requires a + // readable path_len-byte UTF-8 path buffer. let path_bytes = unsafe { std::slice::from_raw_parts(path, path_len) }; let path_str = match std::str::from_utf8(path_bytes) { Ok(s) => s, @@ -234,28 +266,46 @@ pub extern "C" fn jerboa_integrity_hash_region( } } - let data = if length > 0 { - let mut buf = vec![0u8; length as usize]; - match file.read_exact(&mut buf) { - Ok(()) => buf, - Err(e) => { - set_last_error(format!("read failed: {}", e)); - return -1; + let mut context = digest::Context::new(&digest::SHA256); + let mut buf = [0u8; 8192]; + if length > 0 { + let mut remaining = length; + while remaining > 0 { + let want = std::cmp::min(remaining, buf.len() as u64) as usize; + match file.read(&mut buf[..want]) { + Ok(0) => { + set_last_error("read failed: unexpected EOF".to_string()); + return -1; + } + Ok(n) => { + context.update(&buf[..n]); + remaining -= n as u64; + } + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => { + set_last_error(format!("read failed: {}", e)); + return -1; + } } } } else { // Read from offset to end - let mut buf = Vec::new(); - match file.read_to_end(&mut buf) { - Ok(_) => buf, - Err(e) => { - set_last_error(format!("read failed: {}", e)); - return -1; + loop { + match file.read(&mut buf) { + Ok(0) => break, + Ok(n) => context.update(&buf[..n]), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => { + set_last_error(format!("read failed: {}", e)); + return -1; + } } } }; - let hash = digest::digest(&digest::SHA256, &data); + let hash = context.finish(); + // SAFETY: output is non-null above, output_len >= 32, and hash.as_ref() + // is exactly a 32-byte SHA-256 digest. unsafe { std::ptr::copy_nonoverlapping(hash.as_ref().as_ptr(), output, 32); } @@ -277,3 +327,36 @@ pub extern "C" fn jerboa_integrity_hash_file( ) -> i32 { jerboa_integrity_hash_region(path, path_len, 0, 0, output, output_len) } + +#[cfg(test)] +mod tests { + use super::{jerboa_integrity_hash_region, jerboa_integrity_sign_verify}; + + #[test] + fn sign_verify_rejects_exclusion_range_overflow() { + let pubkey = [0u8; 32]; + let sig = [0u8; 64]; + + assert_eq!( + jerboa_integrity_sign_verify( + pubkey.as_ptr(), + pubkey.len(), + sig.as_ptr(), + sig.len(), + u64::MAX, + 1, + ), + -1 + ); + } + + #[test] + fn hash_region_rejects_null_path() { + let mut out = [0u8; 32]; + + assert_eq!( + jerboa_integrity_hash_region(std::ptr::null(), 1, 0, 0, out.as_mut_ptr(), out.len()), + -1 + ); + } +}