Skip to content

Feat: Add tls_bridge.upstream_insecure to skip origin cert verification #677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aslom wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Feat: Add tls_bridge.upstream_insecure to skip origin cert verification #677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions authbridge/authlib/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ type TLSBridgeConfig struct {
// UpstreamCABundle is an extra-roots PEM file for re-origination (private-CA
// origins the agent trusts); empty == system roots only.
UpstreamCABundle string `yaml:"upstream_ca_bundle" json:"upstream_ca_bundle"`
// UpstreamInsecure disables origin cert verification on re-origination
// (InsecureSkipVerify). For internal/self-signed upstreams when the origin CA
// is unavailable. Prefer UpstreamCABundle when possible.
UpstreamInsecure bool `yaml:"upstream_insecure" json:"upstream_insecure"`
// PassthroughHosts are hosts to tunnel (never intercept). Distinct from
// listener.skip_hosts, which bypasses the whole pipeline; these still run the
// egress gate, they just aren't TLS-terminated.
Expand Down
10 changes: 6 additions & 4 deletions authbridge/authlib/tlsbridge/upstream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import (

// NewUpstreamClient builds the HTTP client the TLS bridge uses to re-originate
// to the real origin. RootCAs = system roots + extraRootsPEM (the agent's
// injected upstream trust). It NEVER sets InsecureSkipVerify and never uses the
// mesh-mTLS dialer — re-origination must verify the origin the way the agent would.
func NewUpstreamClient(extraRootsPEM []byte) (*http.Client, error) {
// injected upstream trust). When insecure is true it sets InsecureSkipVerify so
// re-origination does NOT verify the origin cert — for internal/self-signed
// upstreams (e.g. a private LiteLLM endpoint). Prefer extraRootsPEM
// (upstream_ca_bundle) over insecure whenever the origin CA is available.
func NewUpstreamClient(extraRootsPEM []byte, insecure bool) (*http.Client, error) {
pool, err := x509.SystemCertPool()
if err != nil || pool == nil {
pool = x509.NewCertPool()
Expand All @@ -24,7 +26,7 @@ func NewUpstreamClient(extraRootsPEM []byte) (*http.Client, error) {
}
return &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12},
TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12, InsecureSkipVerify: insecure}, //nolint:gosec // opt-in via upstream_insecure for internal/self-signed upstreams
ForceAttemptHTTP2: true,
MaxIdleConns: 100,
IdleConnTimeout: 90 * time.Second,
Expand Down
10 changes: 8 additions & 2 deletions authbridge/authlib/tlsbridge/upstream_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestUpstreamClient_InjectedRootVerifies(t *testing.T) {
caPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw})

// With the origin's CA injected → verifies.
good, err := NewUpstreamClient(caPEM)
good, err := NewUpstreamClient(caPEM, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand All @@ -26,8 +26,14 @@ func TestUpstreamClient_InjectedRootVerifies(t *testing.T) {
_ = resp.Body.Close()

// Without it (system roots only) → the self-signed httptest cert is rejected.
bare, _ := NewUpstreamClient(nil)
bare, _ := NewUpstreamClient(nil, false)
if _, err := bare.Get(srv.URL); err == nil {
t.Errorf("expected verification failure with system roots only")
}

// insecure=true → self-signed origin is accepted without any injected root.
insecure, _ := NewUpstreamClient(nil, true)
if _, err := insecure.Get(srv.URL); err != nil {
t.Errorf("expected success with insecure=true, got %v", err)
}
}
5 changes: 4 additions & 1 deletion authbridge/cmd/authbridge-proxy/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,13 @@ func main() {
log.Fatalf("tls-bridge upstream_ca_bundle read failed: %v", err)
}
}
up, uerr := tlsbridge.NewUpstreamClient(extra)
up, uerr := tlsbridge.NewUpstreamClient(extra, cfg.TLSBridge.UpstreamInsecure)
if uerr != nil {
log.Fatalf("tls-bridge upstream client failed: %v", uerr)
}
if cfg.TLSBridge.UpstreamInsecure {
log.Printf("tls-bridge: upstream_insecure=true — re-origination does NOT verify the upstream TLS cert")
}
Comment on lines +332 to +338

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use log/slog for the insecure warning.

The new log statement uses log.Printf, but per the coding guidelines, log/slog should be used for logging. Since this is an informational warning about reduced security, slog.Warn is the appropriate choice and matches the pattern used elsewhere in this file. Note that while boot-path log.Fatalf is preserved by design based on learnings, standard informational logging should still use slog.

♻️ Proposed fix 
 		up, uerr := tlsbridge.NewUpstreamClient(extra, cfg.TLSBridge.UpstreamInsecure)
 		if uerr != nil {
 			log.Fatalf("tls-bridge upstream client failed: %v", uerr)
 		}
 		if cfg.TLSBridge.UpstreamInsecure {
-			log.Printf("tls-bridge: upstream_insecure=true — re-origination does NOT verify the upstream TLS cert")
+			slog.Warn("tls-bridge: upstream_insecure=true — re-origination does NOT verify the upstream TLS cert")
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
up, uerr := tlsbridge.NewUpstreamClient(extra, cfg.TLSBridge.UpstreamInsecure)
if uerr != nil {
log.Fatalf("tls-bridge upstream client failed: %v", uerr)
}
if cfg.TLSBridge.UpstreamInsecure {
log.Printf("tls-bridge: upstream_insecure=true — re-origination does NOT verify the upstream TLS cert")
}
up, uerr := tlsbridge.NewUpstreamClient(extra, cfg.TLSBridge.UpstreamInsecure)
if uerr != nil {
log.Fatalf("tls-bridge upstream client failed: %v", uerr)
}
if cfg.TLSBridge.UpstreamInsecure {
slog.Warn("tls-bridge: upstream_insecure=true — re-origination does NOT verify the upstream TLS cert")
}
🤖 Prompt for 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.

In `@authbridge/cmd/authbridge-proxy/main.go` around lines 332 - 338, Replace the
informational log.Printf call in the cfg.TLSBridge.UpstreamInsecure branch with
slog.Warn, preserving the existing warning message and condition. Keep the
adjacent log.Fatalf error handling unchanged.

Sources: Coding guidelines, Learnings

minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
var ports map[int]bool // nil => NewDecision defaults to {443, 8443}
if len(cfg.TLSBridge.Ports) > 0 {
Expand Down
Loading