diff --git a/CHANGELOG.md b/CHANGELOG.md index 322f39173..427b4104e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ Unreleased changes are available as `avenga/couper:edge` container. * **Changed** * Automatically add the `private` directive to the response `Cache-Control` HTTP header field value for all resources protected by [JWT](./docs/REFERENCE.md#jwt-block) ([#418](https://github.com/avenga/couper/pull/418)) +* **Fixed** + * improved protection against sniffing using unauthorized requests with non-standard method to non-existant endpoints in protected API ([#441](https://github.com/avenga/couper/pull/441)) + --- ## [1.7.2](https://github.com/avenga/couper/releases/tag/v1.7.2) diff --git a/config/runtime/server.go b/config/runtime/server.go index dfde2f6b7..98617837f 100644 --- a/config/runtime/server.go +++ b/config/runtime/server.go @@ -282,9 +282,9 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca } epOpts.LogHandlerKind = kind.String() - var epHandler http.Handler + var epHandler, protectedHandler http.Handler if parentAPI != nil && parentAPI.CatchAllEndpoint == endpointConf { - epHandler = epOpts.ErrorTemplate.WithError(errors.RouteNotFound) + protectedHandler = epOpts.ErrorTemplate.WithError(errors.RouteNotFound) } else { epErrorHandler, err := newErrorHandler(confCtx, &protectedOptions{ epOpts: epOpts, @@ -299,32 +299,27 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca epOpts.ErrorHandler = epErrorHandler } epHandler = handler.NewEndpoint(epOpts, log, modifier) - } - - scopeMaps, err := newScopeMaps(parentAPI, endpointConf) - if err != nil { - return nil, err - } - scopeControl := ac.NewScopeControl(scopeMaps) - scopeErrorHandler, err := newErrorHandler(confCtx, &protectedOptions{ - epOpts: epOpts, - memStore: memStore, - proxyFromEnv: conf.Settings.NoProxyFromEnv, - srvOpts: serverOptions, - }, log, errorHandlerDefinitions, "api", "endpoint") - if err != nil { - return nil, err - } + scopeMaps, err := newScopeMaps(parentAPI, endpointConf) + if err != nil { + return nil, err + } - var protectedHandler http.Handler - protectedHandler = middleware.NewErrorHandler(scopeControl.Validate, scopeErrorHandler)(epHandler) + scopeControl := ac.NewScopeControl(scopeMaps) + scopeErrorHandler, err := newErrorHandler(confCtx, &protectedOptions{ + epOpts: epOpts, + memStore: memStore, + proxyFromEnv: conf.Settings.NoProxyFromEnv, + srvOpts: serverOptions, + }, log, errorHandlerDefinitions, "api", "endpoint") + if err != nil { + return nil, err + } - accessControl := newAC(srvConf, parentAPI) - if parentAPI != nil && parentAPI.CatchAllEndpoint == endpointConf { - protectedHandler = epOpts.ErrorTemplate.WithError(errors.RouteNotFound) + protectedHandler = middleware.NewErrorHandler(scopeControl.Validate, scopeErrorHandler)(epHandler) } + accessControl := newAC(srvConf, parentAPI) epHandler, err = configureProtectedHandler(accessControls, confCtx, accessControl, config.NewAccessControl(endpointConf.AccessControl, endpointConf.DisableAccessControl), &protectedOptions{ diff --git a/server/http_integration_test.go b/server/http_integration_test.go index 26a482044..f89c8793b 100644 --- a/server/http_integration_test.go +++ b/server/http_integration_test.go @@ -3045,7 +3045,6 @@ func TestConfigBodyContentAccessControl(t *testing.T) { {"/v2", http.Header{}, http.StatusUnauthorized, "application/json", "access control error: ba1: credentials required"}, {"/v3", http.Header{}, http.StatusOK, "application/json", ""}, {"/status", http.Header{}, http.StatusOK, "application/json", ""}, - {"/v5/not-exist", http.Header{}, http.StatusUnauthorized, "application/json", "access control error: ba1: credentials required"}, {"/superadmin", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1", "ba4"}}, http.StatusOK, "application/json", ""}, {"/superadmin", http.Header{}, http.StatusUnauthorized, "application/json", "access control error: ba1: credentials required"}, {"/ba5", http.Header{"Authorization": []string{"Basic VVNSOlBXRA=="}, "X-Ba-User": []string{"USR"}}, http.StatusOK, "application/json", ""}, @@ -3111,6 +3110,61 @@ func TestConfigBodyContentAccessControl(t *testing.T) { } } +func TestAPICatchAll(t *testing.T) { + client := newClient() + + shutdown, hook := newCouper("testdata/integration/config/03_couper.hcl", test.New(t)) + defer shutdown() + + type testCase struct { + name string + path string + method string + header http.Header + status int + wantErrLog string + } + + for _, tc := range []testCase{ + {"exists, authorized", "/v5/exists", http.MethodGet, http.Header{"Authorization": []string{"Basic OmFzZGY="}}, http.StatusOK, ""}, + {"exists, unauthorized", "/v5/exists", http.MethodGet, http.Header{}, http.StatusUnauthorized, "access control error: ba1: credentials required"}, + {"exists, CORS pre-flight", "/v5/exists", http.MethodOptions, http.Header{"Origin": []string{"https://www.example.com"}, "Access-Control-Request-Method": []string{"POST"}}, http.StatusNoContent, ""}, + {"not-exist, authorized", "/v5/not-exist", http.MethodGet, http.Header{"Authorization": []string{"Basic OmFzZGY="}}, http.StatusNotFound, "route not found error"}, + {"not-exist, unauthorized", "/v5/not-exist", http.MethodGet, http.Header{}, http.StatusUnauthorized, "access control error: ba1: credentials required"}, + {"not-exist, non-standard method, authorized", "/v5/not-exist", "BREW", http.Header{"Authorization": []string{"Basic OmFzZGY="}}, http.StatusNotFound, "route not found error"}, + {"not-exist, non-standard method, unauthorized", "/v5/not-exist", "BREW", http.Header{}, http.StatusUnauthorized, "access control error: ba1: credentials required"}, + {"not-exist, CORS pre-flight", "/v5/not-exist", http.MethodOptions, http.Header{"Origin": []string{"https://www.example.com"}, "Access-Control-Request-Method": []string{"POST"}}, http.StatusNoContent, ""}, + } { + t.Run(tc.name, func(subT *testing.T) { + helper := test.New(subT) + hook.Reset() + + req, err := http.NewRequest(tc.method, "http://back.end:8080"+tc.path, nil) + helper.Must(err) + + req.Header = tc.header + + res, err := client.Do(req) + helper.Must(err) + + message := getAccessControlMessages(hook) + if tc.wantErrLog == "" { + if message != "" { + subT.Errorf("Expected error log: %q, actual: %#v", tc.wantErrLog, message) + } + } else { + if message != tc.wantErrLog { + subT.Errorf("Expected error log message: %q, actual: %#v", tc.wantErrLog, message) + } + } + + if res.StatusCode != tc.status { + subT.Fatalf("%q: expected Status %d, got: %d", tc.path, tc.status, res.StatusCode) + } + }) + } +} + func Test_LoadAccessControl(t *testing.T) { // Tests the config load with ACs and "error_handler" blocks... shutdown, _ := newCouper("testdata/integration/config/07_couper.hcl", test.New(t)) diff --git a/server/mux.go b/server/mux.go index 0334bf0e2..41bfa9427 100644 --- a/server/mux.go +++ b/server/mux.go @@ -45,7 +45,11 @@ var fileMethods = []string{ http.MethodOptions, } -const serverOptionsKey = "serverContextOptions" +const ( + serverOptionsKey = "serverContextOptions" + wildcardReplacement = "/{_couper_wildcardMatch*}" + wildcardSearch = "/**" +) func NewMux(options *runtime.MuxOptions) *Mux { opts := options @@ -63,15 +67,15 @@ func NewMux(options *runtime.MuxOptions) *Mux { for path, h := range opts.EndpointRoutes { // TODO: handle method option per endpoint configuration - mux.mustAddRoute(mux.endpointRoot, allowedMethods, path, h) + mux.mustAddRoute(mux.endpointRoot, allowedMethods, path, h, true) } for path, h := range opts.FileRoutes { - mux.mustAddRoute(mux.fileRoot, fileMethods, utils.JoinPath(path, "/**"), h) + mux.mustAddRoute(mux.fileRoot, fileMethods, utils.JoinPath(path, "/**"), h, false) } for path, h := range opts.SPARoutes { - mux.mustAddRoute(mux.spaRoot, fileMethods, path, h) + mux.mustAddRoute(mux.spaRoot, fileMethods, path, h, false) } return mux @@ -94,41 +98,19 @@ func (m *Mux) MustAddRoute(method, path string, handler http.Handler) *Mux { methods = []string{um} } - return m.mustAddRoute(m.endpointRoot, methods, path, handler) + return m.mustAddRoute(m.endpointRoot, methods, path, handler, false) } -func (m *Mux) mustAddRoute(root *pathpattern.Node, methods []string, path string, handler http.Handler) *Mux { - const wildcardReplacement = "/{_couper_wildcardMatch*}" - const wildcardSearch = "/**" - - // TODO: Unique Options per method if configurable later on - pathOptions := &pathpattern.Options{} +func (m *Mux) mustAddRoute(root *pathpattern.Node, methods []string, path string, handler http.Handler, forEndpoint bool) *Mux { + if forEndpoint && strings.HasSuffix(path, wildcardSearch) { + route := mustCreateNode(root, handler, "", path) + m.handler[route] = handler + return m + } for _, method := range methods { - if strings.HasSuffix(path, wildcardSearch) { - pathOptions.SupportWildcard = true - path = path[:len(path)-len(wildcardSearch)] + wildcardReplacement - } - - node, err := root.CreateNode(method+" "+path, pathOptions) - if err != nil { - panic(fmt.Errorf("create path node failed: %s %q: %v", method, path, err)) - } - - var serverOpts *server.Options - if optsHandler, ok := handler.(server.Context); ok { - serverOpts = optsHandler.Options() - } - - node.Value = &routers.Route{ - Method: method, - Path: path, - Server: &openapi3.Server{Variables: map[string]*openapi3.ServerVariable{ - serverOptionsKey: {Default: fmt.Sprintf("%#v", serverOpts)}, - }}, - } - - m.handler[node.Value.(*routers.Route)] = handler + route := mustCreateNode(root, handler, method, path) + m.handler[route] = handler } return m @@ -138,6 +120,9 @@ func (m *Mux) FindHandler(req *http.Request) http.Handler { var route *routers.Route node, paramValues := m.match(m.endpointRoot, req) + if node == nil { + node, paramValues = m.matchWithoutMethod(m.endpointRoot, req) + } if node == nil { // No matches for api or free endpoints. Determine if we have entered an api basePath // and handle api related errors accordingly. @@ -193,6 +178,12 @@ func (m *Mux) match(root *pathpattern.Node, req *http.Request) (*pathpattern.Nod return root.Match(req.Method + " " + req.URL.Path) } +func (m *Mux) matchWithoutMethod(root *pathpattern.Node, req *http.Request) (*pathpattern.Node, []string) { + *req = *req.WithContext(context.WithValue(req.Context(), request.ServerName, m.opts.ServerOptions.ServerName)) + + return root.Match(req.URL.Path) +} + func (m *Mux) hasFileResponse(req *http.Request) (http.Handler, bool) { node, _ := m.match(m.fileRoot, req) if node == nil { @@ -227,6 +218,40 @@ func (m *Mux) getAPIErrorTemplate(reqPath string) (*errors.Template, *config.API return nil, nil } +func mustCreateNode(root *pathpattern.Node, handler http.Handler, method, path string) *routers.Route { + pathOptions := &pathpattern.Options{} + + if strings.HasSuffix(path, wildcardSearch) { + pathOptions.SupportWildcard = true + path = path[:len(path)-len(wildcardSearch)] + wildcardReplacement + } + + nodePath := path + if method != "" { + nodePath = method + " " + path + } + + node, err := root.CreateNode(nodePath, pathOptions) + if err != nil { + panic(fmt.Errorf("create path node failed: %s %q: %v", method, path, err)) + } + + var serverOpts *server.Options + if optsHandler, ok := handler.(server.Context); ok { + serverOpts = optsHandler.Options() + } + + node.Value = &routers.Route{ + Method: method, + Path: path, + Server: &openapi3.Server{Variables: map[string]*openapi3.ServerVariable{ + serverOptionsKey: {Default: fmt.Sprintf("%#v", serverOpts)}, + }}, + } + + return node.Value.(*routers.Route) +} + // isAPIError checks the path w/ and w/o the // trailing slash against the request path. func isAPIError(apiPath, filesBasePath, spaBasePath, reqPath string) bool { diff --git a/server/testdata/integration/config/03_couper.hcl b/server/testdata/integration/config/03_couper.hcl index ee11355e5..815cabcca 100644 --- a/server/testdata/integration/config/03_couper.hcl +++ b/server/testdata/integration/config/03_couper.hcl @@ -57,10 +57,12 @@ server "acs" { api { base_path = "/v5" access_control = ["ba2"] + cors { + allowed_origins = ["*"] + } endpoint "/exists" { - error_file = "../server_error.html" # error_file in endpoint - proxy { - backend = "test" + response { + body = "exists" } } }