security: harden embed crypto ffi
ober
c9781bc07cf461d7f643ca427d29c0eb8f7d33a5
--- 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 125) - (unsafe-sites-without-nearby-safety-comment 314) - (unsafe-comment-review-sites 314) + (unsafe-sites-with-nearby-safety-comment 137) + (unsafe-sites-without-nearby-safety-comment 302) + (unsafe-comment-review-sites 302) (export-review-sites 196)) (vendor (jsqlite (path "vendor/jsqlite") (status accepted-risk-cve-gated))) @@ -86,10 +86,14 @@ 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 compression FFI wrappers now document input/output/result-slot invariants and -reject oversized decompression caps before `u64` overflow. The generated -inventory now reports 125 annotated native unsafe sites and 314 remaining -unsafe review sites. `http_parse.rs` now documents parser output/writev C ABI -contracts and rejects a null second `writev` buffer with a nonzero length. +reject oversized decompression caps before `u64` overflow. `http_parse.rs` now +documents parser output/writev C ABI contracts and rejects a null second +`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 inventory now reports 137 annotated native unsafe sites +and 302 remaining unsafe review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -632,9 +632,13 @@ not started." documents compression/decompression FFI buffer invariants and rejects oversized decompression caps before `u64` overflow. `http_parse.rs` now documents parser output/writev C ABI contracts and rejects a null second - `writev` buffer with a nonzero length. The generated report now shows 125 - annotated native unsafe sites and 314 remaining unsafe review sites. Remaining - work: continue unsafe invariant comments across the rest of `jerboa-native-rs`. + `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`. ### K3-P1-02 — TOCTOU-safe filesystem capability checks **Serves:** G2. **Effort:** 1 week. @@ -1122,7 +1126,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 | 314 unsafe review sites without nearby `SAFETY:` comments in [ffi-audit.md](ffi-audit.md) | 0 | +| Un-annotated Rust `unsafe` blocks | 302 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 @@ -6,11 +6,11 @@ (exports (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))) + (export (symbol "embed_decrypt") (file "jerboa-native-rs/src/embed_crypto.rs") (line 121) (scheme-callers 0 (none))) + (export (symbol "embed_encrypt") (file "jerboa-native-rs/src/embed_crypto.rs") (line 49) (scheme-callers 0 (none))) (export (symbol "embed_pbkdf2_sha256") (file "jerboa-native-rs/src/embed_crypto.rs") (line 19) (scheme-callers 0 (none))) - (export (symbol "embed_random_bytes") (file "jerboa-native-rs/src/embed_crypto.rs") (line 157) (scheme-callers 0 (none))) - (export (symbol "embed_read_passphrase") (file "jerboa-native-rs/src/embed_crypto.rs") (line 180) (scheme-callers 0 (none))) + (export (symbol "embed_random_bytes") (file "jerboa-native-rs/src/embed_crypto.rs") (line 188) (scheme-callers 0 (none))) + (export (symbol "embed_read_passphrase") (file "jerboa-native-rs/src/embed_crypto.rs") (line 213) (scheme-callers 0 (none))) (export (symbol "jerboa_aead_open") (file "jerboa-native-rs/src/crypto.rs") (line 431) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 161)))) (export (symbol "jerboa_aead_seal") (file "jerboa-native-rs/src/crypto.rs") (line 343) (scheme-callers 1 (caller (file "lib/std/crypto/native-rust.ss") (line 140)))) (export (symbol "jerboa_antidebug_check_all") (file "jerboa-native-rs/src/antidebug.rs") (line 147) (scheme-callers 1 (caller (file "lib/std/os/antidebug.ss") (line 46)))) --- 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, and HTTP parse/writev boundary now have nearby `SAFETY:` comments, with the generated unannotated unsafe-site count at 314. | 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, 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. | | 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/embed_crypto.rs +++ b/jerboa-native-rs/src/embed_crypto.rs @@ -5,7 +5,7 @@ //! //! Uses ring's audited implementations instead of hand-rolled C. -use crate::panic::ffi_wrap; +use crate::panic::{ffi_wrap, ffi_wrap_usize}; use ring::{aead, pbkdf2, rand}; use std::num::NonZeroU32; @@ -28,8 +28,14 @@ pub extern "C" fn embed_pbkdf2_sha256( if password.is_null() || salt.is_null() || out.is_null() || out_len == 0 || iterations == 0 { return; } + // SAFETY: password is non-null above and the C ABI contract requires a + // readable password_len-byte buffer. let pw = unsafe { std::slice::from_raw_parts(password, password_len) }; + // SAFETY: salt is non-null above and the C ABI contract requires a readable + // salt_len-byte buffer. let s = unsafe { std::slice::from_raw_parts(salt, salt_len) }; + // SAFETY: out is non-null above and the C ABI contract requires a writable + // out_len-byte destination. let output = unsafe { std::slice::from_raw_parts_mut(out, out_len) }; if let Some(iters) = NonZeroU32::new(iterations) { pbkdf2::derive(pbkdf2::PBKDF2_HMAC_SHA256, iters, s, pw, output); @@ -47,48 +53,65 @@ pub extern "C" fn embed_encrypt( plaintext_len: usize, out: *mut u8, ) -> usize { - if key.is_null() || nonce.is_null() || out.is_null() { - return 0; - } + ffi_wrap_usize(|| { + if key.is_null() || nonce.is_null() || out.is_null() { + return 0; + } + if plaintext.is_null() && plaintext_len > 0 { + return 0; + } + let output_len = match plaintext_len.checked_add(OVERHEAD) { + Some(n) => n, + None => return 0, + }; - let k = unsafe { std::slice::from_raw_parts(key, 32) }; - let n = unsafe { std::slice::from_raw_parts(nonce, NONCE_SIZE) }; - let pt = if plaintext_len == 0 || plaintext.is_null() { - &[] - } else { - unsafe { std::slice::from_raw_parts(plaintext, plaintext_len) } - }; - let output = unsafe { std::slice::from_raw_parts_mut(out, plaintext_len + OVERHEAD) }; + // SAFETY: key is non-null above and the C ABI contract requires a + // readable 32-byte key. + let k = unsafe { std::slice::from_raw_parts(key, 32) }; + // SAFETY: nonce is non-null above and the C ABI contract requires a + // readable NONCE_SIZE-byte nonce. + let n = unsafe { std::slice::from_raw_parts(nonce, NONCE_SIZE) }; + let pt = if plaintext_len == 0 { + &[] + } else { + // SAFETY: nonzero plaintext_len requires a non-null plaintext + // pointer checked above; callers provide a readable buffer. + unsafe { std::slice::from_raw_parts(plaintext, plaintext_len) } + }; + // SAFETY: out is non-null above and the C ABI contract requires a + // writable output_len-byte buffer. + let output = unsafe { std::slice::from_raw_parts_mut(out, output_len) }; - // 1. Copy nonce to out[0..12] - output[..NONCE_SIZE].copy_from_slice(n); + // 1. Copy nonce to out[0..12] + output[..NONCE_SIZE].copy_from_slice(n); - // 2. Copy plaintext to out[28..28+pt_len] (after nonce+tag slot) - output[OVERHEAD..OVERHEAD + plaintext_len].copy_from_slice(pt); + // 2. Copy plaintext to out[28..28+pt_len] (after nonce+tag slot) + output[OVERHEAD..OVERHEAD + plaintext_len].copy_from_slice(pt); - // 3. Seal in place, get separate tag - let unbound_key = match aead::UnboundKey::new(&aead::CHACHA20_POLY1305, k) { - Ok(uk) => uk, - Err(_) => return 0, - }; - let sealing_key = aead::LessSafeKey::new(unbound_key); - let nonce_val = match aead::Nonce::try_assume_unique_for_key(n) { - Ok(nv) => nv, - Err(_) => return 0, - }; + // 3. Seal in place, get separate tag + let unbound_key = match aead::UnboundKey::new(&aead::CHACHA20_POLY1305, k) { + Ok(uk) => uk, + Err(_) => return 0, + }; + let sealing_key = aead::LessSafeKey::new(unbound_key); + let nonce_val = match aead::Nonce::try_assume_unique_for_key(n) { + Ok(nv) => nv, + Err(_) => return 0, + }; - match sealing_key.seal_in_place_separate_tag( - nonce_val, - aead::Aad::empty(), - &mut output[OVERHEAD..OVERHEAD + plaintext_len], - ) { - Ok(tag) => { - // 4. Copy tag to out[12..28] - output[NONCE_SIZE..OVERHEAD].copy_from_slice(tag.as_ref()); - plaintext_len + OVERHEAD + match sealing_key.seal_in_place_separate_tag( + nonce_val, + aead::Aad::empty(), + &mut output[OVERHEAD..OVERHEAD + plaintext_len], + ) { + Ok(tag) => { + // 4. Copy tag to out[12..28] + output[NONCE_SIZE..OVERHEAD].copy_from_slice(tag.as_ref()); + output_len + } + Err(_) => 0, } - Err(_) => 0, - } + }) } /// Decrypt and verify ChaCha20-Poly1305 AEAD ciphertext. @@ -110,7 +133,11 @@ pub extern "C" fn embed_decrypt( return -1; } + // SAFETY: key is non-null above and the C ABI contract requires a + // readable 32-byte key. let k = unsafe { std::slice::from_raw_parts(key, 32) }; + // SAFETY: input is non-null above and input_len >= OVERHEAD, so callers + // provide a readable encrypted payload buffer. let inp = unsafe { std::slice::from_raw_parts(input, input_len) }; let ct_len = input_len - OVERHEAD; @@ -121,6 +148,8 @@ pub extern "C" fn embed_decrypt( // ring's open_in_place expects: ciphertext || tag (appended) // We need at least ct_len + TAG_SIZE bytes in the output buffer + // SAFETY: out is non-null above and the C ABI contract requires a + // writable work buffer of ct_len + TAG_SIZE bytes. let work_buf = unsafe { std::slice::from_raw_parts_mut(out, ct_len + TAG_SIZE) }; work_buf[..ct_len].copy_from_slice(ct_bytes); work_buf[ct_len..ct_len + TAG_SIZE].copy_from_slice(tag_bytes); @@ -141,6 +170,8 @@ pub extern "C" fn embed_decrypt( &mut work_buf[..ct_len + TAG_SIZE], ) { Ok(plaintext) => { + // SAFETY: out_len is non-null above and points to a writable + // usize result slot. unsafe { *out_len = plaintext.len(); } @@ -163,6 +194,8 @@ pub extern "C" fn embed_random_bytes(buf: *mut u8, len: usize) -> i32 { return 0; } let rng = rand::SystemRandom::new(); + // SAFETY: buf is non-null above and the C ABI contract requires a + // writable len-byte buffer. let output = unsafe { std::slice::from_raw_parts_mut(buf, len) }; match rand::SecureRandom::fill(&rng, output) { Ok(()) => 0, @@ -188,6 +221,10 @@ pub extern "C" fn embed_read_passphrase(prompt: *const u8, buf: *mut u8, buflen: if buf.is_null() || buflen <= 1 { return -1; } + // SAFETY: buf is non-null and buflen > 1 above. This block calls POSIX TTY + // functions with fixed C strings, restores terminal attributes after read + // setup succeeds, writes only within buf[0..buflen), and handles EINTR for + // prompt/newline writes and byte reads. unsafe { let path = b"/dev/tty\0".as_ptr() as *const c_char; let fd: c_int = open(path, O_RDWR); @@ -259,3 +296,45 @@ pub extern "C" fn embed_read_passphrase(prompt: *const u8, buf: *mut u8, buflen: len } } + +#[cfg(test)] +mod tests { + use super::{embed_encrypt, OVERHEAD}; + + #[test] + fn encrypt_rejects_null_plaintext_with_nonzero_length() { + let key = [7u8; 32]; + let nonce = [3u8; 12]; + let mut out = [0u8; OVERHEAD + 1]; + + assert_eq!( + embed_encrypt( + key.as_ptr(), + nonce.as_ptr(), + std::ptr::null(), + 1, + out.as_mut_ptr(), + ), + 0 + ); + } + + #[test] + fn encrypt_rejects_output_length_overflow() { + let key = [7u8; 32]; + let nonce = [3u8; 12]; + let plaintext = [1u8; 1]; + let mut out = [0u8; OVERHEAD + 1]; + + assert_eq!( + embed_encrypt( + key.as_ptr(), + nonce.as_ptr(), + plaintext.as_ptr(), + usize::MAX, + out.as_mut_ptr(), + ), + 0 + ); + } +} --- a/jerboa-native-rs/src/panic.rs +++ b/jerboa-native-rs/src/panic.rs @@ -42,6 +42,23 @@ pub fn ffi_wrap<F: FnOnce() -> i32 + panic::UnwindSafe>(f: F) -> i32 { } } +pub fn ffi_wrap_usize<F: FnOnce() -> usize + panic::UnwindSafe>(f: F) -> usize { + match panic::catch_unwind(f) { + Ok(code) => code, + Err(e) => { + let msg = if let Some(s) = e.downcast_ref::<&str>() { + s.to_string() + } else if let Some(s) = e.downcast_ref::<String>() { + s.clone() + } else { + "unknown panic".to_string() + }; + set_last_error(msg); + 0 + } + } +} + pub fn ffi_wrap_ptr<F: FnOnce() -> *mut u8 + panic::UnwindSafe>(f: F) -> *mut u8 { match panic::catch_unwind(f) { Ok(ptr) => ptr,