security: hide grpc handler errors
Jaime Fournier <jaimef@linbsd.org>
7e7c4d7c3421b8fc627ed6b75010fa4cc1b1909c
--- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -775,9 +775,11 @@ they are build-time. Close the runtime loop. - **Status:** partially complete. `(std security audit-log)` now exposes `audit-log-use-env-policy!`, workers bind their env policy to per-run audit logs before recording events, and tests assert registered secret values are - redacted from both JSONL and human summaries. Remaining work: protocol - surfaces need opaque client error refs, the scanner needs a condition-message - HTTP-response rule, and release artifact path-leak checks still need a gate. + redacted from both JSONL and human summaries. `(std net grpc)` handler + failures now log internal details server-side and return client-visible + opaque refs. Remaining work: HTTP/router/MCP/LSP/repl-protocol surfaces need + opaque client error refs, the scanner needs a condition-message HTTP-response + rule, and release artifact path-leak checks still need a gate. ### K3-P1-11 — Parameterized-only SQL in the safe surface **Serves:** G1. **Effort:** 2–3 days. --- a/docs/security-reference.md +++ b/docs/security-reference.md @@ -813,7 +813,7 @@ These modules are implemented but not covered in depth above. | Module | Purpose | |--------|---------| | `(std security sanitize)` | Context-aware sanitization: `sanitize-html`, `sanitize-html-attribute`, `sanitize-url-attribute`, `sql-escape`, `sanitize-path`, `safe-path-join`, `sanitize-header-value`, `sanitize-url`. Raises `&path-traversal`, `&header-injection`, `&url-scheme-violation`. | -| `(std security errors)` | Error classification (internal vs client-safe). Generates opaque error references for correlation. Prevents leaking internal details in error responses. | +| `(std security errors)` | Error classification (internal vs client-safe). Generates opaque error references for correlation. Prevents leaking internal details in error responses; `(std net grpc)` uses it for handler failures. | | `(std security audit)` | Append-only JSONL audit log with SHA-256 hash chain. `audit-log!`, `verify-audit-chain`, `check-capability!/audit`. | | `(std security audit-log)` | Per-run structured audit records with JSONL/summary rendering, per-log redactors, and `audit-log-use-env-policy!` to scrub `(std security env)` registered secret values before emission. | | `(std security auth)` | API key stores, session tokens with expiry, auth middleware pattern, rate limiting for auth attempts. | --- a/lib/std/net/grpc.ss +++ b/lib/std/net/grpc.ss @@ -17,7 +17,11 @@ (import (chezscheme) (only (std native-loader) native-loader-ensure-libc-symbol!) (only (jerboa core) def defstruct try catch finally) - (only (jerboa reader) jerboa-read)) + (only (jerboa reader) jerboa-read) + (only (std security errors) + make-safe-error-handler + safe-error-response-message + safe-error-response-reference)) ;; ---- C socket FFI ---- @@ -168,18 +172,28 @@ (close-port inp) (close-port out))) + (def grpc-safe-error-handler + (make-safe-error-handler + (lambda (ref class-name exn) + (fprintf (current-error-port) + "grpc error ref=~a class=~s detail=~s~%" + ref class-name exn)))) + + (def (grpc-error-response class-name exn) + (let ([safe (grpc-safe-error-handler class-name exn)]) + (list 'error + (safe-error-response-message safe) + (safe-error-response-reference safe)))) + (def (dispatch-rpc services msg) (if (and (pair? msg) (symbol? (car msg))) (let ([handler (hashtable-ref services (car msg) #f)]) (if handler (try (let ([result (apply handler (cdr msg))]) (list 'ok result)) - (catch (exn) (list 'error - (if (message-condition? exn) - (condition-message exn) - (format "~s" exn))))) - (list 'error (format "unknown method: ~s" (car msg))))) - (list 'error "malformed request"))) + (catch (exn) (grpc-error-response 'internal-error exn))) + (grpc-error-response 'not-found (format "unknown method: ~s" (car msg))))) + (grpc-error-response 'bad-request "malformed request"))) (def (grpc-server-stop! srv) (grpc-server-rec-running?-set! srv #f) @@ -257,7 +271,12 @@ (if (and (pair? resp) (eq? (car resp) 'ok)) (cadr resp) (if (and (pair? resp) (eq? (car resp) 'error)) - (error "grpc-call" (cadr resp) method) + (error "grpc-call" + (if (and (pair? (cdr resp)) (string? (cadr resp))) + (cadr resp) + "RPC error") + method + (if (pair? (cddr resp)) (caddr resp) #f)) (error "grpc-call" "malformed response" resp)))))) (def (grpc-call-async client method args callback) --- a/tests/test-grpc.ss +++ b/tests/test-grpc.ss @@ -18,6 +18,19 @@ (begin (set! fail (+ fail 1)) (printf "FAIL ~a: got ~s expected ~s~%" name got expected)))))])) +(define (contains-substring? s needle) + (let ([slen (string-length s)] + [nlen (string-length needle)]) + (let outer ([i 0]) + (cond + [(> (+ i nlen) slen) #f] + [else + (let inner ([j 0]) + (cond + [(= j nlen) #t] + [(char=? (string-ref s (+ i j)) (string-ref needle j)) + (inner (+ j 1))] + [else (outer (+ i 1))]))])))) (printf "--- Phase 2e: gRPC ---~%~%") ;; ---- 1. grpc-status ---- @@ -40,6 +53,9 @@ (define-rpc calc-service greet (lambda (name) (string-append "Hello, " name "!"))) +(define-rpc calc-service fail-secret + (lambda () (error 'fail-secret "secret-token-123"))) + ;; Service is a hashtable (test "service-has-add" (procedure? (hashtable-ref calc-service 'add #f)) #t) (test "service-has-multiply" (procedure? (hashtable-ref calc-service 'multiply #f)) #t) @@ -71,7 +87,16 @@ (guard (exn [#t (test "grpc-unknown-method-error" (message-condition? exn) #t)]) (grpc-call client 'unknown-method) - (test "grpc-unknown-method-should-err" #f #t))) + (test "grpc-unknown-method-should-err" #f #t)) + ;; Handler failures must not reveal raw server exception messages. + (guard (exn [#t (test "grpc-handler-error-redacts-secret" + (and (message-condition? exn) + (not (contains-substring? + (condition-message exn) + "secret-token-123"))) + #t)]) + (grpc-call client 'fail-secret) + (test "grpc-handler-error-should-err" #f #t))) ;; Stop server (grpc-server-stop! server)