diff --git a/CHANGELOG.md b/CHANGELOG.md index 2747a92..6bb9674 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- DPoP `htu` validation against Authorization Servers on non-default ports. The + SSRF-safe pinned HTTP client now keeps a non-default port in the `Host` header, + and DPoP proof generation normalizes the `htu` claim to match: an explicit + default port (`:80`/`:443`) is dropped, non-default ports are preserved, IPv6 + literals stay bracketed, and any userinfo is stripped from `htu` + (RFC 9110 §7.2, RFC 9449 §4.3). + ## [0.1.0] - 2026-05-17 - Initial release. diff --git a/core/authplane/dpop.go b/core/authplane/dpop.go index b56c7eb..3eeccbb 100644 --- a/core/authplane/dpop.go +++ b/core/authplane/dpop.go @@ -11,6 +11,7 @@ import ( "encoding/json" "fmt" "net/url" + "strings" "time" "github.com/go-jose/go-jose/v4" @@ -140,7 +141,7 @@ func NewDPoPSignerWithKey(key stdcrypto.Signer, alg jose.SignatureAlgorithm) (*D // GenerateProof generates a DPoP proof JWT for the given HTTP method and URL. // Per RFC 9449 §4.3, the htu claim is set to the target URI excluding query and fragment. func (d *DPoPSigner) GenerateProof(method, rawURL string, opts *DPoPProofOptions) (string, error) { - htu := stripQueryFragment(rawURL) + htu := normalizeHTU(rawURL) now := time.Now() claims := map[string]any{ "jti": generateJTI(), @@ -178,15 +179,32 @@ func (d *DPoPSigner) Thumbprint() string { return d.jkt } -// stripQueryFragment removes query and fragment from a URL per RFC 9449 §4.3. -func stripQueryFragment(rawURL string) string { +// normalizeHTU derives the DPoP htu claim from a target URL. Per RFC 9449 §4.3 +// the htu is the target URI without query and fragment; the target URI carries +// no userinfo (RFC 9110 §7.1), so credentials are stripped rather than leaked +// into the signed proof. The authority is also normalized to mirror the outbound +// Host header (RFC 9110 §7.2): an explicit default port (:80 for http, :443 for +// https) is dropped while non-default ports are kept. Without this, an htu +// carrying an explicit default port would not match the host the AS reconstructs +// from a port-less Host header. IPv6 literals stay bracketed. +func normalizeHTU(rawURL string) string { u, err := url.Parse(rawURL) if err != nil { return rawURL } + u.User = nil u.RawQuery = "" u.Fragment = "" u.RawFragment = "" + if port := u.Port(); port != "" { + if (u.Scheme == "http" && port == "80") || (u.Scheme == "https" && port == "443") { + host := u.Hostname() + if strings.Contains(host, ":") { + host = "[" + host + "]" + } + u.Host = host + } + } return u.String() } diff --git a/core/authplane/dpop_test.go b/core/authplane/dpop_test.go index 2fc2cd4..f90ebb1 100644 --- a/core/authplane/dpop_test.go +++ b/core/authplane/dpop_test.go @@ -216,6 +216,78 @@ func TestGenerateProof_HTUStripsQueryAndFragment(t *testing.T) { } } +// htu must mirror the outbound Host header's default-port rule (RFC 9110 §7.2): +// an explicit :80/:443 is dropped, non-default ports are kept, and IPv6 literals +// stay bracketed. Otherwise an AS reconstructing the request URI from a port-less +// Host header would see a host mismatch against an htu carrying an explicit +// default port. See core/internal/ssrf hostHeader and verifier/dpop validateHTU. +func TestGenerateProof_HTUNormalizesDefaultPort(t *testing.T) { + signer, _ := NewDPoPSigner(jose.ES256) + + tests := []struct { + name string + url string + wantHTU string + }{ + {"explicit https default port stripped", "https://auth.example.com:443/token", "https://auth.example.com/token"}, + {"explicit http default port stripped", "http://auth.example.com:80/token", "http://auth.example.com/token"}, + {"https non-default port preserved", "https://auth.example.com:9000/token", "https://auth.example.com:9000/token"}, + {"http non-default port preserved", "http://localhost:9000/token", "http://localhost:9000/token"}, + {"ipv6 default port stripped, brackets kept", "https://[::1]:443/token", "https://[::1]/token"}, + {"ipv6 non-default port preserved", "http://[::1]:9000/token", "http://[::1]:9000/token"}, + {"no port unchanged", "https://auth.example.com/token", "https://auth.example.com/token"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proof, err := signer.GenerateProof("POST", tt.url, nil) + if err != nil { + t.Fatalf("GenerateProof: %v", err) + } + claims, err := testutil.ParseSignedToken(proof) + if err != nil { + t.Fatalf("parse: %v", err) + } + if claims["htu"] != tt.wantHTU { + t.Errorf("htu = %v, want %v", claims["htu"], tt.wantHTU) + } + }) + } +} + +// Per RFC 9449 §4.3 the htu is the target URI, which carries no userinfo +// (RFC 9110 §7.1). Credentials must never leak into the signed proof, and an AS +// reconstructs the request URI without userinfo, so it must be stripped. +func TestGenerateProof_HTUStripsUserinfo(t *testing.T) { + signer, _ := NewDPoPSigner(jose.ES256) + + tests := []struct { + name string + url string + wantHTU string + }{ + {"user and password stripped", "https://user:pass@auth.example.com/token", "https://auth.example.com/token"}, + {"user only stripped", "https://user@auth.example.com/token", "https://auth.example.com/token"}, + {"userinfo with non-default port", "https://user:pass@auth.example.com:9000/token", "https://auth.example.com:9000/token"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proof, err := signer.GenerateProof("POST", tt.url, nil) + if err != nil { + t.Fatalf("GenerateProof: %v", err) + } + claims, err := testutil.ParseSignedToken(proof) + if err != nil { + t.Fatalf("parse: %v", err) + } + if claims["htu"] != tt.wantHTU { + t.Errorf("htu = %v, want %v", claims["htu"], tt.wantHTU) + } + }) + } +} + func TestNewDPoPSignerWithKey_ES256(t *testing.T) { key, err := testutil.GenerateES256Key() if err != nil { diff --git a/core/internal/ssrf/ssrf.go b/core/internal/ssrf/ssrf.go index 17ff5c6..7fe9c6c 100644 --- a/core/internal/ssrf/ssrf.go +++ b/core/internal/ssrf/ssrf.go @@ -291,7 +291,7 @@ func pinnedGet(ctx context.Context, v *ValidatedURL, settings FetchSettings, _ * if err != nil { return nil, fmt.Errorf("%w: create request: %v", ErrSSRFBlocked, err) } - req.Host = v.Host + req.Host = hostHeader(*v) pinnedClient := pinnedHTTPClient(settings, v.Host) resp, err := pinnedClient.Do(req) if err != nil { @@ -315,7 +315,7 @@ func pinnedPost(ctx context.Context, v *ValidatedURL, settings FetchSettings, _ if err != nil { return nil, fmt.Errorf("%w: create request: %v", ErrSSRFBlocked, err) } - req.Host = v.Host + req.Host = hostHeader(*v) if contentType != "" { req.Header.Set("Content-Type", contentType) } @@ -440,6 +440,28 @@ func readLimitedResponse(resp *http.Response, maxSize int64) (*HTTPResponse, err }, nil } +// hostHeader returns the value for the outbound HTTP Host header. +// +// Per RFC 9110 §7.2 the Host header must mirror the URI authority, including +// the port when it is not the scheme's default. This is required for RFC 9449 +// DPoP htu validation: the Authorization Server reconstructs the request URI +// from the Host header and compares it against the proof's htu, so dropping a +// non-default port (e.g. :9000) breaks the match. IPv6 literals are bracketed +// per RFC 3986 §3.2.2. +func hostHeader(v ValidatedURL) string { + defaultPort := 80 + if v.Scheme == "https" { + defaultPort = 443 + } + if v.Port == defaultPort { + if strings.Contains(v.Host, ":") { + return "[" + v.Host + "]" + } + return v.Host + } + return net.JoinHostPort(v.Host, strconv.Itoa(v.Port)) +} + func formatIPForURL(ip net.IP) string { if ip.To4() != nil { return ip.String() diff --git a/core/internal/ssrf/ssrf_test.go b/core/internal/ssrf/ssrf_test.go index 97fc896..3a800af 100644 --- a/core/internal/ssrf/ssrf_test.go +++ b/core/internal/ssrf/ssrf_test.go @@ -399,6 +399,74 @@ func TestFormatIPForURL(t *testing.T) { } } +func TestHostHeader(t *testing.T) { + cases := []struct { + name string + v ValidatedURL + want string + }{ + {"https default port omitted", ValidatedURL{Scheme: "https", Host: "auth.example.com", Port: 443}, "auth.example.com"}, + {"http default port omitted", ValidatedURL{Scheme: "http", Host: "example.com", Port: 80}, "example.com"}, + {"https non-default port included", ValidatedURL{Scheme: "https", Host: "auth.example.com", Port: 9000}, "auth.example.com:9000"}, + {"http non-default port included", ValidatedURL{Scheme: "http", Host: "localhost", Port: 9000}, "localhost:9000"}, + {"ipv6 literal bracketed with port", ValidatedURL{Scheme: "http", Host: "::1", Port: 9000}, "[::1]:9000"}, + {"ipv6 literal bracketed default port", ValidatedURL{Scheme: "https", Host: "::1", Port: 443}, "[::1]"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := hostHeader(tc.v); got != tc.want { + t.Errorf("hostHeader() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestSSRFSafeGet_PinnedMode_HostHeaderIncludesPort is the regression test for +// the pinned Host header: it must carry the non-default port so an Authorization +// Server reconstructing the request URI (RFC 9110 §7.2) sees the same authority +// as the DPoP proof's htu (RFC 9449). httptest serves on 127.0.0.1:, +// which is never 80/443. +func TestSSRFSafeGet_PinnedMode_HostHeaderIncludesPort(t *testing.T) { + var receivedHost string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHost = r.Host + w.Write([]byte(`{}`)) + })) + defer server.Close() + wantHost := strings.TrimPrefix(server.URL, "http://") + settings := DevModeFetchSettings() + if _, err := SSRFSafeGet(context.Background(), server.URL+"/.well-known/jwks.json", settings, nil, MaxJWKSSize); err != nil { + t.Fatalf("pinnedGet should succeed: %v", err) + } + if receivedHost != wantHost { + t.Errorf("Host header = %q, want %q (port must be preserved for DPoP htu)", receivedHost, wantHost) + } +} + +// TestSSRFSafePost_PinnedMode_HostHeaderIncludesPort covers the DPoP-relevant +// path: a token-endpoint POST to an AS on a non-default port must send the port +// in the Host header. +func TestSSRFSafePost_PinnedMode_HostHeaderIncludesPort(t *testing.T) { + var receivedHost string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHost = r.Host + w.Write([]byte(`{}`)) + })) + defer server.Close() + wantHost := strings.TrimPrefix(server.URL, "http://") + settings := DevModeFetchSettings() + opts := PostOptions{ + FormData: map[string][]string{"grant_type": {"client_credentials"}}, + MaxSize: MaxMetadataSize, + } + if _, err := SSRFSafePost(context.Background(), server.URL+"/oauth/token", settings, nil, opts); err != nil { + t.Fatalf("pinnedPost should succeed: %v", err) + } + if receivedHost != wantHost { + t.Errorf("Host header = %q, want %q (port must be preserved for DPoP htu)", receivedHost, wantHost) + } +} + func TestParseJSONResponse_Valid(t *testing.T) { resp := &HTTPResponse{Body: []byte(`{"issuer":"https://auth.example.com"}`)} var result map[string]any @@ -622,3 +690,222 @@ func TestSSRFSafeGet_DevModeAllowsLocalhost(t *testing.T) { t.Errorf("expected 200, got %d", resp.Status) } } + +// newIPv6LoopbackServer starts an httptest server bound to the IPv6 loopback +// ([::1]) so the pinned-mode Host-header path is exercised with a bracketed +// IPv6 authority (RFC 3986 §3.2.2). Skips if IPv6 loopback is unavailable. +func newIPv6LoopbackServer(t *testing.T, handler http.Handler) *httptest.Server { + t.Helper() + var lc net.ListenConfig + l, err := lc.Listen(context.Background(), "tcp", "[::1]:0") + if err != nil { + t.Skipf("IPv6 loopback unavailable: %v", err) + } + srv := httptest.NewUnstartedServer(handler) + _ = srv.Listener.Close() + srv.Listener = l + srv.Start() + return srv +} + +// TestSSRFSafeGet_PinnedMode_IPv6HostHeaderBracketed is the IPv6 companion to +// the Host-header regression test: an IPv6 literal must reach the wire bracketed +// and with its non-default port, e.g. Host: [::1]:. +func TestSSRFSafeGet_PinnedMode_IPv6HostHeaderBracketed(t *testing.T) { + var receivedHost string + server := newIPv6LoopbackServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHost = r.Host + w.Write([]byte(`{}`)) + })) + defer server.Close() + wantHost := strings.TrimPrefix(server.URL, "http://") + settings := DevModeFetchSettings() + if _, err := SSRFSafeGet(context.Background(), server.URL+"/.well-known/jwks.json", settings, nil, MaxJWKSSize); err != nil { + t.Fatalf("pinnedGet should succeed: %v", err) + } + if receivedHost != wantHost { + t.Errorf("Host header = %q, want %q (IPv6 literal must be bracketed with port)", receivedHost, wantHost) + } +} + +func TestSSRFSafePost_PinnedMode_IPv6HostHeaderBracketed(t *testing.T) { + var receivedHost string + server := newIPv6LoopbackServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHost = r.Host + w.Write([]byte(`{}`)) + })) + defer server.Close() + wantHost := strings.TrimPrefix(server.URL, "http://") + settings := DevModeFetchSettings() + // MaxSize left zero to exercise the default-size branch in pinnedPost. + opts := PostOptions{FormData: map[string][]string{"grant_type": {"client_credentials"}}} + if _, err := SSRFSafePost(context.Background(), server.URL+"/oauth/token", settings, nil, opts); err != nil { + t.Fatalf("pinnedPost should succeed: %v", err) + } + if receivedHost != wantHost { + t.Errorf("Host header = %q, want %q (IPv6 literal must be bracketed with port)", receivedHost, wantHost) + } +} + +// TestSSRFSafeGet_UnprotectedMode covers the unsafeGet path (SSRFProtection +// disabled), including the nil-client default-client construction. +func TestSSRFSafeGet_UnprotectedMode(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"ok":true}`) + })) + defer server.Close() + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + resp, err := SSRFSafeGet(context.Background(), server.URL, settings, nil, MaxMetadataSize) + if err != nil { + t.Fatalf("unprotected GET should succeed: %v", err) + } + if resp.Status != 200 { + t.Errorf("expected 200, got %d", resp.Status) + } +} + +// TestSSRFSafePost_UnprotectedMode covers the unsafePost path, the form-encoding +// branch, extra headers, and the default-size branch (MaxSize left zero). +func TestSSRFSafePost_UnprotectedMode(t *testing.T) { + var gotCT, gotXHdr string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotCT = r.Header.Get("Content-Type") + gotXHdr = r.Header.Get("X-Test") + fmt.Fprint(w, `{}`) + })) + defer server.Close() + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + opts := PostOptions{ + FormData: map[string][]string{"grant_type": {"client_credentials"}}, + ExtraHeaders: map[string]string{"X-Test": "v"}, + } + if _, err := SSRFSafePost(context.Background(), server.URL+"/oauth/token", settings, nil, opts); err != nil { + t.Fatalf("unprotected POST should succeed: %v", err) + } + if gotCT != "application/x-www-form-urlencoded" { + t.Errorf("Content-Type = %q, want form-urlencoded", gotCT) + } + if gotXHdr != "v" { + t.Errorf("X-Test = %q, want %q", gotXHdr, "v") + } +} + +// TestReadLimitedResponse_ContentLengthTooLarge hits the early Content-Length +// guard in readLimitedResponse. +func TestReadLimitedResponse_ContentLengthTooLarge(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(make([]byte, 200)) // Content-Length set by the server + })) + defer server.Close() + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + _, err := SSRFSafeGet(context.Background(), server.URL, settings, nil, 50) + if err == nil { + t.Fatal("expected error for response exceeding Content-Length limit") + } +} + +// TestReadLimitedResponse_BodyExceedsWithoutContentLength forces a chunked +// response (no Content-Length) so the post-read body-size guard fires. +func TestReadLimitedResponse_BodyExceedsWithoutContentLength(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fl, ok := w.(http.Flusher) + if !ok { + t.Fatal("ResponseWriter is not a Flusher") + } + w.Write(make([]byte, 40)) + fl.Flush() // forces chunked transfer; ContentLength becomes -1 + w.Write(make([]byte, 40)) + })) + defer server.Close() + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + _, err := SSRFSafeGet(context.Background(), server.URL, settings, nil, 50) + if err == nil { + t.Fatal("expected error for body exceeding limit without Content-Length") + } +} + +func TestValidateURL_InvalidURL(t *testing.T) { + _, err := ValidateURL(context.Background(), "http://exa\x7fmple.com", DevModeFetchSettings()) + if err == nil { + t.Fatal("expected parse error for malformed URL") + } +} + +func TestValidateURL_DNSResolutionFails(t *testing.T) { + // .invalid never resolves (RFC 6761), exercising the DNS-failure branch. + _, err := ValidateURL(context.Background(), "https://nonexistent.invalid/path", DevModeFetchSettings()) + if err == nil { + t.Fatal("expected DNS resolution failure for .invalid host") + } +} + +func TestIsIPAllowed_MalformedIP(t *testing.T) { + // A net.IP of invalid length yields nil from To16(). + if IsIPAllowed(net.IP{1, 2, 3}, DevModeFetchSettings()) { + t.Error("malformed IP must not be allowed") + } +} + +func TestExtractEmbeddedIPv4(t *testing.T) { + cases := []struct { + name string + in net.IP + want string // "" means nil expected + }{ + {"malformed length", net.IP{1, 2, 3}, ""}, + {"6to4 maps embedded v4", net.ParseIP("2002:0102:0304::1"), "1.2.3.4"}, + {"teredo blocked server returns server", net.ParseIP("2001:0000:a9fe:0001:0000:0000:0000:0000"), "169.254.0.1"}, + {"teredo all-global returns nil", net.ParseIP("2001:0000:0809:0a0b:0000:0000:0000:0000"), ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := extractEmbeddedIPv4(tc.in) + if tc.want == "" { + if got != nil { + t.Errorf("extractEmbeddedIPv4() = %v, want nil", got) + } + return + } + if got == nil || got.String() != tc.want { + t.Errorf("extractEmbeddedIPv4() = %v, want %s", got, tc.want) + } + }) + } +} + +func TestSSRFSafePost_PinnedMode_InvalidURL(t *testing.T) { + // SSRFProtection on → ValidateURL runs and rejects the malformed URL. + opts := PostOptions{FormData: map[string][]string{"a": {"b"}}} + if _, err := SSRFSafePost(context.Background(), "http://exa\x7fmple.com", DevModeFetchSettings(), nil, opts); err == nil { + t.Fatal("expected validation error for malformed URL in pinned POST") + } +} + +func TestSSRFSafeGet_UnprotectedMode_InvalidURL(t *testing.T) { + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + if _, err := SSRFSafeGet(context.Background(), "http://exa\x7fmple.com", settings, nil, MaxMetadataSize); err == nil { + t.Fatal("expected request-creation error for malformed URL in unsafe GET") + } +} + +func TestSSRFSafePost_UnprotectedMode_InvalidURL(t *testing.T) { + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + opts := PostOptions{FormData: map[string][]string{"a": {"b"}}} + if _, err := SSRFSafePost(context.Background(), "http://exa\x7fmple.com", settings, nil, opts); err == nil { + t.Fatal("expected request-creation error for malformed URL in unsafe POST") + } +} + +// TestSSRFSafePost_UnprotectedMode_RedirectBlocked exercises the CheckRedirect +// guard on the unsafe POST client: redirects must be refused. +func TestSSRFSafePost_UnprotectedMode_RedirectBlocked(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://example.com/", http.StatusFound) + })) + defer server.Close() + settings := FetchSettings{SSRFProtection: false, AllowHTTP: true} + opts := PostOptions{FormData: map[string][]string{"a": {"b"}}} + if _, err := SSRFSafePost(context.Background(), server.URL, settings, nil, opts); err == nil { + t.Fatal("expected redirect to be blocked in unsafe POST") + } +}