Skip to content

Centralise outbound URL validation in StrictHTTPClient #4246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JorisHeadease wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Centralise outbound URL validation in StrictHTTPClient #4246

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec898fc
refactor: centralise outbound URL validation in StrictHTTPClient
JorisHeadease May 11, 2026
3ae1549
test: capture and restore package globals in client tests
JorisHeadease May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ func (auth *Auth) Configure(config core.ServerConfig) error {
// auth.http.config got deprecated in favor of httpclient.timeout
auth.httpClientTimeout = config.HTTPClient.Timeout
}
auth.openID4VCIClient = openid4vci.NewClient(httpclient.NewWithCache(auth.httpClientTimeout), auth.strictMode)
auth.openID4VCIClient = openid4vci.NewClient(httpclient.NewWithCache(auth.httpClientTimeout))
// V1 API related stuff
accessTokenLifeSpan := time.Duration(auth.config.AccessTokenLifeSpan) * time.Second
auth.authzServer = oauth.NewAuthorizationServer(auth.vdrInstance.Resolver(), auth.vcr, auth.vcr.Verifier(), auth.serviceResolver,
auth.keyStore, auth.contractNotary, auth.jsonldManager, accessTokenLifeSpan)
auth.relyingParty = oauth.NewRelyingParty(auth.vdrInstance.Resolver(), auth.serviceResolver,
auth.keyStore, auth.vcr.Wallet(), auth.httpClientTimeout, auth.tlsConfig, config.Strictmode, auth.pkiProvider)
auth.keyStore, auth.vcr.Wallet(), auth.httpClientTimeout, auth.tlsConfig, auth.pkiProvider)

