Skip to content

feat(sandbox): auto-detect TLS and terminate unconditionally for credential injection #533

@johntmyers

Description

@johntmyers

Problem Statement

The sandbox proxy requires explicit tls: terminate configuration on each endpoint for credential injection and L7 inspection to work. This creates multiple confusing failure modes:

User configuration What happens User experience
L4 only (no protocol/tls) on port 443 Raw tunnel, no credential rewriting 401 from upstream (placeholder openshell:resolve:env:KEY leaks verbatim)
protocol: rest on port 443, no tls Proxy tries plaintext L7 on TLS bytes "Protocol mismatch" error, connection fails
protocol: rest + tls: terminate on port 80 Proxy tries TLS handshake on plaintext TLS handshake failure
tls: passthrough on HTTPS Same as omitting tls Looks like explicit choice but breaks L7

Users shouldn't need to think about TLS termination — it's infrastructure, not policy. The proxy should detect TLS automatically, terminate it when present, and always inject credentials when available.

Proposed Design

Core change: auto-detect TLS at the proxy layer

After L4 OPA allows a CONNECT, the proxy peeks the first bytes of the client connection:

  • TLS ClientHello (0x16) → terminate TLS (MITM), then proceed to credential injection and optional L7 inspection
  • HTTP method verb → plaintext HTTP, proceed directly to credential injection and optional L7 inspection
  • Other bytes → raw binary relay (copy_bidirectional)

TLS termination becomes unconditional when TLS is detected. L7 inspection remains opt-in via protocol + access/rules.

New connection flow

Client CONNECT host:port
  │
  ├── OPA L4 check (allow/deny by host+port+binary)
  │   └── deny → 403 Forbidden
  │
  ├── Peek first bytes
  │   ├── TLS ClientHello (0x16) → Terminate TLS
  │   │   ├── Present dynamic cert to client
  │   │   ├── Connect TLS to upstream
  │   │   ├── Inject credentials (if SecretResolver has them)
  │   │   ├── Log HTTP requests (method, path, status) for observability
  │   │   ├── L7 config exists? → Parse HTTP, evaluate OPA per-request
  │   │   └── No L7 config? → Relay plaintext bidirectionally
  │   │
  │   ├── HTTP method verb → Plaintext HTTP
  │   │   ├── Inject credentials
  │   │   ├── L7 config exists? → Parse and evaluate
  │   │   └── No L7 config? → Relay bidirectionally
  │   │
  │   └── Other bytes → Raw binary relay (copy_bidirectional)
  │
  └── tls: skip → Bypass auto-detection, raw tunnel (escape hatch)

tls field changes

Value Behavior
Missing/empty (default) Auto-detect — peek bytes, terminate if TLS detected
skip New — explicit opt-out. Raw tunnel, no termination, no credential injection. Escape hatch for edge cases (client cert mTLS to upstream, non-standard TLS protocols)
terminate Deprecated — treated as auto-detect, log warning
passthrough Deprecated — treated as auto-detect, log warning

Deprecation warning example:

WARN: 'tls: terminate' is deprecated; TLS termination is now automatic.
      Use 'tls: skip' to explicitly disable. This field will be removed in a future version.

What stays the same

  • protocol field: still controls whether L7 inspection happens (rest, future sql)
  • enforcement field: still controls L7 deny behavior (audit vs enforce)
  • access / rules fields: still control allowed HTTP methods/paths
  • SecretResolver placeholder mechanism: unchanged
  • L4 OPA evaluation: unchanged
  • L7 OPA evaluation: unchanged, just no longer coupled to TLS config

New behavior: "terminate but no L7" path

When TLS is terminated but no protocol is configured, the proxy:

  1. Terminates TLS (can see plaintext)
  2. Injects credentials via SecretResolver (rewrites HTTP headers)
  3. Logs HTTP requests for observability (method, path, response status)
  4. Relays all traffic — no OPA L7 evaluation, no enforcement
  5. Re-encrypts to upstream

This is the "transparent credential-injecting proxy" mode that makes L4-only rules work with provider credentials.

tls: skip behavior

