security: harden key agreement ffi buffers
ober
2976cc0403741f5d62c12339a688f1536790ab29
--- 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 438) - (unsafe-sites-with-nearby-safety-comment 97) - (unsafe-sites-without-nearby-safety-comment 341) - (unsafe-comment-review-sites 341) + (unsafe-sites-with-nearby-safety-comment 111) + (unsafe-sites-without-nearby-safety-comment 327) + (unsafe-comment-review-sites 327) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -82,8 +82,11 @@ nonzero-null input checks for HMAC and AEAD optional buffers and checked AEAD seal output length addition for overflow. `secure_mem.rs` now documents the `sysconf`, `mmap`, guard-page `mprotect`, pointer-offset, `mlock`/`munlock`, `madvise`, `munmap`, wipe, and random-fill invariants around the secure memory -region lifecycle. The generated inventory now reports 97 annotated native -unsafe sites and 341 remaining unsafe review sites. +region lifecycle. `ed25519.rs` and `x25519.rs` now document the key buffer, +signature output, shared-secret output, and HKDF slice/output invariants; the +HKDF wrapper now also rejects null optional buffers with nonzero lengths. The +generated inventory now reports 111 annotated native unsafe sites and 327 +remaining unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -626,9 +626,11 @@ not started." also gained invariant comments for digest/HMAC/random/compare/AEAD/KDF buffer sites plus fail-closed nonzero-null input checks and AEAD seal length overflow checks. `secure_mem.rs` now documents the mmap/mlock/guard-page, - wipe, free, and random-fill unsafe invariants. The generated report now shows - 97 annotated native unsafe sites and 341 remaining unsafe review sites. - Remaining work: continue unsafe invariant comments across the rest of + wipe, free, and random-fill unsafe invariants. `ed25519.rs` and `x25519.rs` + now document the key/signature/shared-secret/HKDF FFI buffer invariants, and + HKDF rejects null optional buffers with nonzero lengths. The generated report + now shows 111 annotated native unsafe sites and 327 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 @@ -1117,7 +1119,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 | 341 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 327 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 @@ -4,7 +4,7 @@ (policy scheme-callers-or-documented) (summary (exports 190) (scheme-referenced 183) (no-scheme-reference 7)) (exports - (export (symbol "ed25519_derive_pubkey_standalone") (file "jerboa-native-rs/src/ed25519.rs") (line 44) (scheme-callers 0 (none))) + (export (symbol "ed25519_derive_pubkey_standalone") (file "jerboa-native-rs/src/ed25519.rs") (line 50) (scheme-callers 0 (none))) (export (symbol "ed25519_sign_standalone") (file "jerboa-native-rs/src/ed25519.rs") (line 9) (scheme-callers 0 (none))) (export (symbol "embed_decrypt") (file "jerboa-native-rs/src/embed_crypto.rs") (line 98) (scheme-callers 0 (none))) (export (symbol "embed_encrypt") (file "jerboa-native-rs/src/embed_crypto.rs") (line 43) (scheme-callers 0 (none))) --- 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`, and secure-memory region lifecycle in `secure_mem.rs` now have nearby `SAFETY:` comments, reducing the generated unannotated unsafe-site count to 341. | 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`, and Ed25519/X25519 key-agreement buffers now have nearby `SAFETY:` comments, reducing the generated unannotated unsafe-site count to 327. | 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/ed25519.rs +++ b/jerboa-native-rs/src/ed25519.rs @@ -18,6 +18,8 @@ pub extern "C" fn ed25519_sign_standalone( if data.is_null() && datalen > 0 { return -1; } + // SAFETY: seed is non-null above and the C ABI contract requires a + // readable 32-byte Ed25519 seed. let seed_bytes: [u8; 32] = unsafe { let mut arr = [0u8; 32]; std::ptr::copy_nonoverlapping(seed, arr.as_mut_ptr(), 32); @@ -27,9 +29,13 @@ pub extern "C" fn ed25519_sign_standalone( let msg = if datalen == 0 { &[] } else { + // SAFETY: nonzero datalen requires a non-null data pointer checked + // above; callers provide a readable message buffer of this length. unsafe { std::slice::from_raw_parts(data, datalen) } }; let signature = signing_key.sign(msg); + // SAFETY: sig_out is non-null above and the C ABI contract requires a + // writable 64-byte signature output buffer. unsafe { std::ptr::copy_nonoverlapping(signature.to_bytes().as_ptr(), sig_out, 64); } @@ -45,6 +51,8 @@ pub extern "C" fn ed25519_derive_pubkey_standalone(seed: *const u8, pubkey_out: if seed.is_null() || pubkey_out.is_null() { return -1; } + // SAFETY: seed is non-null above and the C ABI contract requires a + // readable 32-byte Ed25519 seed. let seed_bytes: [u8; 32] = unsafe { let mut arr = [0u8; 32]; std::ptr::copy_nonoverlapping(seed, arr.as_mut_ptr(), 32); @@ -52,6 +60,8 @@ pub extern "C" fn ed25519_derive_pubkey_standalone(seed: *const u8, pubkey_out: }; let signing_key = SigningKey::from_bytes(&seed_bytes); let public_key = signing_key.verifying_key(); + // SAFETY: pubkey_out is non-null above and the C ABI contract requires a + // writable 32-byte public-key output buffer. unsafe { std::ptr::copy_nonoverlapping(public_key.as_bytes().as_ptr(), pubkey_out, 32); } --- a/jerboa-native-rs/src/x25519.rs +++ b/jerboa-native-rs/src/x25519.rs @@ -20,6 +20,8 @@ pub extern "C" fn jerboa_x25519_generate_keypair(private_out: *mut u8, public_ou .unwrap(); let secret = StaticSecret::from(key_bytes); let public = PublicKey::from(&secret); + // SAFETY: output pointers are non-null above and the C ABI contract + // requires writable 32-byte buffers for private and public keys. unsafe { std::ptr::copy_nonoverlapping(secret.as_bytes().as_ptr(), private_out, 32); std::ptr::copy_nonoverlapping(public.as_bytes().as_ptr(), public_out, 32); @@ -42,9 +44,13 @@ pub extern "C" fn jerboa_x25519_public_from_private( return -1; } let mut arr = [0u8; 32]; + // SAFETY: private_key is non-null, private_len == 32, and `arr` is a + // 32-byte destination. unsafe { std::ptr::copy_nonoverlapping(private_key, arr.as_mut_ptr(), 32) }; let secret = StaticSecret::from(arr); let public = PublicKey::from(&secret); + // SAFETY: public_out is non-null above and the C ABI contract requires + // a writable 32-byte public-key output buffer. unsafe { std::ptr::copy_nonoverlapping(public.as_bytes().as_ptr(), public_out, 32); } @@ -75,6 +81,8 @@ pub extern "C" fn jerboa_x25519_diffie_hellman( } let mut priv_arr = [0u8; 32]; let mut pub_arr = [0u8; 32]; + // SAFETY: all pointers are non-null and key lengths were validated to + // exactly 32 bytes above; destination arrays are 32 bytes. unsafe { std::ptr::copy_nonoverlapping(our_private, priv_arr.as_mut_ptr(), 32); std::ptr::copy_nonoverlapping(their_public, pub_arr.as_mut_ptr(), 32); @@ -82,6 +90,8 @@ pub extern "C" fn jerboa_x25519_diffie_hellman( let secret = StaticSecret::from(priv_arr); let public = PublicKey::from(pub_arr); let shared = secret.diffie_hellman(&public); + // SAFETY: shared_out is non-null and shared_len >= 32 above, so the + // caller-provided output buffer can receive the 32-byte shared secret. unsafe { std::ptr::copy_nonoverlapping(shared.as_bytes().as_ptr(), shared_out, 32); } @@ -109,15 +119,25 @@ pub extern "C" fn jerboa_hkdf_sha256( set_last_error("null pointer".into()); return -1; } + if (salt.is_null() && salt_len > 0) || (info.is_null() && info_len > 0) { + set_last_error("null optional buffer with nonzero length".into()); + return -1; + } + // SAFETY: ikm is non-null above and the C ABI contract requires it to + // point to a readable `ikm_len`-byte buffer. let ikm_slice = unsafe { std::slice::from_raw_parts(ikm, ikm_len) }; let salt_opt = if salt.is_null() || salt_len == 0 { None } else { + // SAFETY: salt is non-null here and the C ABI contract requires a + // readable `salt_len`-byte buffer. Some(unsafe { std::slice::from_raw_parts(salt, salt_len) }) }; let info_slice = if info.is_null() || info_len == 0 { &[] } else { + // SAFETY: info is non-null here and the C ABI contract requires a + // readable `info_len`-byte buffer. unsafe { std::slice::from_raw_parts(info, info_len) } }; @@ -127,9 +147,49 @@ pub extern "C" fn jerboa_hkdf_sha256( set_last_error("HKDF expand failed".into()); return -1; } + // SAFETY: output is non-null above and the C ABI contract requires a + // writable `output_len`-byte destination. unsafe { std::ptr::copy_nonoverlapping(out.as_ptr(), output, output_len); } 0 }) } + +#[cfg(test)] +mod tests { + use super::jerboa_hkdf_sha256; + + #[test] + fn hkdf_rejects_null_optional_buffers_with_nonzero_lengths() { + let ikm = [1u8; 32]; + let mut output = [0u8; 32]; + + assert_eq!( + jerboa_hkdf_sha256( + ikm.as_ptr(), + ikm.len(), + std::ptr::null(), + 1, + std::ptr::null(), + 0, + output.as_mut_ptr(), + output.len(), + ), + -1 + ); + assert_eq!( + jerboa_hkdf_sha256( + ikm.as_ptr(), + ikm.len(), + std::ptr::null(), + 0, + std::ptr::null(), + 1, + output.as_mut_ptr(), + output.len(), + ), + -1 + ); + } +}