if err := auth.authzServer.Configure(auth.config.ClockSkew, config.Strictmode); err != nil {
return err
Expand Down
6 changes: 0 additions & 6 deletions auth/client/iam/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth
// ClientMetadata retrieves the client metadata from the client metadata endpoint given in the authorization request.
// We use the AuthorizationServerMetadata struct since it overlaps greatly with the client metadata.
func (hb HTTPClient) ClientMetadata(ctx context.Context, endpoint string) (*oauth.OAuthClientMetadata, error) {
_, err := core.ParsePublicURL(endpoint, hb.strictMode)
if err != nil {
return nil, err
}

// create a GET request
request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
if err != nil {
return nil, err
Expand Down
42 changes: 5 additions & 37 deletions auth/openid4vci/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,44 +73,18 @@ type Client interface {
}

// NewClient returns a Client backed by the provided HTTP request doer.
// In production callers should pass *httpclient.StrictHTTPClient so the
// shared transport policies apply (HTTPS-in-strict, body size limit,
// User-Agent header).
//
// When strictMode is true, target URLs are additionally validated via
// core.ParsePublicURL: HTTPS scheme, no IP hosts, no reserved hostnames.
func NewClient(httpClient core.HTTPRequestDoer, strictMode bool) Client {
return &client{httpClient: httpClient, strictMode: strictMode}
// Production callers should pass *httpclient.StrictHTTPClient so the shared
// transport policies apply: HTTPS-only in strict mode, no IP/reserved hosts
// in strict mode (via core.ParsePublicURL), body size limit, User-Agent.
func NewClient(httpClient core.HTTPRequestDoer) Client {
return &client{httpClient: httpClient}
}

type client struct {
httpClient core.HTTPRequestDoer
strictMode bool
}

// validateURL guards against SSRF by rejecting target URLs that fail
// core.ParsePublicURL (in strict mode: HTTPS only, no IP/reserved hosts).
// Called at the entry of every method that makes outbound HTTP.
//
// TODO: this validation belongs on httpclient.StrictHTTPClient so every
// outbound HTTP call (not just OpenID4VCI) gets the IP/reserved-host check,
// not only the HTTPS scheme check that StrictHTTPClient.Do enforces today.
// Placed here for now to preserve parity with master, where the equivalent
// caller (auth/client/iam.HTTPClient) validated via oauth.IssuerIdToWellKnown
// → core.ParsePublicURL before issuing the request, and to address a CodeQL
// SSRF finding on this PR. Tracked as a follow-up to consolidate the check
// in the shared HTTP client.
func (c *client) validateURL(name, target string) error {
if _, err := core.ParsePublicURL(target, c.strictMode); err != nil {
return fmt.Errorf("openid4vci: invalid %s URL: %w", name, err)
}
return nil
}

func (c *client) OpenIDCredentialIssuerMetadata(ctx context.Context, issuerURL string) (*OpenIDCredentialIssuerMetadata, error) {
if err := c.validateURL("issuer", issuerURL); err != nil {
return nil, err
}
// Per §12.2.1, the Credential Issuer Identifier MUST NOT contain query
// or fragment components.
if parsed, _ := url.Parse(issuerURL); parsed != nil && (parsed.RawQuery != "" || parsed.Fragment != "") {
Expand Down Expand Up @@ -145,9 +119,6 @@ func (c *client) OpenIDCredentialIssuerMetadata(ctx context.Context, issuerURL s
}

func (c *client) RequestNonce(ctx context.Context, nonceEndpoint string) (string, error) {
if err := c.validateURL("nonce endpoint", nonceEndpoint); err != nil {
return "", err
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, nonceEndpoint, http.NoBody)
if err != nil {
return "", err
Expand All @@ -171,9 +142,6 @@ func (c *client) RequestNonce(ctx context.Context, nonceEndpoint string) (string
}

func (c *client) RequestCredential(ctx context.Context, opts RequestCredentialOpts) (*CredentialResponse, error) {
if err := c.validateURL("credential endpoint", opts.CredentialEndpoint); err != nil {
return nil, err
}
body := CredentialRequest{
Proofs: &CredentialRequestProofs{
JWT: []string{opts.ProofJWT},
Expand Down
35 changes: 14 additions & 21 deletions auth/openid4vci/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestClient_RequestNonce(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
nonce, err := client.RequestNonce(context.Background(), srv.URL)
require.NoError(t, err)
assert.Equal(t, "test-nonce-123", nonce)
Expand All @@ -54,7 +54,7 @@ func TestClient_RequestNonce(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.RequestNonce(context.Background(), srv.URL)
require.Error(t, err)
assert.Contains(t, err.Error(), "500")
Expand All @@ -67,7 +67,7 @@ func TestClient_RequestNonce(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.RequestNonce(context.Background(), srv.URL)
require.Error(t, err)
assert.Contains(t, err.Error(), "empty c_nonce")
Expand All @@ -91,7 +91,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
metadata, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL)
require.NoError(t, err)
require.NotNil(t, metadata)
Expand All @@ -110,7 +110,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice")
require.NoError(t, err)
assert.Equal(t, "/.well-known/openid-credential-issuer/oauth2/alice", capturedPath)
Expand All @@ -126,7 +126,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/foo%2Fbar")
require.NoError(t, err)
assert.Equal(t, "/.well-known/openid-credential-issuer/foo%2Fbar", capturedRawPath)
Expand All @@ -141,7 +141,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL)
require.Error(t, err)
assert.Contains(t, err.Error(), "credential_issuer")
Expand All @@ -154,21 +154,14 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL)
require.Error(t, err)
assert.Contains(t, err.Error(), "404")
})

t.Run("rejects non-https issuer URL in strict mode", func(t *testing.T) {
client := NewClient(http.DefaultClient, true)
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), "http://issuer.example/")
require.Error(t, err)
assert.Contains(t, err.Error(), "invalid issuer URL")
})

t.Run("rejects issuer URL with query or fragment per §12.2.1", func(t *testing.T) {
client := NewClient(http.DefaultClient, false)
client := NewClient(http.DefaultClient)
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), "https://issuer.example/?foo=bar")
require.Error(t, err)
assert.Contains(t, err.Error(), "query and fragment")
Expand All @@ -185,7 +178,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL)
require.Error(t, err)
assert.Contains(t, err.Error(), "decoding issuer metadata")
Expand Down Expand Up @@ -216,7 +209,7 @@ func TestClient_RequestCredential(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
resp, err := client.RequestCredential(context.Background(), RequestCredentialOpts{
CredentialEndpoint: srv.URL,
AccessToken: "test-token",
Expand All @@ -240,7 +233,7 @@ func TestClient_RequestCredential(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.RequestCredential(context.Background(), RequestCredentialOpts{
CredentialEndpoint: srv.URL,
AccessToken: "t",
Expand All @@ -261,7 +254,7 @@ func TestClient_RequestCredential(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.RequestCredential(context.Background(), RequestCredentialOpts{
CredentialEndpoint: srv.URL,
AccessToken: "test-token",
Expand All @@ -279,7 +272,7 @@ func TestClient_RequestCredential(t *testing.T) {
}))
defer srv.Close()

client := NewClient(srv.Client(), false)
client := NewClient(srv.Client())
_, err := client.RequestCredential(context.Background(), RequestCredentialOpts{
CredentialEndpoint: srv.URL,
AccessToken: "test-token",
Expand Down
11 changes: 3 additions & 8 deletions auth/services/oauth/relying_party.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"fmt"
"github.com/nuts-foundation/nuts-node/pki"
"net/url"
"strings"
"time"

"github.com/lestrrat-go/jwx/v2/jwt"
Expand All @@ -47,24 +46,23 @@ type relyingParty struct {
keyResolver resolver.KeyResolver
privateKeyStore nutsCrypto.KeyStore
serviceResolver didman.CompoundServiceResolver
strictMode bool
httpClientTimeout time.Duration
httpClientTLS *tls.Config
wallet holder.Wallet
pkiValidator pki.Validator
}

// NewRelyingParty returns an implementation of RelyingParty
// NewRelyingParty returns an implementation of RelyingParty.
// Strict-mode URL validation is centralised in http/client.StrictHTTPClient; this constructor no longer takes a strictMode flag.
func NewRelyingParty(
didResolver resolver.DIDResolver, serviceResolver didman.CompoundServiceResolver, privateKeyStore nutsCrypto.KeyStore,
wallet holder.Wallet, httpClientTimeout time.Duration, httpClientTLS *tls.Config, strictMode bool, pkiValidator pki.Validator) RelyingParty {
wallet holder.Wallet, httpClientTimeout time.Duration, httpClientTLS *tls.Config, pkiValidator pki.Validator) RelyingParty {
return &relyingParty{
keyResolver: resolver.DIDKeyResolver{Resolver: didResolver},
serviceResolver: serviceResolver,
privateKeyStore: privateKeyStore,
httpClientTimeout: httpClientTimeout,
httpClientTLS: httpClientTLS,
strictMode: strictMode,
wallet: wallet,
pkiValidator: pkiValidator,
}
Expand Down Expand Up @@ -110,9 +108,6 @@ func (s *relyingParty) CreateJwtGrant(ctx context.Context, request services.Crea
}

func (s *relyingParty) RequestRFC003AccessToken(ctx context.Context, jwtGrantToken string, authorizationServerEndpoint url.URL) (*oauth.TokenResponse, error) {
if s.strictMode && strings.ToLower(authorizationServerEndpoint.Scheme) != "https" {
return nil, fmt.Errorf("authorization server endpoint must be HTTPS when in strict mode: %s", authorizationServerEndpoint.String())
}
httpClient := strictHttp.NewWithTLSConfig(s.httpClientTimeout, s.httpClientTLS)
authClient, err := client.NewHTTPClient("", s.httpClientTimeout, client.WithHTTPClient(httpClient), client.WithRequestEditorFn(core.UserAgentRequestEditor))
if err != nil {
Expand Down
37 changes: 0 additions & 37 deletions auth/services/oauth/relying_party_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -76,42 +75,6 @@ func TestRelyingParty_RequestRFC003AccessToken(t *testing.T) {
assert.EqualError(t, err, "remote server/nuts node returned error creating access token: server returned HTTP 502 (expected: 200)")
})

t.Run("endpoint security validation (only HTTPS in strict mode)", func(t *testing.T) {
ctx := createRPContext(t, nil)
httpServer := httptest.NewServer(&http2.Handler{
StatusCode: http.StatusOK,
})
httpsServer := httptest.NewTLSServer(&http2.Handler{
StatusCode: http.StatusOK,
})
t.Cleanup(httpServer.Close)
t.Cleanup(httpsServer.Close)

t.Run("HTTPS in strict mode", func(t *testing.T) {
ctx.relyingParty.strictMode = true

response, err := ctx.relyingParty.RequestRFC003AccessToken(context.Background(), bearerToken, *test.MustParseURL(httpsServer.URL))

assert.NoError(t, err)
assert.NotNil(t, response)
})
t.Run("HTTP allowed in non-strict mode", func(t *testing.T) {
ctx.relyingParty.strictMode = false

response, err := ctx.relyingParty.RequestRFC003AccessToken(context.Background(), bearerToken, *test.MustParseURL(httpServer.URL))

assert.NoError(t, err)
assert.NotNil(t, response)
})
t.Run("HTTP not allowed in strict mode", func(t *testing.T) {
ctx.relyingParty.strictMode = true

response, err := ctx.relyingParty.RequestRFC003AccessToken(context.Background(), bearerToken, *test.MustParseURL(httpServer.URL))

assert.EqualError(t, err, fmt.Sprintf("authorization server endpoint must be HTTPS when in strict mode: %s", httpServer.URL))
assert.Nil(t, response)
})
})
}

func TestService_CreateJwtBearerToken(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion docs/pages/deployment/configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,6 @@ requesting an access token from another node on ``/n2n/auth/v1/accesstoken`` doe
json-ld context can only be downloaded from trusted domains configured in ``jsonld.contexts.remoteallowlist``,
and the ``internalratelimiter`` is always on.

Interacting with remote Nuts nodes requires HTTPS: it will refuse to connect to plain HTTP endpoints when in strict mode.
Interacting with remote Nuts nodes requires HTTPS: it will refuse to connect to plain HTTP endpoints when in strict mode.
Strict mode additionally rejects outbound URLs whose host is an IP literal or an RFC 2606 reserved hostname/TLD (e.g. ``*.localhost``, ``*.test``, ``example.com/net/org``).
This applies to every outbound HTTP call made via the shared HTTP client (OpenID4VCI, OAuth relying-party, IAM, Discovery, did:web resolution, etc.) and to every redirect target along the way.
Loading
Loading