Skip to content

refactor(cli): remove global --tls-ca, --tls-cert, --tls-key flags#62

Merged
drew merged 1 commit intomainfrom
remove-tls-cli-flags/an
Mar 3, 2026
Merged

refactor(cli): remove global --tls-ca, --tls-cert, --tls-key flags#62
drew merged 1 commit intomainfrom
remove-tls-cli-flags/an

Conversation

@drew
Copy link
Collaborator

@drew drew commented Mar 3, 2026

Summary

  • Removed the three global CLI flags (--tls-ca, --tls-cert, --tls-key) from the Cli struct
  • TLS certificates are always resolved automatically from cluster metadata, making explicit CLI flags unnecessary
  • The TlsOptions struct, new() constructor, and all auto-resolution logic remain intact for programmatic and test use

TLS certificates are always resolved automatically from cluster metadata,
making the explicit CLI flags unnecessary. The TlsOptions struct and
auto-resolution logic remain intact for programmatic and test use.
@drew drew self-assigned this Mar 3, 2026
@drew drew merged commit 4e949ad into main Mar 3, 2026
6 checks passed
@drew drew deleted the remove-tls-cli-flags/an branch March 3, 2026 02:07
drew pushed a commit that referenced this pull request Mar 16, 2026
… (!37)

> **🔧 security-fix-agent**

Closes #62

## Security Fix

### Summary
The CONNECT proxy accepted hostnames from clients and connected to whatever IP they resolved to, with no validation against internal address ranges. While the OPA policy is default-deny, a misconfigured or overly permissive policy could allow SSRF to cloud metadata (169.254.169.254), localhost, or RFC1918 services. This fix adds DNS resolution before connecting and rejects any host that resolves to an internal IP.

### Severity Assessment
- **Impact:** Medium — if exploited, could reach cloud metadata (IAM creds), cluster-internal services, or host-local services
- **Exploitability:** Very low — requires OPA policy misconfiguration or DNS rebinding attack
- **Affected components:** `crates/navigator-sandbox/src/proxy.rs` — `handle_tcp_connection`

### Changes Made
- `crates/navigator-sandbox/src/proxy.rs`: Added `is_internal_ip()` helper that checks IPv4 loopback/private/link-local, IPv6 loopback/link-local, and IPv4-mapped IPv6. Added `resolve_and_reject_internal()` that resolves DNS and rejects internal IPs. Inserted check between OPA allow and `TcpStream::connect`, with control plane endpoints exempt.
- `architecture/security-policy.md`: Added SSRF Protection section with blocked ranges table and flow diagram
- `architecture/sandbox.md`: Updated proxy connection flow diagram and added SSRF protection subsection
- `architecture/README.md`: Added internal IP rejection step to proxy description

### Tests Added
- **Unit:** 17 tests in `proxy::tests` — covers IPv4 loopback/private/link-local, IPv6 loopback/link-local, IPv4-mapped IPv6, public IPs, DNS resolution of localhost/127.0.0.1/169.254.169.254, and DNS failure handling
- **Integration/E2E:** N/A — the proxy runs inside a Linux network namespace; unit tests for IP checking and DNS resolution cover the security boundary

### Documentation Updated
- `architecture/security-policy.md`: New SSRF Protection section with blocked IP ranges and Mermaid flowchart
- `architecture/sandbox.md`: Updated proxy flow diagram and added SSRF protection subsection
- `architecture/README.md`: Added step 4 to proxy description

### Verification
All 85 sandbox tests pass including 17 new proxy SSRF tests. Pre-commit (fmt, clippy, full test suite) passes clean with zero warnings.
drew added a commit that referenced this pull request Mar 16, 2026
TLS certificates are always resolved automatically from cluster metadata,
making the explicit CLI flags unnecessary. The TlsOptions struct and
auto-resolution logic remain intact for programmatic and test use.
drew pushed a commit that referenced this pull request Mar 16, 2026
… (!37)

> **🔧 security-fix-agent**

Closes #62

## Security Fix

### Summary
The CONNECT proxy accepted hostnames from clients and connected to whatever IP they resolved to, with no validation against internal address ranges. While the OPA policy is default-deny, a misconfigured or overly permissive policy could allow SSRF to cloud metadata (169.254.169.254), localhost, or RFC1918 services. This fix adds DNS resolution before connecting and rejects any host that resolves to an internal IP.

### Severity Assessment
- **Impact:** Medium — if exploited, could reach cloud metadata (IAM creds), cluster-internal services, or host-local services
- **Exploitability:** Very low — requires OPA policy misconfiguration or DNS rebinding attack
- **Affected components:** `crates/navigator-sandbox/src/proxy.rs` — `handle_tcp_connection`

### Changes Made
- `crates/navigator-sandbox/src/proxy.rs`: Added `is_internal_ip()` helper that checks IPv4 loopback/private/link-local, IPv6 loopback/link-local, and IPv4-mapped IPv6. Added `resolve_and_reject_internal()` that resolves DNS and rejects internal IPs. Inserted check between OPA allow and `TcpStream::connect`, with control plane endpoints exempt.
- `architecture/security-policy.md`: Added SSRF Protection section with blocked ranges table and flow diagram
- `architecture/sandbox.md`: Updated proxy connection flow diagram and added SSRF protection subsection
- `architecture/README.md`: Added internal IP rejection step to proxy description

### Tests Added
- **Unit:** 17 tests in `proxy::tests` — covers IPv4 loopback/private/link-local, IPv6 loopback/link-local, IPv4-mapped IPv6, public IPs, DNS resolution of localhost/127.0.0.1/169.254.169.254, and DNS failure handling
- **Integration/E2E:** N/A — the proxy runs inside a Linux network namespace; unit tests for IP checking and DNS resolution cover the security boundary

### Documentation Updated
- `architecture/security-policy.md`: New SSRF Protection section with blocked IP ranges and Mermaid flowchart
- `architecture/sandbox.md`: Updated proxy flow diagram and added SSRF protection subsection
- `architecture/README.md`: Added step 4 to proxy description

### Verification
All 85 sandbox tests pass including 17 new proxy SSRF tests. Pre-commit (fmt, clippy, full test suite) passes clean with zero warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant