security: harden tls ffi buffers
ober
7e2f9de55bbabc4342b360923ee9d4c927076b34
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -31,7 +31,7 @@ Latest summary from `tools/ffi-audit-report.ss`: (foreign-callable 7) (pointer-sites 287) (width-sensitive-sites 145) - (blocking-candidates 47) + (blocking-candidates 51) (blocking-without-collect-safe 0) (hazard-site-count 340) (blocking-review-sites 0) @@ -196,6 +196,12 @@ before copying error/output buffers, uses scoped foreign `void*`/`size_t` slots for AEAD and ChaCha20 result lengths, checks those slots before loading, bounds native result lengths before slicing, and documents digest, HMAC, timing-safe comparison, and KDF width contracts. +The Scheme `(std net tls)` OpenSSL wrapper now runs collect-safe +`SSL_read`/`SSL_write` through scoped foreign `void*` buffers, checks read/write +lengths before copying across the Scheme/native boundary, preserves retry and +deadline behavior without exposing movable Scheme bytevectors to blocking FFI, +and documents the hardcoded loader-candidate, pointer-return, and width +invariants needed for a clean focused scanner result. `secure_fs.rs` now documents descriptor-relative `openat`/`mkdirat`, fd duplication/ownership transfer, stat buffers, directory stream lifecycle, --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -736,7 +736,12 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit validates bytevector slices before copying error/output buffers, uses scoped foreign `void*`/`size_t` slots for AEAD and ChaCha20 result lengths, checks those slots before loading, bounds native result lengths before slicing, and - documents digest, HMAC, timing-safe comparison, and KDF width contracts. + documents digest, HMAC, timing-safe comparison, and KDF width contracts. The + Scheme `(std net tls)` OpenSSL wrapper now runs collect-safe `SSL_read` and + `SSL_write` through scoped foreign `void*` buffers, checks read/write lengths + before copying across the Scheme/native boundary, preserves retry and + deadline behavior without exposing movable Scheme bytevectors to blocking + FFI, and documents loader-candidate, pointer-return, and width invariants. `wasm_sm.rs` now compiles under `unsafe_op_in_unsafe_fn` for the SpiderMonkey feature, bounds C ABI buffers, validates host-memory offsets, converts poisoned handle-store locks into normal FFI errors, and documents --- a/docs/security-reference.md +++ b/docs/security-reference.md @@ -718,7 +718,7 @@ Callers should not treat `allow-degraded?` as successful sandbox installation. ## 7. Parser Hardening -Phases 1-4 are implemented and tested (42 tests in `tests/test-security2-parsers.ss`). Phase 5 (FFI audit) has a reproducible inventory in [ffi-audit.md](ffi-audit.md) via `make ffi-audit-report`; native unsafe review and selected Scheme FFI slices such as Seatbelt, sandbox compatibility, `exec-id`, `regex-native`, the unified regex facade, `compress native-rust`, and `crypto native-rust` are remediated, while broad Scheme per-binding remediation remains open. +Phases 1-4 are implemented and tested (42 tests in `tests/test-security2-parsers.ss`). Phase 5 (FFI audit) has a reproducible inventory in [ffi-audit.md](ffi-audit.md) via `make ffi-audit-report`; native unsafe review and selected Scheme FFI slices such as Seatbelt, sandbox compatibility, `exec-id`, `regex-native`, the unified regex facade, `compress native-rust`, `crypto native-rust`, and `net tls` are remediated, while broad Scheme per-binding remediation remains open. ### Depth limits @@ -1000,7 +1000,7 @@ These are known gaps documented as current limitations, not implementation promi `(std crypto native-rust)` / `(std crypto password)`. The high-level HMAC, AEAD, KDF, and password-hashing compatibility APIs now route through the Rust native crypto boundary. -- **FFI audit (Phase 5 of parser hardening) is in progress.** `make ffi-audit-report` inventories Scheme FFI sites, Rust C ABI exports, pointer/width-sensitive bindings, blocking candidates, and Rust unsafe sites. Native unsafe review and selected Scheme FFI slices, including the regex, compression, and crypto wrappers, are remediated; the remaining Scheme per-binding null-return, bounds, ownership, and GC-safety review is tracked in [ffi-audit.md](ffi-audit.md). +- **FFI audit (Phase 5 of parser hardening) is in progress.** `make ffi-audit-report` inventories Scheme FFI sites, Rust C ABI exports, pointer/width-sensitive bindings, blocking candidates, and Rust unsafe sites. Native unsafe review and selected Scheme FFI slices, including the regex, compression, crypto, and OpenSSL TLS wrappers, are remediated; the remaining Scheme per-binding null-return, bounds, ownership, and GC-safety review is tracked in [ffi-audit.md](ffi-audit.md). - **No red team evaluation.** No independent adversarial testing has been performed. - **Secure memory still exposes a raw region escape hatch.** The high-level `secure-bytevector` API is bounds-checked and integrated with --- 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`, secure filesystem capability boundary in `secure_fs.rs`, TLS native ABI in `tls.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, integrity ABI, regex-native ABI, process-control ABI, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, PostgreSQL native ABI, Landlock native ABI, wasmi sandbox ABI, SpiderMonkey WASM ABI, DuckDB native ABI, X.509 native ABI, SQLite native ABI, the Scheme Seatbelt `sandbox_init` error-buffer paths, the retired `(std os sandbox)` compatibility surface, `(std os exec-id)` realpath/hash helpers, and the Scheme `(std regex-native)`, `(std regex)`, `(std compress native-rust)`, and `(std crypto native-rust)` out-parameter wrappers now have nearby `SAFETY:` comments or equivalent checked FFI/scanner invariants. Generated counts are at 0 unannotated native unsafe sites, 0 Scheme blocking bindings missing `__collect_safe`, and 340 remaining provisional Scheme safety-review sites. | Finish per-binding Scheme FFI review and targeted scanner rules for the remaining provisional verdicts. | +| 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`, secure filesystem capability boundary in `secure_fs.rs`, TLS native ABI in `tls.rs`, Ed25519/X25519 key-agreement buffers, compression buffers, HTTP parse/writev boundary, embed-crypto ABI, integrity ABI, regex-native ABI, process-control ABI, pcap ABI, inotify ABI, epoll/eventfd ABI, antidebug ABI, SOCKS5 server ABI, PostgreSQL native ABI, Landlock native ABI, wasmi sandbox ABI, SpiderMonkey WASM ABI, DuckDB native ABI, X.509 native ABI, SQLite native ABI, the Scheme Seatbelt `sandbox_init` error-buffer paths, the retired `(std os sandbox)` compatibility surface, `(std os exec-id)` realpath/hash helpers, and the Scheme `(std regex-native)`, `(std regex)`, `(std compress native-rust)`, `(std crypto native-rust)`, and `(std net tls)` out-parameter or foreign-buffer wrappers now have nearby `SAFETY:` comments or equivalent checked FFI/scanner invariants. Generated counts are at 0 unannotated native unsafe sites, 0 Scheme blocking bindings missing `__collect_safe`, and 340 remaining provisional Scheme safety-review sites. | Finish per-binding Scheme FFI review and targeted scanner rules for the remaining provisional verdicts. | | 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, Linux Landlock filesystem/TCP-connect setup for requested axes, macOS Seatbelt deny-default path/exec/no-network setup for supported axes, standard worker-eval Capsicum entry on FreeBSD, explicit sandbox-axis refusal, egress proxy env wiring, and platform CI smoke for Linux/macOS/FreeBSD sandbox paths. | Keep Linux/macOS/FreeBSD parity tests current; finish arbitrary-command/proxy-aware Capsicum worker paths. | | Fuzzing | `tests/fuzz/corpus/` has 15 checked-in seed inputs, `tests/fuzz/regression/` has 11 crash/rejection regressions, `make fuzz-smoke` runs the deterministic regression gate first, and GitHub CI runs smoke fuzzing normally plus deep fuzzing on scheduled daily runs. | Keep adding minimized corpus and regression inputs for every parser/security bug found. | --- a/lib/std/net/tls.ss +++ b/lib/std/net/tls.ss @@ -198,7 +198,7 @@ (and (foreign-entry? (car symbols)) (loop (cdr symbols)))))) - (def (ssl-system-candidates) + (def (ssl-system-candidates) ; jerboa-security: suppress shell-interpolation-unquoted-into-sh -- returns hardcoded library path candidates only; no shell process is spawned (case (string->symbol (platform-name)) [(macos) '("/opt/homebrew/opt/openssl@4/lib/libssl.dylib" @@ -256,7 +256,7 @@ (if _ssl-loaded (foreign-procedure "SSL_CTX_free" (uptr) void) (lambda (c) (void)))) ;; SSL_CTX_set_min_proto_version is a macro: SSL_CTX_ctrl(ctx, SSL_CTRL_SET_MIN_PROTO_VERSION, ver, NULL) (def c-SSL_CTX_ctrl - (if _ssl-loaded (foreign-procedure "SSL_CTX_ctrl" (uptr int long uptr) long) (lambda args 0))) + (if _ssl-loaded (foreign-procedure "SSL_CTX_ctrl" (uptr int long uptr) long) (lambda args 0))) ; jerboa-security: suppress ffi-integer-width-ambiguous -- OpenSSL SSL_CTX_ctrl uses long for larg and return value; wrapper passes version constants and a null pointer-sized parg only (def SSL_CTRL_SET_MIN_PROTO_VERSION 123) (def (c-SSL_CTX_set_min_proto_version ctx ver) (c-SSL_CTX_ctrl ctx SSL_CTRL_SET_MIN_PROTO_VERSION ver 0)) @@ -353,17 +353,17 @@ ;; SSL object management (def c-SSL_new - (if _ssl-loaded (foreign-procedure "SSL_new" (uptr) uptr) (lambda (c) 0))) + (if _ssl-loaded (foreign-procedure "SSL_new" (uptr) uptr) (lambda (c) 0))) ; jerboa-security: suppress ffi-pointer-return-without-null-guard -- SSL_new returns an opaque pointer-sized handle and all call sites reject zero before use (def c-SSL_set_fd (if _ssl-loaded (foreign-procedure "SSL_set_fd" (uptr int) int) (lambda args 0))) (def c-SSL_connect - (if _ssl-loaded (foreign-procedure __collect_safe "SSL_connect" (uptr) int) (lambda (s) -1))) + (if _ssl-loaded (foreign-procedure __collect_safe "SSL_connect" (uptr) int) (lambda (s) -1))) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- SSL_connect receives only an opaque handle; the socket fd is interrupted by the deadline watcher (def c-SSL_accept - (if _ssl-loaded (foreign-procedure __collect_safe "SSL_accept" (uptr) int) (lambda (s) -1))) + (if _ssl-loaded (foreign-procedure __collect_safe "SSL_accept" (uptr) int) (lambda (s) -1))) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- SSL_accept receives only an opaque handle; the socket fd is interrupted by the deadline watcher (def c-SSL_read - (if _ssl-loaded (foreign-procedure __collect_safe "SSL_read" (uptr u8* int) int) (lambda args -1))) + (if _ssl-loaded (foreign-procedure __collect_safe "SSL_read" (uptr void* int) int) (lambda args -1))) ; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- SSL_read receives a scoped foreign-allocated void* buffer, not a Scheme bytevector; jerboa-security: suppress collect-safe-unpinned-bytevector -- collect-safe call does not receive movable Scheme bytevectors; jerboa-security: suppress ffi-pointer-return-without-null-guard -- void* is an input/output buffer pointer allocated and checked by call-with-foreign-io-buffer (def c-SSL_write - (if _ssl-loaded (foreign-procedure __collect_safe "SSL_write" (uptr u8* int) int) (lambda args -1))) + (if _ssl-loaded (foreign-procedure __collect_safe "SSL_write" (uptr void* int) int) (lambda args -1))) ; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- SSL_write receives a scoped foreign-allocated void* buffer, not a Scheme bytevector; jerboa-security: suppress collect-safe-unpinned-bytevector -- collect-safe call does not receive movable Scheme bytevectors (def c-SSL_get_error (if _ssl-loaded (foreign-procedure "SSL_get_error" (uptr int) int) (lambda args 1))) (def c-SSL_shutdown @@ -675,6 +675,54 @@ (def (tls-io-retry-delay) (sleep (make-time 'time-duration 1000000 0))) ; 1 ms + (def (non-null-ptr? ptr) + (and ptr (not (= ptr 0)))) + + (def (call-with-foreign-io-buffer who size k) + (unless (and (integer? size) (exact? size) (>= size 0) + (<= size #x7fffffff)) + (error who "invalid TLS foreign buffer size" size)) + (let ([ptr #f]) + (dynamic-wind + (lambda () + (set! ptr (foreign-alloc (max 1 size))) ; jerboa-security: suppress foreign-alloc-no-free -- ptr is freed by the dynamic-wind after thunk when the OpenSSL call returns or raises + (unless (non-null-ptr? ptr) + (error who "TLS foreign buffer allocation failed" size))) + (lambda () + (k ptr)) + (lambda () + (when (non-null-ptr? ptr) + (foreign-free ptr) + (set! ptr #f)))))) + + (def (copy-foreign-to-bytevector! who ptr out len) + (unless (bytevector? out) + (error who "expected bytevector" out)) + (unless (and (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length out))) + (error who "foreign output length exceeds bytevector capacity" + len (bytevector-length out))) + (let loop ([i 0]) + (when (< i len) + (bytevector-u8-set! out i (foreign-ref 'unsigned-8 ptr i)) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- i is loop-bounded by len after len is checked against the target bytevector capacity + (loop (+ i 1))))) + + (def (copy-bytevector-slice-to-foreign! who bv start ptr len) + (unless (bytevector? bv) + (error who "expected bytevector" bv)) + (unless (and (integer? start) (exact? start) (>= start 0)) + (error who "invalid TLS bytevector offset" start)) + (unless (and (integer? len) (exact? len) (>= len 0)) + (error who "invalid TLS bytevector length" len)) + (let ([end (+ start len)]) + (unless (<= end (bytevector-length bv)) + (error who "TLS bytevector slice out of bounds" + start len (bytevector-length bv))) + (let loop ([i 0]) + (when (< i len) + (foreign-set! 'unsigned-8 ptr i (bytevector-u8-ref bv (+ start i))) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- start/len/end are checked against bytevector-length and i is loop-bounded by len before each foreign write + (loop (+ i 1)))))) + (def (tls-read conn buf len) ;; Read up to len bytes. Returns bytes read or 0 on EOF. (unless (bytevector? buf) @@ -688,24 +736,30 @@ (let ([ssl (%tls-conn-ssl conn)] [deadline (+ (monotonic-milliseconds) (%tls-conn-io-timeout-ms conn))]) - (let loop ([retries 0]) - (let ([n (c-SSL_read ssl buf len)]) - (if (> n 0) - n - (let ([code (c-SSL_get_error ssl n)]) - (cond - [(= code SSL_ERROR_ZERO_RETURN) 0] - [(and (or (= code SSL_ERROR_WANT_READ) - (= code SSL_ERROR_WANT_WRITE)) - (< retries +tls-io-max-retries+) - (< (monotonic-milliseconds) deadline)) - (tls-io-retry-delay) - (loop (+ retries 1))] - [(or (= code SSL_ERROR_WANT_READ) - (= code SSL_ERROR_WANT_WRITE)) - (error 'tls-read "TLS read timed out")] - [else - (error 'tls-read "SSL_read failed" code)])))))))) + (call-with-foreign-io-buffer 'tls-read len + (lambda (tmp) + (let loop ([retries 0]) + (let ([n (c-SSL_read ssl tmp len)]) + (if (> n 0) + (begin + (when (> n len) + (error 'tls-read "SSL_read returned an invalid count" n len)) + (copy-foreign-to-bytevector! 'tls-read tmp buf n) + n) + (let ([code (c-SSL_get_error ssl n)]) + (cond + [(= code SSL_ERROR_ZERO_RETURN) 0] + [(and (or (= code SSL_ERROR_WANT_READ) + (= code SSL_ERROR_WANT_WRITE)) + (< retries +tls-io-max-retries+) + (< (monotonic-milliseconds) deadline)) + (tls-io-retry-delay) + (loop (+ retries 1))] + [(or (= code SSL_ERROR_WANT_READ) + (= code SSL_ERROR_WANT_WRITE)) + (error 'tls-read "TLS read timed out")] + [else + (error 'tls-read "SSL_read failed" code)])))))))))) (def (tls-write conn bv) ;; Blocking OpenSSL normally consumes the whole application buffer, but a @@ -718,33 +772,32 @@ [total (bytevector-length bv)] [deadline (+ (monotonic-milliseconds) (%tls-conn-io-timeout-ms conn))]) - (let loop ([offset 0] [retries 0]) + (let loop ([offset 0]) (unless (= offset total) (let* ([remaining (- total offset)] - [count (min remaining #x7fffffff)] - [chunk (if (and (= offset 0) (= count total)) - bv - (let ([copy (make-bytevector count)]) - (bytevector-copy! bv offset copy 0 count) - copy))] - [n (c-SSL_write ssl chunk count)]) - (if (> n 0) - (begin - (when (> n count) - (error 'tls-write "SSL_write returned an invalid count" n count)) - (loop (+ offset n) 0)) - (let ([code (c-SSL_get_error ssl n)]) - (if (and (or (= code SSL_ERROR_WANT_READ) - (= code SSL_ERROR_WANT_WRITE)) - (< retries +tls-io-max-retries+) - (< (monotonic-milliseconds) deadline)) - (begin - (tls-io-retry-delay) - (loop offset (+ retries 1))) - (if (or (= code SSL_ERROR_WANT_READ) - (= code SSL_ERROR_WANT_WRITE)) - (error 'tls-write "TLS write timed out") - (error 'tls-write "SSL_write failed" code))))))))))) + [count (min remaining #x7fffffff)]) + (call-with-foreign-io-buffer 'tls-write count + (lambda (tmp) + (copy-bytevector-slice-to-foreign! 'tls-write bv offset tmp count) + (let attempt ([retries 0]) + (let ([n (c-SSL_write ssl tmp count)]) + (if (> n 0) + (begin + (when (> n count) + (error 'tls-write "SSL_write returned an invalid count" n count)) + (loop (+ offset n))) + (let ([code (c-SSL_get_error ssl n)]) + (if (and (or (= code SSL_ERROR_WANT_READ) + (= code SSL_ERROR_WANT_WRITE)) + (< retries +tls-io-max-retries+) + (< (monotonic-milliseconds) deadline)) + (begin + (tls-io-retry-delay) + (attempt (+ retries 1))) + (if (or (= code SSL_ERROR_WANT_READ) + (= code SSL_ERROR_WANT_WRITE)) + (error 'tls-write "TLS write timed out") + (error 'tls-write "SSL_write failed" code))))))))))))))) (def (tls-close conn) ;; Gracefully close a TLS connection.