fix(http): strict-mode SSRF guard (#4420) and docs (#4423)

docs/pages/deployment/security-considerations.rst

@@ -15,6 +15,59 @@ In addition to securing the internal APIs, it's recommended to limit access to t
 This will allow you to control access to the public APIs, do TLS termination and add additional security measures.
 Block any path that's not used by the Nuts node.
 
+.. _ssrf-protection:
+
+Outbound HTTP and SSRF protection
+*********************************
+
+The Nuts node fetches URLs supplied by other parties, for example DID documents, OAuth metadata and
+Discovery Service endpoints. An attacker could register a URL that resolves to an address inside your
+network and let the node request it on their behalf. This is known as Server-Side Request Forgery (SSRF).
+
+In strict mode the node refuses outbound HTTP connections to non-public addresses: loopback,
+private (RFC 1918), carrier-grade NAT, link-local, unique local, and the other special-purpose ranges
+published in the IANA
+`IPv4 <https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml>`_ and
+`IPv6 <https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml>`_
+special-purpose address registries. The check runs at connect time against the resolved IP address, so it
+also covers redirects and cannot be bypassed with DNS rebinding. Cloud provider metadata endpoints (such as
+``169.254.169.254`` and Azure's ``168.63.129.16``) are always blocked, following the
+`OWASP SSRF prevention cheat sheet <https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html>`_.
+Redirects that downgrade from HTTPS to HTTP are refused as well.
+
+A blocked request fails with an error like::
+
+    strictmode: blocked connection to non-public address 10.0.0.5
+
+Allowing internal ranges
+========================
+
+If a legitimate flow targets a private address, for example an internal OpenID4VCI issuer or an OAuth
+flow that stays inside your network, then permit that range with ``http.client.allowedinternalcidrs``.
+If part of an allowed range must stay unreachable, then deny that part with ``http.client.deniedcidrs``;
+denied ranges take precedence:
+
+.. code-block:: yaml
+
+    http:
+      client:
+        allowedinternalcidrs:
+          - 10.0.0.0/8
+        deniedcidrs:
+          - 10.5.0.0/16
+
+Keep allowed ranges as narrow as possible. Every address in an allowed range becomes reachable for any
+URL an external party can make the node fetch.
+
+Blocking additional ranges
+==========================
+
+Some networks use publicly routable addresses for internal-only systems. Public addresses are not
+covered by the built-in guard, so add such ranges to ``http.client.deniedcidrs`` explicitly.
+
+The built-in blocks for cloud metadata endpoints cannot be lifted with ``allowedinternalcidrs``.
+Both options only apply in strict mode; without strict mode the SSRF guard is disabled.
+
 D(D)oS Protection
 *****************
 

docs/pages/release_notes.rst

@@ -2,6 +2,13 @@
 Release notes
 #############
 
+****************
+Unreleased
+****************
+
+## Security
+* #4420: Harden the strict-mode HTTP client against SSRF. In strict mode the client now refuses at connect time to reach non-public addresses (loopback, private/RFC1918, unique local, link-local and unspecified), checked against the resolved IP so DNS-rebinding cannot bypass it, and refuses to follow a redirect that downgrades from HTTPS to HTTP. Cloud provider metadata endpoints are always blocked, following the OWASP SSRF prevention cheat sheet. Deployments that legitimately reach a private address for an internal flow (such as an internal credential offering or OAuth user flow) can permit specific ranges with ``http.client.allowedinternalcidrs``; publicly routable ranges that are internal-only can additionally be blocked with ``http.client.deniedcidrs``, which takes precedence. See :ref:`Outbound HTTP and SSRF protection <ssrf-protection>` for deployment guidance. Reported by @raysabee, fixed by @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4420
+
 *****************
 Peanut (v6.2.10)
 *****************

e2e-tests/openid4vci/issuer-initiated/docker-compose.yml

@@ -44,4 +44,4 @@ services:
       - "../../shared_config/nodeB-http-nginx.conf:/etc/nginx/conf.d/nuts-http.conf:ro"
       - "../../tls-certs/nodeB-certificate.pem:/etc/nginx/ssl/server.pem:ro"
       - "../../tls-certs/nodeB-certificate.pem:/etc/nginx/ssl/key.pem:ro"
-      - "../../tls-certs/truststore.pem:/etc/nginx/ssl/truststore.pem:ro"
+      - "../../tls-certs/truststore.pem:/etc/nginx/ssl/truststore.pem:ro"

e2e-tests/openid4vci/issuer-initiated/node-A/nuts.yaml

@@ -5,6 +5,13 @@ internalratelimiter: false
 http:
   internal:
     address: :8081
+  client:
+    # Docker auto-assigns the compose network an arbitrary private subnet, so permit all RFC1918
+    # ranges for the strict-mode SSRF guard. Narrow-allowlist precision is covered by unit tests.
+    allowedinternalcidrs:
+      - 10.0.0.0/8
+      - 172.16.0.0/12
+      - 192.168.0.0/16
 auth:
   contractvalidators:
     - dummy

e2e-tests/openid4vci/issuer-initiated/node-B/nuts.yaml

@@ -5,6 +5,13 @@ internalratelimiter: false
 http:
   internal:
     address: :8081
+  client:
+    # Docker auto-assigns the compose network an arbitrary private subnet, so permit all RFC1918
+    # ranges for the strict-mode SSRF guard. Narrow-allowlist precision is covered by unit tests.
+    allowedinternalcidrs:
+      - 10.0.0.0/8
+      - 172.16.0.0/12
+      - 192.168.0.0/16
 auth:
   contractvalidators:
     - dummy

e2e-tests/openid4vci/network-issuance/docker-compose.yml

@@ -53,4 +53,4 @@ services:
       - "../../tls-certs/nodeB-certificate.pem:/etc/nginx/ssl/key.pem:ro"
       - "../../tls-certs/truststore.pem:/etc/nginx/ssl/truststore.pem:ro"
     depends_on:
-      - nodeB-backend
+      - nodeB-backend

e2e-tests/openid4vci/network-issuance/node-A/nuts.yaml

@@ -6,6 +6,13 @@ datadir: /opt/nuts/data
 http:
   internal:
     address: :8081
+  client:
+    # Docker auto-assigns the compose network an arbitrary private subnet, so permit all RFC1918
+    # ranges for the strict-mode SSRF guard. Narrow-allowlist precision is covered by unit tests.
+    allowedinternalcidrs:
+      - 10.0.0.0/8
+      - 172.16.0.0/12
+      - 192.168.0.0/16
 auth:
   contractvalidators:
     - dummy

e2e-tests/openid4vci/network-issuance/node-B/nuts.yaml

@@ -6,6 +6,13 @@ datadir: /opt/nuts/data
 http:
   internal:
     address: :8081
+  client:
+    # Docker auto-assigns the compose network an arbitrary private subnet, so permit all RFC1918
+    # ranges for the strict-mode SSRF guard. Narrow-allowlist precision is covered by unit tests.
+    allowedinternalcidrs:
+      - 10.0.0.0/8
+      - 172.16.0.0/12
+      - 192.168.0.0/16
 auth:
   contractvalidators:
     - dummy

go.mod

@@ -199,6 +199,7 @@ require (
 )
 
 require (
+	code.dny.dev/ssrf v0.3.0
 	github.com/aws/aws-sdk-go-v2 v1.41.1
 	github.com/aws/aws-sdk-go-v2/config v1.32.7
 	github.com/aws/aws-sdk-go-v2/feature/rds/auth v1.6.17

go.sum

@@ -1,3 +1,5 @@
+code.dny.dev/ssrf v0.3.0 h1:QDbeKauc3vR5/w9TwlhVYAQwK15cECL7sHkCUUT55ms=
+code.dny.dev/ssrf v0.3.0/go.mod h1:5thKUcQNqSxzz5TVBRkvHZOGmfRM3vI+WU2GGx9xUbw=
 filippo.io/edwards25519 v1.1.1 h1:YpjwWWlNmGIDyXOn8zLzqiD+9TyIlPhGFG96P39uBpw=
 filippo.io/edwards25519 v1.1.1/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
 github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.0/go.mod h1:bjGvMhVMb+EEm3VRNQawDMUyMMjo+S5ewNjflkep/0Q=

http/client/client.go

@@ -24,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"time"
 
@@ -33,6 +34,10 @@ import (
 )
 
 // SafeHttpTransport is a http.Transport that can be used as a default transport for HTTP clients.
+// It carries the strict-mode SSRF dial guard (see denyNonPublicAddr), but only that: the HTTPS
+// requirement, the redirect-downgrade check and the response size limit live on StrictHTTPClient.
+// Do not build a raw http.Client on this transport for outbound requests; use the New* constructors
+// so all strict-mode protections apply.
 var SafeHttpTransport *http.Transport
 
 func init() {
@@ -43,6 +48,14 @@ func init() {
 	SafeHttpTransport.TLSClientConfig.MinVersion = tls.VersionTLS12
 	// to prevent slow responses from public clients to have significant impact (default was unlimited)
 	SafeHttpTransport.MaxConnsPerHost = 5
+	// guard against SSRF: in strict mode, refuse to connect to loopback/link-local/
+	// unspecified addresses, checked against the resolved IP so DNS-rebinding cannot
+	// bypass it. Keeps the default dialer timeouts used by http.DefaultTransport.
+	SafeHttpTransport.DialContext = (&net.Dialer{
+		Timeout:   30 * time.Second,
+		KeepAlive: 30 * time.Second,
+		Control:   denyNonPublicAddr,
+	}).DialContext
 	// set DefaultCachingTransport to SafeHttpTransport so it is set even when caching is disabled
 	DefaultCachingTransport = SafeHttpTransport
 }
@@ -55,6 +68,23 @@ func httpSpanName(_ string, r *http.Request) string {
 // StrictMode is a flag that can be set to true to enable strict mode for the HTTP client.
 var StrictMode bool
 
+// checkRedirect is the http.Client.CheckRedirect policy for the strict HTTP client.
+// Setting CheckRedirect replaces the standard library's default policy, so the
+// 10-redirect cap is reimplemented here. In strict mode it also refuses to follow
+// a redirect to a non-HTTPS target: the HTTPS check in Do only guards the first
+// hop, and the dialer guard sees only the resolved IP and not the scheme, so
+// without this a valid remote host could redirect the client onto a plaintext
+// internal endpoint.
+func checkRedirect(req *http.Request, via []*http.Request) error {
+	if len(via) >= 10 {
+		return errors.New("stopped after 10 redirects")
+	}
+	if StrictMode && req.URL.Scheme != "https" {
+		return errors.New("strictmode is enabled, but redirect target is not over HTTPS")
+	}
+	return nil
+}
+
 // DefaultMaxHttpResponseSize is a default maximum size of an HTTP response body that will be read.
 // Very large or unbounded HTTP responses can cause denial-of-service, so it's good to limit how much data is read.
 // This of course heavily depends on the use case, but 1MB is a reasonable default.
@@ -75,8 +105,9 @@ func New(timeout time.Duration) *StrictHTTPClient {
 	transport := getTransport(SafeHttpTransport)
 	return &StrictHTTPClient{
 		client: &http.Client{
-			Transport: transport,
-			Timeout:   timeout,
+			Transport:     transport,
+			Timeout:       timeout,
+			CheckRedirect: checkRedirect,
 		},
 	}
 }
@@ -98,8 +129,9 @@ func NewWithCache(timeout time.Duration) *StrictHTTPClient {
 	transport := getTransport(DefaultCachingTransport)
 	return &StrictHTTPClient{
 		client: &http.Client{
-			Transport: transport,
-			Timeout:   timeout,
+			Transport:     transport,
+			Timeout:       timeout,
+			CheckRedirect: checkRedirect,
 		},
 	}
 }
@@ -112,8 +144,9 @@ func NewWithTLSConfig(timeout time.Duration, tlsConfig *tls.Config) *StrictHTTPC
 	transport.TLSClientConfig = tlsConfig
 	return &StrictHTTPClient{
 		client: &http.Client{
-			Transport: getTransport(transport),
-			Timeout:   timeout,
+			Transport:     getTransport(transport),
+			Timeout:       timeout,
+			CheckRedirect: checkRedirect,
 		},
 	}
 }

http/client/client_test.go

@@ -35,6 +35,8 @@ import (
 )
 
 func TestStrictHTTPClient(t *testing.T) {
+	oldStrictMode := StrictMode
+	t.Cleanup(func() { StrictMode = oldStrictMode })
 	t.Run("caching transport", func(t *testing.T) {
 		t.Run("strict mode enabled", func(t *testing.T) {
 			rt := &stubRoundTripper{}
@@ -99,6 +101,65 @@ func TestStrictHTTPClient(t *testing.T) {
 	})
 }
 
+func TestStrictHTTPClient_RedirectScheme(t *testing.T) {
+	original := tracing.Enabled()
+	tracing.SetEnabled(false) // ensure the constructed client uses the raw transport
+	t.Cleanup(func() { tracing.SetEnabled(original) })
+
+	// httptest servers bind to loopback, which the strict-mode dial guard blocks. Permit loopback
+	// through the allowlist rather than bypassing the guard, so the guard stays active and the
+	// redirect scheme check is what must block the plaintext hop.
+	oldAllow := allowedNonPublicNets
+	require.NoError(t, SetAllowedNonPublicCIDRs([]string{"127.0.0.0/8", "::1/128"}))
+	t.Cleanup(func() { allowedNonPublicNets = oldAllow })
+
+	// Plaintext HTTP endpoint the redirect points to.
+	var reached atomic.Bool
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		reached.Store(true)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer target.Close()
+
+	// Valid HTTPS remote that redirects the client onto the plaintext endpoint.
+	redirector := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, target.URL, http.StatusFound)
+	}))
+	defer redirector.Close()
+
+	// Trust the test TLS server via NewWithTLSConfig, a real production constructor.
+	tlsConfig := redirector.Client().Transport.(*http.Transport).TLSClientConfig
+
+	setStrictMode := func(t *testing.T, v bool) {
+		old := StrictMode
+		StrictMode = v
+		t.Cleanup(func() { StrictMode = old })
+	}
+
+	t.Run("strict mode refuses redirect from HTTPS to HTTP", func(t *testing.T) {
+		setStrictMode(t, true)
+		reached.Store(false)
+
+		client := NewWithTLSConfig(time.Second, tlsConfig)
+		req, _ := http.NewRequest("GET", redirector.URL, nil)
+		_, err := client.Do(req)
+
+		assert.Error(t, err, "strict mode must not follow a redirect from HTTPS to HTTP")
+		assert.False(t, reached.Load(), "the plaintext endpoint must not be contacted")
+	})
+	t.Run("non-strict mode follows redirect to HTTP", func(t *testing.T) {
+		setStrictMode(t, false)
+		reached.Store(false)
+
+		client := NewWithTLSConfig(time.Second, tlsConfig)
+		req, _ := http.NewRequest("GET", redirector.URL, nil)
+		_, err := client.Do(req)
+
+		assert.NoError(t, err)
+		assert.True(t, reached.Load(), "non-strict mode should follow the redirect")
+	})
+}
+
 func TestLimitedReadAll(t *testing.T) {
 	t.Run("less than limit", func(t *testing.T) {
 		data := strings.Repeat("a", 10)