fix(http): serve /status/diagnostics only on the internal interface

docs/pages/deployment/monitoring.rst

@@ -87,6 +87,10 @@ Basic diagnostics
     this page is intended to be read by humans, not machines.
     all but the ``status`` entry are related to V5 functionality (gRPC network, VDRv1 and VCRv1 APIs).
 
+.. note::
+
+    this endpoint is only served on the internal interface. It discloses the exact software version, the node's network peers and data store counts, so requests arriving through the public interface are refused with HTTP 403. The ``/status`` endpoint (without ``/diagnostics``) remains available on both interfaces as a liveness signal.
+
 Returns the status of the various services in ``yaml`` format:
 
 .. code-block:: text

docs/pages/release_notes.rst

@@ -14,6 +14,7 @@ Unreleased
 * #4233: ``request-credential`` API gains an optional ``credential_request_params`` JSON object overlaid on top of the OpenID4VCI Credential Request body sent to the issuer. Lets the wallet talk to issuers that accept additional fields, or to override the credential request entirely.
 
 ## Security
+* #4442: ``/status/diagnostics`` is now only served on the internal interface; requests through the public interface are refused with HTTP 403 and an explanatory message. The endpoint discloses the exact software version, the gRPC peer list and data store counts, and the deployment documentation has always listed it as internal. ``/status`` remains available on both interfaces as a liveness signal, so external load balancers and uptime monitors probing ``/status`` are unaffected. If an external monitor probes ``/status/diagnostics``, point it at ``/status`` or at the internal interface instead. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4442
 * #4441: Inbound HTTP request bodies are now limited to 1MB on both the public and internal interfaces; larger requests are rejected with HTTP 413 (Request Entity Too Large). Previously no limit was enforced, contrary to what the deployment documentation stated. The heaviest legitimate requests (OAuth POSTs carrying Verifiable Presentations) stay well below this limit, and it matches the ``client_max_body_size 1M`` reverse proxy configuration the documentation recommends. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4441
 * #4439: Helm chart (version 0.0.9): default ``verbosity`` changed from ``debug`` to ``info``, matching the node's own default. Debug verbosity produces far more log output than production needs and increases the impact of any log-hygiene issue. Set ``nuts.config.verbosity: debug`` in your own values to restore the old behavior. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4439
 * #4440: In strictmode, ``http.log: metadata-and-body`` is no longer honored: the node resets it to ``metadata`` at startup and logs a warning. Full body logging wrote OAuth token endpoint request and response bodies (client assertions, VP tokens, authorization codes and issued access tokens) to the log at Info severity. Non-strictmode deployments are unaffected. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4440

http/engine.go

