admin: require path segment boundary in remote access control (#7673)

admin.go

@@ -212,8 +212,8 @@ type AdminAccess struct {
 // AdminPermissions specifies what kinds of requests are allowed
 // to be made to the admin endpoint.
 type AdminPermissions struct {
-	// The API paths allowed. Paths are simple prefix matches.
-	// Any subpath of the specified paths will be allowed.
+	// The API paths allowed. A request path must either equal an
+	// allowed path or be a subpath with a path-segment boundary.
 	Paths []string `json:"paths,omitempty"`
 
 	// The HTTP methods allowed for the given paths.
@@ -718,7 +718,7 @@ func (remote RemoteAdmin) enforceAccessControls(r *http.Request) error {
 						// verify path
 						pathFound := accessPerm.Paths == nil
 						for _, allowedPath := range accessPerm.Paths {
-							if strings.HasPrefix(r.URL.Path, allowedPath) {
+							if adminPathAllowed(r.URL.Path, allowedPath) {
 								pathFound = true
 								break
 							}
@@ -747,6 +747,19 @@ func (remote RemoteAdmin) enforceAccessControls(r *http.Request) error {
 	}
 }
 
+func adminPathAllowed(reqPath, allowedPath string) bool {
+	if allowedPath == "" || allowedPath == "/" {
+		return strings.HasPrefix(reqPath, allowedPath)
+	}
+	if reqPath == allowedPath {
+		return true
+	}
+	if strings.HasSuffix(allowedPath, "/") {
+		return strings.HasPrefix(reqPath, allowedPath)
+	}
+	return strings.HasPrefix(reqPath, allowedPath+"/")
+}
+
 func stopAdminServer(srv *http.Server) error {
 	if srv == nil {
 		return fmt.Errorf("no admin server")

admin_test.go

@@ -16,8 +16,11 @@ package caddy
 
 import (
 	"context"
+	"crypto"
+	"crypto/tls"
 	"crypto/x509"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"maps"
 	"net/http"
@@ -53,6 +56,13 @@ var testCfg = []byte(`{
 		}
 		`)
 
+type testAdminPublicKey string
+
+func (k testAdminPublicKey) Equal(x crypto.PublicKey) bool {
+	other, ok := x.(testAdminPublicKey)
+	return ok && k == other
+}
+
 func TestUnsyncedConfigAccess(t *testing.T) {
 	// each test is performed in sequence, so
 	// each change builds on the previous ones;
@@ -651,6 +661,99 @@ func TestAllowedOriginsUnixSocket(t *testing.T) {
 	}
 }
 
+func TestRemoteAdminAccessControlPathSegmentMatching(t *testing.T) {
+	const authorizedKey testAdminPublicKey = "authorized"
+	peerCert := &x509.Certificate{PublicKey: authorizedKey}
+
+	tests := []struct {
+		name        string
+		allowedPath string
+		requestPath string
+		wantErr     bool
+	}{
+		{
+			name:        "exact path",
+			allowedPath: "/pki/ca/prod",
+			requestPath: "/pki/ca/prod",
+			wantErr:     false,
+		},
+		{
+			name:        "subpath",
+			allowedPath: "/pki/ca/prod",
+			requestPath: "/pki/ca/prod/certificates",
+			wantErr:     false,
+		},
+		{
+			name:        "trailing slash subpath",
+			allowedPath: "/pki/ca/prod/",
+			requestPath: "/pki/ca/prod/certificates",
+			wantErr:     false,
+		},
+		{
+			name:        "sibling with shared prefix",
+			allowedPath: "/pki/ca/prod",
+			requestPath: "/pki/ca/prod-backup",
+			wantErr:     true,
+		},
+		{
+			name:        "same segment plus digit",
+			allowedPath: "/pki/ca/prod",
+			requestPath: "/pki/ca/prod1",
+			wantErr:     true,
+		},
+		{
+			name:        "root path",
+			allowedPath: "/",
+			requestPath: "/pki/ca/prod",
+			wantErr:     false,
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			remote := RemoteAdmin{
+				AccessControl: []*AdminAccess{
+					{
+						Permissions: []AdminPermissions{
+							{
+								Methods: []string{http.MethodGet},
+								Paths:   []string{test.allowedPath},
+							},
+						},
+						publicKeys: []crypto.PublicKey{authorizedKey},
+					},
+				},
+			}
+
+			req := httptest.NewRequest(http.MethodGet, "https://localhost:2021"+test.requestPath, nil)
+			req.TLS = &tls.ConnectionState{
+				VerifiedChains: [][]*x509.Certificate{{peerCert}},
+			}
+
+			err := remote.enforceAccessControls(req)
+			if test.wantErr {
+				if err == nil {
+					t.Errorf("test %d (%s): allowed path %q, request path %q: expected forbidden error, got nil", i, test.name, test.allowedPath, test.requestPath)
+					return
+				}
+				var apiErr APIError
+				if !errors.As(err, &apiErr) {
+					t.Errorf("test %d (%s): allowed path %q, request path %q: expected APIError with HTTP status %d, got %T: %v", i, test.name, test.allowedPath, test.requestPath, http.StatusForbidden, err, err)
+					return
+				}
+				if apiErr.HTTPStatus != http.StatusForbidden {
+					t.Errorf("test %d (%s): allowed path %q, request path %q: expected HTTP status %d, got %d", i, test.name, test.allowedPath, test.requestPath, http.StatusForbidden, apiErr.HTTPStatus)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Errorf("test %d (%s): allowed path %q, request path %q: expected no error, got %v", i, test.name, test.allowedPath, test.requestPath, err)
+			}
+		})
+	}
+}
+
 func TestReplaceRemoteAdminServer(t *testing.T) {
 	const testCert = `MIIDCTCCAfGgAwIBAgIUXsqJ1mY8pKlHQtI3HJ23x2eZPqwwDQYJKoZIhvcNAQEL
 BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTIzMDEwMTAwMDAwMFoXDTI0MDEw