security: harden nrepl socket boundary
ober
01bbf9848722f4eb9aed7de01ed0c0154ac68491
--- a/docs/ffi-audit.md +++ b/docs/ffi-audit.md @@ -29,13 +29,13 @@ Latest summary from `tools/ffi-audit-report.ss`: (load-shared-object 55) (define-ftype 4) (foreign-callable 7) - (pointer-sites 290) - (width-sensitive-sites 145) + (pointer-sites 293) + (width-sensitive-sites 148) (blocking-candidates 62) (blocking-without-collect-safe 0) - (hazard-site-count 343) + (hazard-site-count 347) (blocking-review-sites 0) - (safety-review-sites 343)) + (safety-review-sites 347)) (native (rust-file-count 29) (c-file-count 5) @@ -214,7 +214,11 @@ and keeps `free-stat` as a compatibility no-op for callers using the exported stat accessors. The Scheme `(std os aproc)` wrapper now routes collect-safe `read`, `write`, and `waitpid` through scoped foreign `void*` buffers/status slots while preserving the existing high-level API, and checks slice bounds -before subprocess write copies. +before subprocess write copies. The Scheme `(std nrepl)` TCP listener now keeps +blocking `accept`, `read`, and `write` off movable Scheme bytevectors by routing +them through scoped foreign `void*` buffers, guards the errno pointer before +dereference, keeps token authentication on an accumulator-based comparison, and +returns generic protocol errors instead of raw condition text. `secure_fs.rs` now documents descriptor-relative `openat`/`mkdirat`, fd duplication/ownership transfer, stat buffers, directory stream lifecycle, @@ -226,7 +230,7 @@ negative accepted fds before adopting them as `TcpStream`, and documents socket shutdown invariants used to break blocked TLS I/O. The generated inventory now reports 390 annotated native unsafe sites, 0 remaining unsafe review sites, and 0 Scheme blocking calls missing `__collect_safe`; the clean-tree Scheme FFI -inventory now reports 343 remaining provisional safety-review sites. +inventory now reports 347 remaining provisional safety-review sites. Remaining work before closing K3-P1-01: --- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -754,7 +754,11 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit native storage. The Scheme `(std os aproc)` wrapper now routes collect-safe `read`, `write`, and `waitpid` through checked Scheme wrappers over scoped foreign `void*` buffers/status slots, and validates subprocess write slice - bounds before copying. + bounds before copying. The Scheme `(std nrepl)` TCP listener now routes + blocking `accept`, `read`, and `write` through scoped foreign `void*` + buffers, guards the errno pointer, keeps token authentication on an + accumulator-based comparison, and returns generic protocol errors instead of + raw condition messages. `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 @@ -771,7 +775,7 @@ the current inventory and remediation trail now live in [ffi-audit.md](ffi-audit shutdown invariants for blocked TLS I/O. The generated report now shows 390 annotated native unsafe sites, 0 remaining unsafe review sites, and 0 Scheme blocking calls missing `__collect_safe`; the clean-tree Scheme FFI inventory - now reports 62 blocking candidates, 0 blocking-review sites, and 343 + now reports 62 blocking candidates, 0 blocking-review sites, and 347 remaining provisional safety-review sites. Remaining work: finish per-binding Scheme FFI review and targeted scanner rules. --- 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`, `crypto native-rust`, `net tls`, `net tls-rustls`, `std os posix`, and `std os aproc` 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`, `net tls`, `net tls-rustls`, `std os posix`, `std os aproc`, and `std nrepl` 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, crypto, OpenSSL TLS, rustls TLS, POSIX, and async-process 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, OpenSSL TLS, rustls TLS, POSIX, async-process, and nREPL TCP 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)`, `(std crypto native-rust)`, `(std net tls)`, `(std net tls-rustls)`, `(std os posix)`, and `(std os aproc)` 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 343 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)`, `(std net tls)`, `(std net tls-rustls)`, `(std os posix)`, `(std os aproc)`, and `(std nrepl)` 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 347 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/src/std/nrepl.ss +++ b/src/std/nrepl.ss @@ -295,7 +295,7 @@ [s (and sid (get-session sid))]) (cond [(and s (nrepl-session-authed s)) #t] - [(token=? (dict-ref msg "token") *server-token*) + [(token=? (dict-ref msg "token") *server-token*) ; jerboa-security: suppress non-constant-time-secret-compare -- token=? performs an accumulator-based character comparison and does not call string=? on secret material (when sid (session-set-authed! sid)) #t] [else #f]))])) @@ -358,7 +358,7 @@ (when (< off len) (let* ([remaining (- len off)] [chunk (make-bytevector remaining)]) - (bytevector-copy! bv off chunk 0 remaining) + (bytevector-copy! bv off chunk 0 remaining) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- off is below len, remaining is len-off, and chunk is allocated to exactly remaining bytes (let ([n (posix-write fd chunk remaining)]) (when (<= n 0) (error 'nrepl-start! "short write to nREPL control file")) @@ -1514,9 +1514,7 @@ (send-response! out (make-response msg "status" (list "error" "done") - "ex" (if (message-condition? exn) - (condition-message exn) - (format "~a" exn))))))) + "ex" "internal server error"))))) (handle-message msg out)) (lp)))))) @@ -1540,10 +1538,10 @@ #f (load-shared-object #f)))) -(def c-socket (foreign-procedure "socket" (int int int) int)) -(def c-bind (foreign-procedure "bind" (int u8* int) int)) +(def c-socket (foreign-procedure "socket" (int int int) int)) ; jerboa-security: suppress ffi-pointer-return-without-null-guard -- socket returns an integer fd/status, and tcp-listen* checks negative failure before use +(def c-bind-raw (foreign-procedure "bind" (int void* int) int)) ; jerboa-security: suppress ffi-pointer-return-without-null-guard -- bind returns int status, not a pointer; jerboa-security: suppress ffi-integer-width-ambiguous -- sockaddr length is checked against the Scheme bytevector before crossing the FFI boundary (def c-listen (foreign-procedure "listen" (int int) int)) -(def c-accept (foreign-procedure __collect_safe "accept" (int u8* u8*) int)) +(def c-accept-raw (foreign-procedure __collect_safe "accept" (int void* void*) int)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- accept receives scoped foreign sockaddr and length slots, not movable Scheme bytevectors; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- foreign-allocated output slots are declared void*; jerboa-security: suppress ffi-pointer-return-without-null-guard -- accept returns a file descriptor, not a pointer (def c-close (foreign-procedure "close" (int) int)) (def c-setsockopt (foreign-procedure "setsockopt" (int int int u8* int) int)) (def c-htons (foreign-procedure "htons" (unsigned-short) unsigned-short)) @@ -1554,8 +1552,8 @@ ;; __varargs_after attribute tells Chez where the variadic boundary is. (def c-fcntl (foreign-procedure (__varargs_after 2) "fcntl" (int int int) int)) (def c-getsockname (foreign-procedure "getsockname" (int u8* u8*) int)) -(def c-read (foreign-procedure __collect_safe "read" (int u8* size_t) ssize_t)) -(def c-write (foreign-procedure __collect_safe "write" (int u8* size_t) ssize_t)) +(def c-read-raw (foreign-procedure __collect_safe "read" (int void* size_t) ssize_t)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- read receives a scoped foreign buffer, not a movable Scheme bytevector; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- foreign-allocated read buffer is declared void*; jerboa-security: suppress ffi-integer-width-ambiguous -- count is bounded by the target bytevector length before crossing the FFI boundary +(def c-write-raw (foreign-procedure __collect_safe "write" (int void* size_t) ssize_t)) ; jerboa-security: suppress collect-safe-unpinned-bytevector -- write receives a scoped foreign buffer, not a movable Scheme bytevector; jerboa-security: suppress u8star-ffi-with-foreign-alloc -- foreign-allocated write buffer is declared void*; jerboa-security: suppress ffi-integer-width-ambiguous -- count is bounded by the source bytevector length before crossing the FFI boundary (def c-errno-location (let ((mt (symbol->string (machine-type)))) @@ -1571,7 +1569,110 @@ (else (foreign-procedure "__errno_location" () void*))))) -(def (get-errno) (foreign-ref 'int (c-errno-location) 0)) +(def (get-errno) + (let ([ptr (c-errno-location)]) + (if (and ptr (not (= ptr 0))) + (foreign-ref 'int ptr 0) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- errno slot is a fixed C int at offset 0 after the runtime pointer is checked for NULL + 0))) + +(def (nrepl-non-null-ptr? ptr) + (and ptr (not (= ptr 0)))) + +(def (call-with-nrepl-foreign-buffer who size k) + (unless (and (integer? size) (exact? size) (>= size 0)) + (error who "invalid 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 released by the dynamic-wind cleanup after the blocking socket syscall returns or raises + (unless (nrepl-non-null-ptr? ptr) + (error who "foreign buffer allocation failed" size))) + (lambda () + (k ptr)) + (lambda () + (when (nrepl-non-null-ptr? ptr) + (foreign-free ptr) + (set! ptr #f)))))) + +(def (call-with-nrepl-foreign-buffers who size-a size-b k) + (call-with-nrepl-foreign-buffer who size-a + (lambda (ptr-a) + (call-with-nrepl-foreign-buffer who size-b + (lambda (ptr-b) + (k ptr-a ptr-b)))))) + +(def (copy-foreign-to-bytevector! who ptr out len) + (unless (and (bytevector? out) (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length out))) + (error who "foreign output length exceeds bytevector capacity" len)) + (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 bounded by len after len is checked against the target bytevector capacity + (loop (+ i 1))))) + +(def (copy-bytevector-to-foreign! who bv ptr len) + (unless (and (bytevector? bv) (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length bv))) + (error who "foreign input length exceeds bytevector capacity" len)) + (let loop ([i 0]) + (when (< i len) + (foreign-set! 'unsigned-8 ptr i (bytevector-u8-ref bv i)) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- i is bounded by len after len is checked against the source bytevector capacity + (loop (+ i 1))))) + +(def (c-bind fd addr-bv len) + (unless (and (bytevector? addr-bv) (integer? len) (exact? len) (>= len 0) + (<= len (bytevector-length addr-bv))) + (error 'c-bind "sockaddr length exceeds bytevector capacity" len)) + (call-with-nrepl-foreign-buffer 'c-bind len + (lambda (ptr) + (copy-bytevector-to-foreign! 'c-bind addr-bv ptr len) + (c-bind-raw fd ptr len)))) + +(def (c-read fd bv count) + (unless (and (bytevector? bv) (integer? count) (exact? count) (>= count 0) + (<= count (bytevector-length bv))) + (error 'c-read "read count exceeds bytevector capacity" count)) + (if (= count 0) + 0 + (call-with-nrepl-foreign-buffer 'c-read count + (lambda (ptr) + (let ([n (c-read-raw fd ptr count)]) + (when (> n 0) + (copy-foreign-to-bytevector! 'c-read ptr bv n)) + n))))) + +(def (c-write fd bv count) + (unless (and (bytevector? bv) (integer? count) (exact? count) (>= count 0) + (<= count (bytevector-length bv))) + (error 'c-write "write count exceeds bytevector capacity" count)) + (if (= count 0) + 0 + (call-with-nrepl-foreign-buffer 'c-write count + (lambda (ptr) + (copy-bytevector-to-foreign! 'c-write bv ptr count) + (c-write-raw fd ptr count))))) + +(def (c-accept fd addr-bv len-bv) + (unless (and (bytevector? addr-bv) (bytevector? len-bv) + (>= (bytevector-length len-bv) 4)) + (error 'c-accept "expected sockaddr and four-byte length bytevectors")) + (let ([addr-size (bytevector-length addr-bv)] + [requested-len (bytevector-s32-native-ref len-bv 0)]) + (unless (and (integer? requested-len) (>= requested-len 0) + (<= requested-len addr-size)) + (error 'c-accept "sockaddr length exceeds bytevector capacity" requested-len)) + (call-with-nrepl-foreign-buffers 'c-accept addr-size 4 + (lambda (addr-ptr len-ptr) + (copy-bytevector-to-foreign! 'c-accept addr-bv addr-ptr addr-size) + (foreign-set! 'int len-ptr 0 requested-len) ; jerboa-security: suppress ffi-pointer-arithmetic-without-bounds -- len-ptr is a scoped four-byte socklen_t slot and offset 0 is the only written field + (let ([rc (c-accept-raw fd addr-ptr len-ptr)]) + (when (>= rc 0) + (let ([actual-len (foreign-ref 'int len-ptr 0)]) + (bytevector-s32-native-set! len-bv 0 actual-len) + (copy-foreign-to-bytevector! 'c-accept addr-ptr addr-bv + (min addr-size (max 0 actual-len))))) + rc))))) + (def EINTR 4) ;; Constants below differ between Linux (the false branch) and BSD-derived