When tls: skip is set:

  • No TLS auto-detection
  • Raw copy_bidirectional tunnel
  • No credential injection (placeholders leak — intentional)
  • No L7 inspection
  • Use case: client cert mTLS to upstream, custom TLS protocols, debugging

Implementation Plan

1. l7/tls.rs — Add looks_like_tls() helper

Peek first 3 bytes: 0x16 0x03 0x01-0x03 (TLS handshake + version). Non-destructive peek via TcpStream::peek().

2. l7/mod.rs — Update TlsMode enum and parsing

enum TlsMode {
    Auto,       // default — peek and terminate if TLS
    Skip,       // explicit opt-out — raw tunnel
}

Parse "skip"Skip. Parse "terminate" / "passthrough"Auto + log deprecation warning. Parse missing/empty → Auto.

Remove validation that requires protocol when tls: terminate is set (TLS termination no longer implies L7).

3. proxy.rs — Restructure handle_tcp_connection

Replace the current three-branch dispatch (L7+terminate / L7+passthrough / L4-only) with:

  1. Check for tls: skip → raw tunnel
  2. Peek bytes → detect TLS or plaintext HTTP
  3. If TLS → terminate → check for L7 config → inspect or relay
  4. If HTTP → check for L7 config → inspect or relay
  5. Otherwise → raw relay

The key structural change: TLS termination moves before the L7 config check.

4. l7/relay.rs — Add "relay with credential injection only" path

For the terminate-but-no-L7 case: parse HTTP requests minimally (for credential rewriting and observability logging), but skip OPA evaluation. Forward everything.

5. policy.rs — Remove has_tls_terminate_endpoints field

No longer needed. The SandboxPolicy struct loses this field. All callers updated.

6. lib.rs — Remove PR #528 workaround

The SecretResolver bypass when no terminate endpoints exist is no longer needed. SecretResolver::from_provider_env() always creates placeholders; the proxy always terminates TLS; credentials always get rewritten.

7. Validation changes (l7/mod.rs)

  • Remove: warning about L7 endpoints lacking tls: terminate
  • Add: tls: skip + protocol: rest on port 443 → warn that L7 inspection won't work
  • Keep: enforcement: enforce requires protocol

8. Tests

  • Add looks_like_tls() unit test with real ClientHello bytes
  • Add test for deprecated tls field warning
  • Add test for tls: skip escape hatch behavior
  • Update testdata/sandbox-policy.yaml — remove explicit tls: terminate
  • Remove has_tls_terminate_endpoints from all test SandboxPolicy construction
  • Existing L7 tests should pass unchanged (auto-detect produces same behavior as explicit terminate)

Backward Compatibility

Existing config Change? Outcome
tls: terminate + L7 Deprecation warning Same behavior
tls: passthrough + L7 on 443 Fixed Was broken (protocol mismatch), now works
L4 only on 443 Improved Credentials now injected automatically
Explicit tls: passthrough Minor change Now terminates; migrate to tls: skip
Non-TLS endpoints (port 80) No change Auto-detect sees plaintext, no termination

No existing working configuration breaks.

Agent Investigation

Explored the full proxy architecture:

  • crates/openshell-sandbox/src/proxy.rs — main connection handler, Branch A/B/C dispatch
  • crates/openshell-sandbox/src/l7/mod.rsTlsMode enum, parse_l7_config(), validation
  • crates/openshell-sandbox/src/l7/tls.rs — MITM implementation, cert cache, CA generation
  • crates/openshell-sandbox/src/l7/relay.rs — L7 inspection relay
  • crates/openshell-sandbox/src/l7/rest.rs — HTTP parsing, looks_like_http(), credential rewriting
  • crates/openshell-sandbox/src/secrets.rsSecretResolver placeholder mechanism
  • crates/openshell-sandbox/src/opa.rs — L4/L7 OPA evaluation
  • crates/openshell-sandbox/data/sandbox-policy.rego — Rego rules

Confirmed that the CA trust injection (write_ca_files()) already happens at sandbox startup regardless of policy, so the trust cost is already paid. The SecretResolver already works on any plaintext HTTP path — the only missing piece is ensuring TLS is always terminated so the proxy can see plaintext.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions