fix: validate mTLS client identity files - #951
Conversation
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a well-scoped, well-tested mTLS validation fix. Verified there's no circular import (types.py → exc.py is safe since exc.py's telemetry import is lazy), the cert-chain loading logic is correct, and the key-without-cert / combined-PEM behavior changes are intentional and covered by new unit tests. Only one minor (low) consistency note about mixed direct-vs-getattr access in _kernel_tls_kwargs.
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a focused, well-tested mTLS validation fix. Key-without-cert is now rejected on both the Thrift/unified and kernel paths, missing/empty identity files are reported with the failing option name, and the new SSLOptions helpers are covered by thorough unit tests. No circular-import or correctness concerns; one Low note about duplicated file-validation logic between SSLOptions and the kernel backend.
Description
Fix the AUTH-015 and AUTH-016 mTLS conformance gaps for both the Thrift and kernel backends:
SSLOptions, while preserving separate cert/key files, combined cert+key PEM files, and password forwarding on the Thrift pathHow is this tested?
Focused unit coverage passed for
SSLOptions, unified HTTP setup, kernel TLS argument translation, Thrift TLS wiring, and session argument forwarding (33 tests total). The changed production files also pass Black formatting checks.Related Tickets & Documents
Fixes #942