Feat: Auto-generate the tls_bridge CA when generate_ca is set - #707
Conversation
📝 WalkthroughWalkthroughChangesTLS bridge configuration now supports optional self-signed CA generation. CA creation is centralized, persisted with defined permissions, validated through new tests, and integrated into proxy startup with generation warnings and client trust guidance. TLS bridge CA provisioning
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Proxy
participant EnsureFileSource
participant FileSystem
participant TLSBridge
Proxy->>EnsureFileSource: initialize CA source
EnsureFileSource->>FileSystem: check tls.crt and tls.key
EnsureFileSource->>FileSystem: generate and persist CA when configured
EnsureFileSource-->>Proxy: return CA source and generated status
Proxy->>TLSBridge: construct bridge with CA source
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/authlib/tlsbridge/ca.go`:
- Around line 158-181: Update EnsureFileSource so the generate=true load path
also checks ca.crt; when tls.crt and tls.key exist but the trust file is
missing, repair or regenerate ca.crt and surface any failure instead of
returning generated=false silently. Preserve NewFileSource behavior for complete
material and for present-but-invalid or partially present CA files, without
overwriting existing CA material.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b5e4f4f-b9e0-4af1-bbe0-d6b300db91e0
📒 Files selected for processing (4)
authbridge/authlib/config/config.goauthbridge/authlib/tlsbridge/ca.goauthbridge/authlib/tlsbridge/ca_test.goauthbridge/cmd/authbridge-proxy/main.go
5d3f6f4 to
4deea9b
Compare
mrsabath
left a comment
There was a problem hiding this comment.
Summary
Clean, security-conscious change. I verified the safety-critical claims against the code (not just the PR body):
- ✅ Key perms — signing key persisted
0600, cert +ca.crt0644(NewGeneratedFileSource); the test assertsmode & 0o077 == 0. - ✅ Generation gate —
EnsureFileSourcemints only whengenerate && !fileExists(tls.crt) && !fileExists(tls.key). Any presence (even partial) falls through toNewFileSource. - ✅ Never overwrites present-but-invalid — garbage/partial material returns the loud
NewFileSourceerror and is left on disk untouched (tested). - ✅ In-cluster path untouched —
generate_cadefaults to false (bool zero value), so a missing cert-manager Secret stilllog.Fatalfs. Fail-loud preserved. - ✅ Refactor is behavior-preserving —
genSelfSignedCAkeeps the exact cert template (NotBefore -1m,NotAfter +365d,IsCA,CertSign|DigitalSignature);NewEphemeralSource's output is unchanged, it just delegates now.slogwas already imported. - ✅ Tests are assertive and cover the corners — write+reload-as-valid-CA, key perms, generate-when-absent, fail-loud-when-
generate=false, load-without-overwrite, no-overwrite-when-present-but-invalid, and nested-parent-dir creation.
All Go CI (authlib, authbridge-proxy, authbridge-envoy), CodeQL(go), and Trivy are green.
一举两得 (yī jǔ liǎng dé, "one action, two gains"): it removes the manual openssl friction from the standalone demo and hardens the missing/invalid-cert path in the same stroke.
One non-blocking thought (no change needed): if a user ends up with exactly one of tls.crt/tls.key present, they correctly get a fail-loud load error rather than a regenerate — the safe choice — but the error may not obviously hint "remove the stray file to regenerate." Fine to leave to the startup logs.
Areas reviewed: Go (config / tlsbridge / cmd), Security, Tests, commit conventions
Commits: 1, signed-off (DCO passes)
CI status: green
Verdict: APPROVE
Running authbridge-proxy standalone as a forward proxy with tls_bridge required minting a signing CA by hand (openssl) into ca_dir: main.go always loaded tls.crt/tls.key via NewFileSource and fatally exited when they were absent. Add tls_bridge.generate_ca (default false). When true AND the ca_dir CA set is incomplete, the bridge mints a self-signed CA (reusing the existing ephemeral-CA machinery), persists tls.crt/tls.key/ca.crt into ca_dir, and logs the exact NODE_EXTRA_CA_CERTS=<ca_dir>/ca.crt line for clients to trust. The manual openssl step disappears from the local demo. Default (generate_ca=false) is unchanged: a missing/invalid ca_dir CA still fails loudly, preserving the in-cluster contract where the CA is a mounted cert-manager Secret. Robust against partial writes: the three files are written atomically (temp + rename), and EnsureFileSource regenerates when generate=true and the on-disk set is INCOMPLETE (any of tls.crt/tls.key/ca.crt missing). This self-heals two partial-write failure modes — an orphaned tls.key that would otherwise wedge every subsequent boot (NewFileSource fatal on the missing cert), and a missing ca.crt trust anchor that loads fine yet leaves clients unable to verify forged leaves. A COMPLETE-but-invalid set is never overwritten (still fails loud, so a real Secret is protected). Implementation: - config: TLSBridgeConfig.GenerateCA (generate_ca); note that ca_dir must persist across restarts (ephemeral storage re-mints each boot). - tlsbridge: extract genSelfSignedCA (adds PKCS#8 key PEM); NewEphemeralSource delegates to it. NewGeneratedFileSource writes key 0600, cert+ca.crt 0644 via an atomic temp+rename helper. EnsureFileSource loads a complete set or self-heals an incomplete one. - cmd/authbridge-proxy: call EnsureFileSource; warn with the trust-cert hint when generated. Tests: generated CA reloads and is a valid signing CA; key not group/other-readable; generate-when-absent; fail-loud when generate=false; load-without-overwrite when complete; complete-but-invalid not overwritten; self-heal of incomplete sets (orphaned key, orphaned cert, missing ca.crt); missing parent dirs created. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
4deea9b to
e6d4853
Compare
Summary
Running
authbridge-proxystandalone as a forward proxy withtls_bridgerequired minting a signing CA by hand (openssl) intoca_dir—main.goalways loadedtls.crt/tls.keyviaNewFileSourceandlog.Fatalf'd when they were absent. Thatopensslstep is the biggest friction point in the local demo.Change
Add
tls_bridge.generate_ca(default false). Whentrueand theca_dirCA set is incomplete, the bridge mints a self-signed CA (reusing the existing ephemeral-CA machinery), persiststls.crt/tls.key/ca.crtintoca_dir, and logs the exactNODE_EXTRA_CA_CERTS=<ca_dir>/ca.crtline to trust. The local demo drops the wholeopensslblock:then
HTTPS_PROXY=http://localhost:8081 NODE_EXTRA_CA_CERTS=/tmp/ab/ca/ca.crt claude.Safety — in-cluster behavior unchanged
generate_cadefaults to false, so the operator/cluster path is untouched:generate_ca=false→ alwaysNewFileSource; a missing or invalid CA stillFatals (fail-loud on a missing cert-manager Secret).generate_ca=true+ a complete set present → loads it; a complete-but-invalid set is never overwritten (fails loud, so a real Secret is protected).Robustness against partial writes
generate_ca=trueself-heals an incomplete on-disk set — any oftls.crt/tls.key/ca.crtmissing → regenerate all three. This closes two partial-write failure modes:tls.key(killed/errored after the key write, before the cert) that would otherwise wedge every subsequent boot —NewFileSourcefatals on the missing cert forever, reintroducing the manualrmthis feature removes;ca.crttrust anchor that loads fine yet leaves clients unable to verify the forged leaves (no WARN, opaque client-side TLS errors).The three files are also written atomically (temp + rename), so a reader never sees a half-written cert/key.
Implementation
config:TLSBridgeConfig.GenerateCA(generate_ca); doc notesca_dirmust persist across restarts (ephemeral storage re-mints each boot → clients re-trust).tlsbridge: factoredgenSelfSignedCA(now also emits the PKCS#8 key PEM);NewEphemeralSourcedelegates to it.NewGeneratedFileSourcewrites key0600, cert +ca.crt0644via an atomic temp+rename helper.EnsureFileSourceloads a complete set, or self-heals an incomplete one.cmd/authbridge-proxy: callEnsureFileSource;WARNwith the trust-cert hint when generated.Discoverability is via the
generate_cadoc comment and the startupWARN(which prints the exactNODE_EXTRA_CA_CERTSpath) — no separate demo doc exists to update.Testing Instructions
Tests cover: generated CA reloads via
NewFileSourceand is a valid signing CA; key not group/other-readable; generate-when-absent; fail-loud whengenerate=false; load-without-overwrite when the set is complete; complete-but-invalid not overwritten; self-heal of incomplete sets (orphaned key, orphaned cert, missingca.crt); missing parent dirs created.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com