@@ -43,6 +43,20 @@ import (
 
 const moduleName = "HTTP"
 
+// diagnosticsPath is the status endpoint that reports software version, git commit, peer list
+// and store counts. It is only meant to be available on the internal interface, but it shares
+// its first path segment with /status, which deliberately stays public as a liveness signal
+// for load balancers and uptime monitors.
+//
+// Interface binding (MultiEcho) resolves on the first path segment only and rejects subpath
+// binds, so /status and /status/diagnostics cannot be bound to different interfaces through
+// the bind table: the route is registered on every interface that serves /status. Therefore
+// internalOnlyMiddleware refuses requests that reach this path through any interface other
+// than the internal one. If the bind table ever learns longest-prefix matching so subpaths
+// can be bound to their own interface, this path should become a regular internal-only bind
+// and the middleware can be removed.
+const diagnosticsPath = StatusPath + "/diagnostics"
+
 // requestBodySizeLimit caps the size of inbound HTTP request bodies on all interfaces, returning
 // 413 Request Entity Too Large when exceeded. The largest legitimate requests are OAuth POSTs
 // carrying Verifiable Presentations; a deliberately heavy presentation (5 JWT credentials, each
@@ -112,10 +126,53 @@ func (h *Engine) Configure(serverConfig core.ServerConfig) error {
 	h.applyTracingMiddleware(h.server)
 	h.applyRateLimiterMiddleware(h.server, serverConfig)
 	h.applyLoggerMiddleware(h.server, []string{MetricsPath, StatusPath, HealthPath}, h.config.Log)
+	diagnosticsGuard, err := internalOnlyMiddleware(h.config.Internal.Address, diagnosticsPath)
+	if err != nil {
+		return err
+	}
+	h.server.Use(diagnosticsGuard)
 	h.server.Use(middleware.BodyLimit(requestBodySizeLimit))
 	return h.applyAuthMiddleware(h.server, InternalPath, h.config.Internal.Auth)
 }
 
+// internalOnlyMiddleware returns middleware that refuses requests to the given path (and its
+// subpaths) unless the connection was accepted on the interface the internal API is bound to.
+//
+// How the interface is determined: Go's net/http server stores the connection's local address
+// (the address the listener accepted on) in the request context under http.LocalAddrContextKey.
+// Every HTTP interface listens on its own port, since binding the same port twice fails at
+// startup, so comparing the local address port against the internal interface's configured
+// port identifies the interface the request arrived on. When the operator configures the
+// public and internal interface to the same address there is only one server and the ports
+// always match, which honors that (explicitly configured) setup. The comparison uses only the
+// port, not the host: the configured bind host (e.g. ":8081" or "0.0.0.0:8081") does not have
+// to equal the connection's concrete local IP.
+//
+// Requests are refused with 403 rather than 404: the endpoint's existence is public knowledge
+// (the node is open source), so a 404 would hide nothing from an attacker, while the explicit
+// message tells an operator whose external monitor probes this path exactly why it broke.
+// When the local address is missing from the request context or unparseable, the request is
+// refused (fail closed); this cannot happen with Go's net/http server, which always sets it.
+func internalOnlyMiddleware(internalAddress string, path string) (echo.MiddlewareFunc, error) {
+	_, internalPort, err := net.SplitHostPort(internalAddress)
+	if err != nil {
+		return nil, fmt.Errorf("invalid internal address %s: %w", internalAddress, err)
+	}
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c echo.Context) error {
+			if !matchesPath(c.Request().URL.Path, path) {
+				return next(c)
+			}
+			if localAddr, ok := c.Request().Context().Value(http.LocalAddrContextKey).(net.Addr); ok {
+				if _, port, err := net.SplitHostPort(localAddr.String()); err == nil && port == internalPort {
+					return next(c)
+				}
+			}
+			return echo.NewHTTPError(http.StatusForbidden, "only available on the internal interface")
+		}
+	}, nil
+}
+
 func (h *Engine) configureClient(serverConfig core.ServerConfig) error {
 	client.StrictMode = serverConfig.Strictmode
 	if err := client.SetAllowedNonPublicCIDRs(h.config.Client.AllowedInternalCIDRs); err != nil {

http/engine_test.go

@@ -471,6 +471,49 @@ func TestEngine_RequestBodyLimit(t *testing.T) {
 	})
 }
 
+func TestEngine_DiagnosticsInternalOnly(t *testing.T) {
+	// Configure sets the package-global client strict mode flag; restore it afterwards.
+	oldStrictMode := client.StrictMode
+	t.Cleanup(func() { client.StrictMode = oldStrictMode })
+	noop := func() {}
+	engine := New(noop, nil)
+	engine.config = createTestConfig()
+	require.NoError(t, engine.Configure(*core.NewServerConfig()))
+	// Simulate the routes the status engine registers under /status.
+	engine.Router().GET("/status", func(c echo.Context) error {
+		return c.String(http.StatusOK, "OK")
+	})
+	engine.Router().GET("/status/diagnostics", func(c echo.Context) error {
+		return c.String(http.StatusOK, "diagnostics")
+	})
+	require.NoError(t, engine.Start())
+	defer engine.Shutdown()
+	assertServerStarted(t, engine.config.Public.Address)
+	assertServerStarted(t, engine.config.Internal.Address)
+
+	t.Run("/status is available on the public interface", func(t *testing.T) {
+		response, err := http.Get("http://" + engine.config.Public.Address + "/status")
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, response.StatusCode)
+	})
+	t.Run("/status/diagnostics is available on the internal interface", func(t *testing.T) {
+		response, err := http.Get("http://" + engine.config.Internal.Address + "/status/diagnostics")
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, response.StatusCode)
+		body, err := io.ReadAll(response.Body)
+		require.NoError(t, err)
+		assert.Equal(t, "diagnostics", string(body))
+	})
+	t.Run("/status/diagnostics is forbidden on the public interface", func(t *testing.T) {
+		response, err := http.Get("http://" + engine.config.Public.Address + "/status/diagnostics")
+		require.NoError(t, err)
+		body, err := io.ReadAll(response.Body)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusForbidden, response.StatusCode)
+		assert.NotContains(t, string(body), "diagnostics")
+	})
+}
+
 func assertServerStarted(t *testing.T, address string) {
 	t.Helper()
 	var err error