Skip to content

Commit 4627b56

Browse files
authored
fix(ssrf): refuse outbound requests through HTTP(S)_PROXY (L-6)
The SSRF dial guard validates the address the transport dials. When a proxy is configured, the transport dials the proxy and sends the real target inside the CONNECT/request envelope, so the guard only validated the proxy and internal/rebound targets could slip through. - Wrap tr.Proxy in ssrfGuardedTransport so that any request whose proxy resolution returns a proxy URL is refused with a clear error. - Log a one-time warning when proxy env vars are detected. - Add TestSSRFGuardedTransport_RefusesProxy regression test. - Document the proxy refusal in SECURITY.md and AGENTS.md. Closes L-6 in sec_findings.md.
1 parent fe4fbcf commit 4627b56

4 files changed

Lines changed: 73 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
116116
- **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. `rm -rf ./` / `rm -rf ./..` are normalised to `.` / `..` for wipe-target matching, and `${VAR:--rf}` default-value flag substitutions are treated as fail-closed. Regression suite in `classifier_bypass_test.go`.
117117
- **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.<token>` subprotocol. The token is no longer embedded in every `GET /` response; it is only delivered when the request includes the correct `?token=` query parameter (Jupyter-style), and a non-loopback bind prints a loud warning. The localhost origin check remains as defense-in-depth.
118118
- **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go` + `internal/danger/classifier.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs (loopback, RFC1918, RFC4193, link-local, RFC 6598 CGNAT `100.64.0.0/10`, RFC 2544 benchmark `198.18.0.0/15`, and unspecified), then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable.
119+
- **SSRF guard proxy refusal** (`cmd/odek/ssrf_guard.go`) — when `HTTP(S)_PROXY` is set, the transport would dial the proxy instead of the target and the dial guard would only validate the proxy address. `ssrfGuardedTransport` detects an active proxy, logs a warning, and refuses the request so SSRF protection is not silently voided.
119120
- **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking. Every `/api/*` endpoint additionally requires the per-instance `odek_ws_token` (cookie or `X-Odek-Ws-Token` header) and rejects non-loopback `Host` headers, closing DNS-rebinding reads of `/api/sessions`, `/api/resources`, and `/api/models`.
120121
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
121122
- **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons.

cmd/odek/ssrf_guard.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@ import (
55
"fmt"
66
"net"
77
"net/http"
8+
"net/url"
9+
"os"
10+
"sync"
811
"time"
912

1013
"github.com/BackendStack21/odek/internal/danger"
1114
)
1215

16+
// proxyWarnOnce ensures the proxy-disabled warning is logged only once per
17+
// process even though ssrfGuardedTransport is called for several tools.
18+
var proxyWarnOnce sync.Once
19+
1320
// SSRF / DNS-rebinding dial guard.
1421
//
1522
// danger.ClassifyURL inspects only the literal hostname of a URL, so a domain
@@ -116,6 +123,42 @@ func ssrfGuardedTransport(allowedHosts ...string) *http.Transport {
116123
} else {
117124
tr = &http.Transport{Proxy: http.ProxyFromEnvironment}
118125
}
126+
127+
// Proxies bypass the dial-layer SSRF guard: the transport dials the proxy,
128+
// and the target address is sent inside the CONNECT/request envelope. An
129+
// attacker-controlled proxy (or a rebinding target reachable through a
130+
// legitimate proxy) would therefore defeat the guard. Fail closed: if a proxy
131+
// is configured for a request, refuse it rather than silently void protection.
132+
if tr.Proxy != nil {
133+
if proxyEnvSet() {
134+
proxyWarnOnce.Do(func() {
135+
fmt.Fprintf(os.Stderr, "warning: HTTP(S)_PROXY is set but odek's SSRF guard cannot validate target addresses through a proxy; proxy routing is disabled for outbound tool traffic\n")
136+
})
137+
}
138+
originalProxy := tr.Proxy
139+
tr.Proxy = func(req *http.Request) (*url.URL, error) {
140+
proxyURL, err := originalProxy(req)
141+
if err != nil {
142+
return nil, err
143+
}
144+
if proxyURL != nil {
145+
return nil, fmt.Errorf("refusing request through HTTP(S)_PROXY: SSRF guard cannot validate target address via a proxy")
146+
}
147+
return nil, nil
148+
}
149+
}
150+
119151
tr.DialContext = ssrfGuardedDial(base.DialContext, net.DefaultResolver.LookupIPAddr, allowedHosts...)
120152
return tr
121153
}
154+
155+
// proxyEnvSet reports whether any of the standard HTTP(S)_PROXY environment
156+
// variables are set, so callers can warn that outbound proxying is disabled.
157+
func proxyEnvSet() bool {
158+
for _, v := range []string{"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy", "NO_PROXY", "no_proxy"} {
159+
if os.Getenv(v) != "" {
160+
return true
161+
}
162+
}
163+
return false
164+
}

cmd/odek/ssrf_guard_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"net"
66
"net/http"
7+
"net/url"
78
"strings"
89
"testing"
910

@@ -225,6 +226,32 @@ func TestWebSearch_SSRF_ResolvesInternal(t *testing.T) {
225226

226227
// TestSSRFGuardedTransport_Installed is a guard against regressions that would
227228
// silently drop the SSRF protection from the production constructors.
229+
func TestSSRFGuardedTransport_RefusesProxy(t *testing.T) {
230+
// Replace the default transport with one whose Proxy function always returns
231+
// a proxy URL. This avoids depending on the test environment's proxy env
232+
// vars and proves the guard refuses proxy-routed requests regardless of how
233+
// the proxy was configured.
234+
orig := http.DefaultTransport
235+
http.DefaultTransport = &http.Transport{
236+
Proxy: http.ProxyURL(&url.URL{Scheme: "http", Host: "proxy.example.com:8080"}),
237+
}
238+
defer func() { http.DefaultTransport = orig }()
239+
240+
tr := ssrfGuardedTransport()
241+
client := &http.Client{Transport: tr}
242+
243+
resp, err := client.Get("http://example.com/")
244+
if err == nil {
245+
if resp != nil {
246+
resp.Body.Close()
247+
}
248+
t.Fatal("expected request to be refused when a proxy is configured")
249+
}
250+
if !strings.Contains(err.Error(), "refusing request through HTTP(S)_PROXY") {
251+
t.Errorf("error %q should mention proxy refusal", err)
252+
}
253+
}
254+
228255
func TestSSRFGuardedTransport_Installed(t *testing.T) {
229256
b := newBrowserTool(danger.DangerousConfig{})
230257
if b.client.Transport == nil {

docs/SECURITY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,8 @@ client := &http.Client{
394394

395395
There is no user-facing allowlist config field today; the list is derived from each tool's own operator-controlled `base_url`. If you need a broader or user-editable allowlist, add a `dangerous.ssrf_allowed_hosts` (or `network.allowed_hosts`) array to the config and merge it into the set passed to `ssrfGuardedTransport`.
396396

397+
When `HTTP(S)_PROXY` is set, the transport would dial the proxy address instead of the target, so the dial-time guard would validate only the proxy and the real target could be an internal/rebound address. `ssrfGuardedTransport` detects an active proxy and refuses the request with a clear error rather than silently disabling SSRF protection. Outbound tool traffic therefore requires direct connections.
398+
397399
### 18b. CGNAT and benchmark IP blocking
398400

399401
Go's `net.IP.IsPrivate()` covers RFC1918 and RFC4193 private ranges, but it does not cover RFC 6598 CGNAT (`100.64.0.0/10`) or the RFC 2544 benchmark-testing range (`198.18.0.0/15`). Tailscale and similar overlay networks use `100.64/10` addresses, so an attacker who could steer the agent to `http://100.x.x.x` (or a hostname that rebinding resolves to such an address) could reach unauthenticated internal services that the operator expected to be unreachable from the agent.

0 commit comments

Comments
 (0)