diff --git a/admin.go b/admin.go index 97af846ef..ecdca859f 100644 --- a/admin.go +++ b/admin.go @@ -554,6 +554,18 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { accessControl.publicKeys = append(accessControl.publicKeys, cert.PublicKey) clientCertPool.AddCert(cert) } + for j, perm := range accessControl.Permissions { + for _, permPath := range perm.Paths { + if permPath == "" || permPath == "/" { + continue + } + cleanPath := path.Clean(permPath) + hasCanonicalTrailingSlash := cleanPath != "/" && strings.TrimSuffix(permPath, "/") == cleanPath + if cleanPath != permPath && !hasCanonicalTrailingSlash { + return fmt.Errorf("access control %d permission %d: path %q is not canonical (did you mean %q?)", i, j, permPath, cleanPath) + } + } + } } // create TLS config that will enforce mutual authentication @@ -723,14 +735,18 @@ 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 { + reqPathHadTrailingSlash := strings.HasSuffix(reqPath, "/") + reqPath = path.Clean(reqPath) + if allowedPath == "" { return true } - if strings.HasSuffix(allowedPath, "/") { - return strings.HasPrefix(reqPath, allowedPath) + allowedPathHadTrailingSlash := allowedPath != "/" && strings.HasSuffix(allowedPath, "/") + allowedPath = path.Clean(allowedPath) + if allowedPath == "/" { + return true + } + if reqPath == allowedPath { + return !allowedPathHadTrailingSlash || reqPathHadTrailingSlash } return strings.HasPrefix(reqPath, allowedPath+"/") } diff --git a/admin_test.go b/admin_test.go index dda06a9e9..1c42ee93f 100644 --- a/admin_test.go +++ b/admin_test.go @@ -684,6 +684,18 @@ func TestRemoteAdminAccessControlPathSegmentMatching(t *testing.T) { requestPath: "/pki/ca/prod/certificates", wantErr: false, }, + { + name: "trailing slash scope excludes exact path without slash", + allowedPath: "/pki/ca/prod/", + requestPath: "/pki/ca/prod", + wantErr: true, + }, + { + name: "trailing slash scope includes exact path with slash", + allowedPath: "/pki/ca/prod/", + requestPath: "/pki/ca/prod/", + wantErr: false, + }, { name: "sibling with shared prefix", allowedPath: "/pki/ca/prod", @@ -702,6 +714,48 @@ func TestRemoteAdminAccessControlPathSegmentMatching(t *testing.T) { requestPath: "/pki/ca/prod", wantErr: false, }, + { + name: "empty path preserves allow all", + allowedPath: "", + requestPath: "/pki/ca/prod", + wantErr: false, + }, + { + name: "dot-dot traversal out of scope", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod/../../../../load", + wantErr: true, + }, + { + name: "dot-dot traversal out of trailing slash scope", + allowedPath: "/pki/ca/prod/", + requestPath: "/pki/ca/prod/../../../../load", + wantErr: true, + }, + { + name: "encoded traversal (net/url decodes %2e to . before path.Clean resolves ..)", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod/%2e%2e/load", + wantErr: true, + }, + { + name: "sibling traversal", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod/../staging", + wantErr: true, + }, + { + name: "collapsed slashes", + allowedPath: "/pki/ca/prod", + requestPath: "/pki//ca/prod", + wantErr: false, + }, + { + name: "exact request with trailing slash", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod/", + wantErr: false, + }, } for i, test := range tests { @@ -839,6 +893,96 @@ vq+SH04xKhtFudVBAQ==` }, wantErr: true, }, + { + name: "non-canonical root path with trailing slash", + cfg: &Config{ + Admin: &AdminConfig{ + Identity: &IdentityConfig{}, + Remote: &RemoteAdmin{ + Listen: "localhost:2021", + AccessControl: []*AdminAccess{ + { + PublicKeys: []string{testCert}, + Permissions: []AdminPermissions{{Methods: []string{"GET"}, Paths: []string{"//"}}}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "canonical subpath with trailing slash", + cfg: &Config{ + Admin: &AdminConfig{ + Identity: &IdentityConfig{}, + Remote: &RemoteAdmin{ + Listen: "localhost:2021", + AccessControl: []*AdminAccess{ + { + PublicKeys: []string{testCert}, + Permissions: []AdminPermissions{{Methods: []string{"GET"}, Paths: []string{"/foo/"}}}, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "non-canonical path /..", + cfg: &Config{ + Admin: &AdminConfig{ + Identity: &IdentityConfig{}, + Remote: &RemoteAdmin{ + Listen: "localhost:2021", + AccessControl: []*AdminAccess{ + { + PublicKeys: []string{testCert}, + Permissions: []AdminPermissions{{Methods: []string{"GET"}, Paths: []string{"/.."}}}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "non-canonical path with double slashes", + cfg: &Config{ + Admin: &AdminConfig{ + Identity: &IdentityConfig{}, + Remote: &RemoteAdmin{ + Listen: "localhost:2021", + AccessControl: []*AdminAccess{ + { + PublicKeys: []string{testCert}, + Permissions: []AdminPermissions{{Methods: []string{"GET"}, Paths: []string{"/foo//bar"}}}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "non-canonical path with double trailing slashes", + cfg: &Config{ + Admin: &AdminConfig{ + Identity: &IdentityConfig{}, + Remote: &RemoteAdmin{ + Listen: "localhost:2021", + AccessControl: []*AdminAccess{ + { + PublicKeys: []string{testCert}, + Permissions: []AdminPermissions{{Methods: []string{"GET"}, Paths: []string{"/foo//"}}}, + }, + }, + }, + }, + }, + wantErr: true, + }, } for _, test := range tests {