fix(core): DSPX-4682 protect pprof endpoints

docs/Configuring.md

@@ -135,7 +135,7 @@ Root level key `server`
 | `auth.dpop.enforce`     | If true, DPoP bindings on Access Tokens are enforced.                                                         | `false` | OPENTDF_SERVER_AUTH_DPOP_ENFORCE     |
 | `auth.enforceDPoP`      | [DEPRECATED] Use `auth.dpop.enforce`. Still honored: DPoP is enforced when either field is true.              | `false` | OPENTDF_SERVER_AUTH_ENFORCEDPOP      |
 | `cryptoProvider`        | A list of public/private keypairs and their use. Described [below](#crypto-provider)                          | empty   |                                      |
-| `enable_pprof`          | Enable golang performance profiling                                                                           | `false` | OPENTDF_SERVER_ENABLE_PPROF          |
+| `enable_pprof`          | Enable Go performance profiling under `/debug/pprof/`; uses HTTP authentication and authorization when server authentication is enabled, with collection durations capped at 30 seconds | `false` | OPENTDF_SERVER_ENABLE_PPROF          |
 | `grpc.reflection`       | The configuration for the grpc server.                                                                        | `true`  | OPENTDF_SERVER_GRPC_REFLECTION       |
 | `public_hostname`       | The public facing hostname for the server.                                                                    |         | OPENTDF_SERVER_PUBLIC_HOSTNAME       |
 | `host`                  | The host address for the server.                                                                              | `""`    | OPENTDF_SERVER_HOST                  |

service/internal/server/server.go

@@ -11,6 +11,7 @@ import (
 	"net/http/pprof"
 	"net/textproto"
 	"regexp"
+	"strconv"
 	"strings"
 	"time"
 
@@ -34,9 +35,11 @@ import (
 )
 
 const (
-	defaultWriteTimeout time.Duration = 10 * time.Second
-	defaultReadTimeout  time.Duration = 10 * time.Second
-	shutdownTimeout     time.Duration = 5 * time.Second
+	defaultWriteTimeout     time.Duration = 10 * time.Second
+	defaultReadTimeout      time.Duration = 10 * time.Second
+	shutdownTimeout         time.Duration = 5 * time.Second
+	maxPprofDurationSeconds               = 30
+	maxPprofFormBodyBytes   int64         = 1024
 )
 
 type Error string
@@ -347,6 +350,16 @@ func newHTTPServer(c Config, connectRPC http.Handler, extraHTTP http.Handler, a
 
 	httpHandler := extraHTTP
 
+	// Keep profiling behind the same authentication and authorization boundary as
+	// the other HTTP handlers.
+	if c.EnablePprof {
+		httpHandler = pprofHandler(httpHandler)
+		// Need to extend write timeout to collect pprof data.
+		if c.HTTPServerConfig.WriteTimeout < maxPprofDurationSeconds*time.Second {
+			c.HTTPServerConfig.WriteTimeout = maxPprofDurationSeconds * time.Second
+		}
+	}
+
 	// Add authN interceptor to extra handlers.
 	if c.Auth.Enabled {
 		httpHandler = a.MuxHandler(httpHandler)
@@ -395,15 +408,6 @@ func newHTTPServer(c Config, connectRPC http.Handler, extraHTTP http.Handler, a
 		httpHandler = corsHandler.Handler(httpHandler)
 	}
 
-	// Enable pprof
-	if c.EnablePprof {
-		httpHandler = pprofHandler(httpHandler)
-		// Need to extend write timeout to collect pprof data.
-		if c.HTTPServerConfig.WriteTimeout < 30*time.Second {
-			c.HTTPServerConfig.WriteTimeout = 30 * time.Second //nolint:mnd // easier to read that we are overriding the default
-		}
-	}
-
 	var handler http.Handler
 	if !c.TLS.Enabled {
 		handler = h2c.NewHandler(routeConnectRPCRequests(connectRPC, httpHandler), &http2.Server{})
@@ -447,10 +451,14 @@ func routeConnectRPCRequests(connectRPC http.Handler, httpHandler http.Handler)
 	})
 }
 
-// ppprof handler
+// pprofHandler routes profiling requests and bounds caller-controlled collection durations.
 func pprofHandler(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if strings.HasPrefix(r.URL.Path, "/debug/pprof/") {
+			if supportsPprofDuration(r.URL.Path) && !validatePprofDuration(w, r) {
+				return
+			}
+
 			switch r.URL.Path {
 			case "/debug/pprof/cmdline":
 				pprof.Cmdline(w, r)
@@ -469,6 +477,51 @@ func pprofHandler(h http.Handler) http.Handler {
 	})
 }
 
+func supportsPprofDuration(path string) bool {
+	switch path {
+	case "/debug/pprof/", "/debug/pprof/cmdline", "/debug/pprof/symbol":
+		return false
+	default:
+		return true
+	}
+}
+
+func validatePprofDuration(w http.ResponseWriter, r *http.Request) bool {
+	r.Body = http.MaxBytesReader(w, r.Body, maxPprofFormBodyBytes)
+	if err := r.ParseForm(); err != nil {
+		writePprofFormError(w, err)
+		return false
+	}
+	//nolint:gosec // MaxBytesReader bounds the complete request body above.
+	if err := r.ParseMultipartForm(maxPprofFormBodyBytes); err != nil && !errors.Is(err, http.ErrNotMultipart) {
+		writePprofFormError(w, err)
+		return false
+	}
+
+	seconds := r.FormValue("seconds")
+	if seconds == "" {
+		return true
+	}
+
+	duration, err := strconv.ParseFloat(seconds, 64)
+	if err == nil && duration > maxPprofDurationSeconds {
+		http.Error(w, fmt.Sprintf("pprof duration must not exceed %d seconds", maxPprofDurationSeconds), http.StatusBadRequest)
+		return false
+	}
+
+	return true
+}
+
+func writePprofFormError(w http.ResponseWriter, err error) {
+	var maxBytesError *http.MaxBytesError
+	if errors.As(err, &maxBytesError) {
+		http.Error(w, "pprof request body too large", http.StatusRequestEntityTooLarge)
+		return
+	}
+
+	http.Error(w, "invalid pprof form body", http.StatusBadRequest)
+}
+
 func newConnectRPC(c Config, authInts []connect.Interceptor, ints []connect.Interceptor, logger *logger.Logger) (*ConnectRPC, error) {
 	interceptors := make([]connect.HandlerOption, 0)
 

service/internal/server/server_test.go

@@ -1,9 +1,13 @@
 package server
 
 import (
+	"bytes"
 	"context"
+	"fmt"
+	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -80,6 +84,154 @@ func TestMergeStringSlices(t *testing.T) {
 	}
 }
 
+func TestNewHTTPServer_PprofRequiresAuthentication(t *testing.T) {
+	server, err := newHTTPServer(Config{
+		Auth:        auth.Config{Enabled: true},
+		EnablePprof: true,
+	}, http.NotFoundHandler(), http.NotFoundHandler(), &auth.Authentication{}, logger.CreateTestLogger())
+	require.NoError(t, err)
+
+	recorder := httptest.NewRecorder()
+	request := httptest.NewRequest(http.MethodGet, "/debug/pprof/", nil)
+	server.Handler.ServeHTTP(recorder, request)
+
+	assert.Equal(t, http.StatusUnauthorized, recorder.Code)
+	assert.Contains(t, recorder.Body.String(), "missing authorization header")
+}
+
+func TestPprofHandler(t *testing.T) {
+	fallback := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	})
+	handler := pprofHandler(fallback)
+
+	tests := []struct {
+		name       string
+		target     string
+		wantStatus int
+	}{
+		{
+			name:       "serves profiling index",
+			target:     "/debug/pprof/",
+			wantStatus: http.StatusOK,
+		},
+		{
+			name:       "rejects long CPU profile",
+			target:     "/debug/pprof/profile?seconds=31",
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "rejects long trace",
+			target:     "/debug/pprof/trace?seconds=30.1",
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "rejects long delta profile",
+			target:     "/debug/pprof/goroutine?seconds=60",
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "preserves invalid duration default",
+			target:     "/debug/pprof/?seconds=invalid",
+			wantStatus: http.StatusOK,
+		},
+		{
+			name:       "passes through non-profiling request",
+			target:     "/healthz",
+			wantStatus: http.StatusNoContent,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			recorder := httptest.NewRecorder()
+			request := httptest.NewRequest(http.MethodGet, test.target, nil)
+
+			handler.ServeHTTP(recorder, request)
+
+			assert.Equal(t, test.wantStatus, recorder.Code)
+		})
+	}
+}
+
+func TestPprofHandlerRejectsBodyDuration(t *testing.T) {
+	fallback := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	})
+	handler := pprofHandler(fallback)
+
+	t.Run("URL encoded", func(t *testing.T) {
+		request := httptest.NewRequest(http.MethodPost, "/debug/pprof/profile", strings.NewReader("seconds=31"))
+		request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		recorder := httptest.NewRecorder()
+
+		handler.ServeHTTP(recorder, request)
+
+		assert.Equal(t, http.StatusBadRequest, recorder.Code)
+	})
+
+	t.Run("multipart", func(t *testing.T) {
+		var body bytes.Buffer
+		writer := multipart.NewWriter(&body)
+		require.NoError(t, writer.WriteField("seconds", "31"))
+		require.NoError(t, writer.Close())
+
+		request := httptest.NewRequest(http.MethodPost, "/debug/pprof/trace", &body)
+		request.Header.Set("Content-Type", writer.FormDataContentType())
+		recorder := httptest.NewRecorder()
+
+		handler.ServeHTTP(recorder, request)
+
+		assert.Equal(t, http.StatusBadRequest, recorder.Code)
+	})
+}
+
+func TestPprofHandlerRejectsOversizedBody(t *testing.T) {
+	fallback := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	})
+	handler := pprofHandler(fallback)
+
+	t.Run("URL encoded", func(t *testing.T) {
+		body := "seconds=1&padding=" + strings.Repeat("x", int(maxPprofFormBodyBytes))
+		request := httptest.NewRequest(http.MethodPost, "/debug/pprof/profile", strings.NewReader(body))
+		request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		recorder := httptest.NewRecorder()
+
+		handler.ServeHTTP(recorder, request)
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, recorder.Code)
+	})
+
+	t.Run("multipart", func(t *testing.T) {
+		var body bytes.Buffer
+		writer := multipart.NewWriter(&body)
+		require.NoError(t, writer.WriteField("seconds", "1"))
+		require.NoError(t, writer.WriteField("padding", strings.Repeat("x", int(maxPprofFormBodyBytes))))
+		require.NoError(t, writer.Close())
+
+		request := httptest.NewRequest(http.MethodPost, "/debug/pprof/trace", &body)
+		request.Header.Set("Content-Type", writer.FormDataContentType())
+		recorder := httptest.NewRecorder()
+
+		handler.ServeHTTP(recorder, request)
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, recorder.Code)
+	})
+}
+
+func TestPprofHandlerPreservesSymbolPostBody(t *testing.T) {
+	programCounter := reflect.ValueOf(TestPprofHandlerPreservesSymbolPostBody).Pointer()
+	request := httptest.NewRequest(http.MethodPost, "/debug/pprof/symbol", strings.NewReader(fmt.Sprintf("%#x", programCounter)))
+	request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	recorder := httptest.NewRecorder()
+
+	pprofHandler(http.NotFoundHandler()).ServeHTTP(recorder, request)
+
+	assert.Equal(t, http.StatusOK, recorder.Code)
+	assert.Contains(t, recorder.Body.String(), "TestPprofHandlerPreservesSymbolPostBody")
+}
+
 func Test_OpenTDFServer_RegisterReflectionHandlers_Enabled_RegistersOnlyExternalHandlers(t *testing.T) {
 	server := newReflectionTestServer(t, true)