diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 9120d496a..b0d63017e 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -2785,8 +2785,11 @@ a new API state machine, queue contract, or verification-accounting field. to block unauthenticated SSRF against internal hosts. That same probe path must also validate user-supplied addresses before probing, reject metadata, link-local, multicast, and unspecified destinations, and pin each outbound - dial to the first permitted resolved IP so DNS rebinding cannot swap the - target between validation and connect time. That same `/api/connections` + dial to the permitted IPs from the validating resolution so DNS rebinding + cannot swap the target between validation and connect time. Within that + pinned set the dialer tries each permitted IP in resolution order, so a + host whose first address family is unreachable (e.g. `localhost` resolving + to `::1` while the service listens only on `127.0.0.1`) still connects. That same `/api/connections` payload now also owns the additive `systems[]` grouping contract for the infrastructure settings source manager. Those grouped rows must stay source-oriented and backend-authored: one primary source row may carry diff --git a/pkg/securityutil/outbound_http.go b/pkg/securityutil/outbound_http.go index 42c30f6e9..9000d206b 100644 --- a/pkg/securityutil/outbound_http.go +++ b/pkg/securityutil/outbound_http.go @@ -72,7 +72,12 @@ func resolveOutboundIPAddrs(ctx context.Context, host string, opts RestrictedOut return resolveOutboundFetchIPs(ctx, host) } -func resolvePermittedOutboundIP(ctx context.Context, host string, opts RestrictedOutboundHTTPOptions) (net.IP, error) { +// resolvePermittedOutboundIPs resolves host and returns every permitted IP in +// resolution order. Callers that dial must try each returned IP: a host like +// "localhost" can resolve to ::1 first while the service listens only on +// 127.0.0.1, so pinning the first permitted IP alone turns an address-family +// mismatch into a hard connection failure. +func resolvePermittedOutboundIPs(ctx context.Context, host string, opts RestrictedOutboundHTTPOptions) ([]net.IP, error) { host = strings.TrimSpace(host) if host == "" { return nil, fmt.Errorf("URL hostname is required") @@ -87,7 +92,7 @@ func resolvePermittedOutboundIP(ctx context.Context, host string, opts Restricte if err := validateOutboundIP(ip, opts); err != nil { return nil, err } - return ip, nil + return []net.IP{ip}, nil } baseCtx := ctx @@ -105,13 +110,17 @@ func resolvePermittedOutboundIP(ctx context.Context, host string, opts Restricte return nil, fmt.Errorf("hostname %s did not resolve", host) } + var permitted []net.IP var blockedErr error for _, addr := range addrs { if err := validateOutboundIP(addr.IP, opts); err != nil { blockedErr = err continue } - return addr.IP, nil + permitted = append(permitted, addr.IP) + } + if len(permitted) > 0 { + return permitted, nil } if blockedErr != nil { @@ -120,6 +129,14 @@ func resolvePermittedOutboundIP(ctx context.Context, host string, opts Restricte return nil, fmt.Errorf("hostname %s did not resolve", host) } +func resolvePermittedOutboundIP(ctx context.Context, host string, opts RestrictedOutboundHTTPOptions) (net.IP, error) { + ips, err := resolvePermittedOutboundIPs(ctx, host, opts) + if err != nil { + return nil, err + } + return ips[0], nil +} + // ValidateOutboundFetchURL validates a fully-qualified HTTP(S) URL against the restricted outbound policy. func ValidateOutboundFetchURL(ctx context.Context, raw string, opts RestrictedOutboundHTTPOptions) (*url.URL, error) { parsed, err := NormalizeAbsoluteHTTPURL(raw) @@ -233,7 +250,7 @@ func (r *restrictedRoundTripper) RoundTrip(req *http.Request) (*http.Response, e } // NewRestrictedOutboundHTTPClient returns an HTTP client that validates redirects and pins direct outbound dials -// to the first permitted resolved IP for the requested host. +// to the permitted resolved IPs for the requested host, trying each in resolution order until one connects. func NewRestrictedOutboundHTTPClient(timeout time.Duration, opts RestrictedOutboundHTTPOptions) *http.Client { transport := cloneRestrictedTransport(opts.TLSConfig) transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { @@ -242,13 +259,30 @@ func NewRestrictedOutboundHTTPClient(timeout time.Duration, opts RestrictedOutbo return nil, fmt.Errorf("parse outbound address %q: %w", addr, err) } - permittedIP, err := resolvePermittedOutboundIP(ctx, host, opts) + permittedIPs, err := resolvePermittedOutboundIPs(ctx, host, opts) if err != nil { return nil, err } + // Try every permitted IP from the validating resolution, not just the + // first: "localhost" commonly resolves to ::1 ahead of 127.0.0.1, and a + // service bound only to one loopback family (e.g. Ollama on 127.0.0.1) + // would otherwise be unreachable even though curl and browsers connect + // fine. Each candidate was validated above, so rebinding protection is + // unchanged. dialer := net.Dialer{Timeout: 10 * time.Second} - return dialer.DialContext(ctx, network, net.JoinHostPort(permittedIP.String(), port)) + var dialErr error + for _, permittedIP := range permittedIPs { + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(permittedIP.String(), port)) + if err == nil { + return conn, nil + } + dialErr = err + if ctx.Err() != nil { + break + } + } + return nil, dialErr } client := &http.Client{ diff --git a/pkg/securityutil/outbound_http_test.go b/pkg/securityutil/outbound_http_test.go index 7d794e56a..b826921f4 100644 --- a/pkg/securityutil/outbound_http_test.go +++ b/pkg/securityutil/outbound_http_test.go @@ -2,7 +2,10 @@ package securityutil import ( "context" + "io" + "net" "net/http" + "net/http/httptest" "strings" "testing" ) @@ -31,6 +34,53 @@ func TestRestrictedOutboundHTTPClient_BlocksMetadataServiceEvenViaProxy(t *testi } } +// A host like "localhost" can resolve to ::1 ahead of 127.0.0.1 while the +// target service (e.g. Ollama's default bind) listens only on 127.0.0.1. The +// restricted dialer must fall through to later permitted IPs instead of +// failing on the first refused address family. +func TestRestrictedOutboundHTTPClient_TriesAllPermittedResolvedIPs(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, "ok") + })) + defer srv.Close() + + serverHost, port, err := net.SplitHostPort(srv.Listener.Addr().String()) + if err != nil { + t.Fatalf("failed to parse test server address: %v", err) + } + if serverHost != "127.0.0.1" { + t.Skipf("test server bound to %s, need an IPv4 loopback listener", serverHost) + } + + client := NewRestrictedOutboundHTTPClient(0, RestrictedOutboundHTTPOptions{ + AllowedSchemes: []string{"http", "https"}, + AllowPrivateIPs: true, + AllowLoopback: true, + ResolveIPAddrs: func(ctx context.Context, host string) ([]net.IPAddr, error) { + return []net.IPAddr{ + {IP: net.ParseIP("::1")}, + {IP: net.ParseIP("127.0.0.1")}, + }, nil + }, + }) + + resp, err := client.Get("http://ipv6-first.test:" + port + "/") + if err != nil { + t.Fatalf("expected fallback to the second permitted IP to succeed, got %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("failed to read response body: %v", err) + } + if string(body) != "ok" { + t.Fatalf("expected body %q, got %q", "ok", string(body)) + } +} + func TestRestrictedOutboundHTTPClient_BlocksLinkLocalEvenViaProxy(t *testing.T) { client := NewRestrictedOutboundHTTPClient(0, RestrictedOutboundHTTPOptions{ AllowedSchemes: []string{"http", "https"},