diff --git a/admin.go b/admin.go index 766d8506a..fcf8ab6cb 100644 --- a/admin.go +++ b/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") diff --git a/admin_test.go b/admin_test.go index 3801c301a..db6d6c45a 100644 --- a/admin_test.go +++ b/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