diff --git a/docs/content/security.md b/docs/content/security.md index adf54e0f0d..05e46dbd56 100644 --- a/docs/content/security.md +++ b/docs/content/security.md @@ -154,7 +154,8 @@ packages however it is recommended that administrators keep the policy under the If the document produced by the ``allow`` rule is ``true``, the request is processed normally. If the document is undefined or **not** ``true``, the -request is rejected immediately. +request is rejected immediately. The count of requests rejected by an OPA instance +are surfaced via the performance metrics in the [Status](../management-status) information. OPA provides the following `input` document when executing the authorization policy: diff --git a/server/server.go b/server/server.go index 7ea9c3398d..15a4c8a6ca 100644 --- a/server/server.go +++ b/server/server.go @@ -95,6 +95,7 @@ const ( PromHandlerIndex = "index" PromHandlerCatch = "catchall" PromHandlerHealth = "health" + PromHandlerAPIAuthz = "authz" ) const pqMaxCacheSize = 100 @@ -183,18 +184,16 @@ func (s *Server) Init(ctx context.Context) (*Server, error) { s.partials = map[string]rego.PartialResult{} s.preparedEvalQueries = newCache(pqMaxCacheSize) s.defaultDecisionPath = s.generateDefaultDecisionPath() - s.interQueryBuiltinCache = iCache.NewInterQueryCache(s.manager.InterQueryBuiltinCacheConfig()) - s.manager.RegisterCacheTrigger(s.updateCacheConfig) s.manager.RegisterNDCacheTrigger(s.updateNDCache) - // authorizer, if configured, needs the iCache to be set up already - s.Handler = s.initHandlerAuth(s.Handler) + s.Handler = s.initHandlerAuthn(s.Handler) + // compression handler s.Handler, err = s.initHandlerCompression(s.Handler) if err != nil { return nil, err } - s.DiagnosticHandler = s.initHandlerAuth(s.DiagnosticHandler) + s.DiagnosticHandler = s.initHandlerAuthn(s.DiagnosticHandler) return s, s.store.Commit(ctx, txn) } @@ -641,9 +640,18 @@ func (s *Server) getListenerForUNIXSocket(u *url.URL, h http.Handler, t httpList return domainSocketLoop, l, nil } -func (s *Server) initHandlerAuth(handler http.Handler) http.Handler { - // Add authorization handler. This must come BEFORE authentication handler - // so that the latter can run first. +func (s *Server) initHandlerAuthn(handler http.Handler) http.Handler { + switch s.authentication { + case AuthenticationToken: + handler = identifier.NewTokenBased(handler) + case AuthenticationTLS: + handler = identifier.NewTLSBased(handler) + } + + return handler +} + +func (s *Server) initHandlerAuthz(handler http.Handler) http.Handler { switch s.authorization { case AuthorizationBasic: handler = authorizer.NewBasic( @@ -655,13 +663,10 @@ func (s *Server) initHandlerAuth(handler http.Handler) http.Handler { authorizer.PrintHook(s.manager.PrintHook()), authorizer.EnablePrintStatements(s.manager.EnablePrintStatements()), authorizer.InterQueryCache(s.interQueryBuiltinCache)) - } - switch s.authentication { - case AuthenticationToken: - handler = identifier.NewTokenBased(handler) - case AuthenticationTLS: - handler = identifier.NewTLSBased(handler) + if s.metrics != nil { + handler = s.instrumentHandler(handler.ServeHTTP, PromHandlerAPIAuthz) + } } return handler @@ -690,6 +695,16 @@ func (s *Server) initRouters() { diagRouter := mux.NewRouter() + // authorizer, if configured, needs the iCache to be set up already + s.interQueryBuiltinCache = iCache.NewInterQueryCache(s.manager.InterQueryBuiltinCacheConfig()) + s.manager.RegisterCacheTrigger(s.updateCacheConfig) + + // Add authorization handler. This must come BEFORE authentication handler + // so that the latter can run first. + handlerAuthz := s.initHandlerAuthz(mainRouter) + + handlerAuthzDiag := s.initHandlerAuthz(diagRouter) + // All routers get the same base configuration *and* diagnostic API's for _, router := range []*mux.Router{mainRouter, diagRouter} { router.StrictSlash(true) @@ -771,6 +786,10 @@ func (s *Server) initRouters() { s.Handler = mainRouter s.DiagnosticHandler = diagRouter + + // Add authorization handler in the end so that it can run first + s.Handler = handlerAuthz + s.DiagnosticHandler = handlerAuthzDiag } func (s *Server) instrumentHandler(handler func(http.ResponseWriter, *http.Request), label string) http.Handler { diff --git a/server/server_test.go b/server/server_test.go index a9bf0391b4..3335b2601a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/gorilla/mux" + "github.com/open-policy-agent/opa/internal/prometheus" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/propagation" @@ -3348,6 +3349,166 @@ func TestStatusV1(t *testing.T) { } } +func TestStatusV1MetricsWithSystemAuthzPolicy(t *testing.T) { + + ctx := context.Background() + + // Add the authz policy + store := inmem.New() + txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams) + authzPolicy := `package system.authz + default allow = false + allow { + input.path = ["v1", "status"] + }` + + if err := store.UpsertPolicy(ctx, txn, "test", []byte(authzPolicy)); err != nil { + t.Fatal(err) + } + + if err := store.Commit(ctx, txn); err != nil { + t.Fatal(err) + } + + // Add Prometheus Registerer to be used by plugins + inner := metrics.New() + + logger := func(logger logging.Logger) func(attrs map[string]interface{}, f string, a ...interface{}) { + return func(attrs map[string]interface{}, f string, a ...interface{}) { + logger.WithFields(attrs).Error(f, a...) + } + }(logging.NewNoOpLogger()) + + prom := prometheus.New(inner, logger) + serverOpts := []func(s *Server){func(s *Server) { s.WithAuthorization(AuthorizationBasic) }, func(s *Server) { s.WithMetrics(prom) }} + + f := newFixtureWithStore(t, store, serverOpts...) + + // Expect HTTP 500 before status plugin is registered + req := newReqV1(http.MethodGet, "/status", "") + f.server.Handler.ServeHTTP(f.recorder, req) + + if f.recorder.Result().StatusCode != http.StatusInternalServerError { + t.Fatal("expected internal error") + } + + // Register Status plugin + manual := plugins.TriggerManual + bs := pluginStatus.New(&pluginStatus.Config{Trigger: &manual, Prometheus: true}, f.server.manager).WithMetrics(prom) + err := bs.Start(context.Background()) + if err != nil { + t.Fatal(err) + } + f.server.manager.Register(pluginStatus.Name, bs) + + // Fetch the status info + req = newReqV1(http.MethodGet, "/status", "") + f.reset() + f.server.Handler.ServeHTTP(f.recorder, req) + if f.recorder.Result().StatusCode != http.StatusOK { + t.Fatal("expected ok") + } + + var resp1 struct { + Result struct { + Plugins struct { + Status struct { + State string + } + } + } + } + if err := util.NewJSONDecoder(f.recorder.Body).Decode(&resp1); err != nil { + t.Fatal(err) + } else if resp1.Result.Plugins.Status.State != "OK" { + t.Fatal("expected plugin state for status to be 'OK' but got:", resp1) + } + + // Make requests that should get denied + req = newReqV1(http.MethodGet, "/policies", "") + f.reset() + f.server.Handler.ServeHTTP(f.recorder, req) + + if f.recorder.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("Expected success but got %v", f.recorder) + } + + req = newReqV1(http.MethodGet, "/data", "") + f.reset() + f.server.Handler.ServeHTTP(f.recorder, req) + + if f.recorder.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("Expected success but got %v", f.recorder) + } + + // Check Prometheus status metrics in the Status API + + req = newReqV1(http.MethodGet, "/status", "") + f.reset() + f.server.Handler.ServeHTTP(f.recorder, req) + + if f.recorder.Result().StatusCode != http.StatusOK { + t.Fatal("expected ok") + } + + var resp struct { + Result struct { + Plugins struct { + Status struct { + State string + } + } + Metrics map[string]interface{} + } + } + if err := util.NewJSONDecoder(f.recorder.Body).Decode(&resp); err != nil { + t.Fatal(err) + } else if resp.Result.Plugins.Status.State != "OK" { + t.Fatal("expected plugin state for status to be 'OK' but got:", resp) + } + + met, ok := resp.Result.Metrics["prometheus"] + if !ok { + t.Fatal("expected prometheus metrics to be present in status") + } + + promMet, ok := met.(map[string]interface{}) + if !ok { + t.Fatal("expected prometheus metrics to be a map") + } + + httpMet, ok := promMet["http_request_duration_seconds"].(map[string]interface{}) + if !ok { + t.Fatal("expected http_request_duration_seconds metric to be a map") + } + + innerMet, ok := httpMet["metric"].([]interface{}) + if !ok { + t.Fatal("expected http_request_duration_seconds histogram metric to be a list") + } + + expected := []interface{}{map[string]interface{}{"name": "code", "value": "401"}, + map[string]interface{}{"name": "handler", "value": "authz"}, + map[string]interface{}{"name": "method", "value": "get"}} + + found := false + for _, m := range innerMet { + item, ok := m.(map[string]interface{}) + if ok { + if reflect.DeepEqual(item["label"].([]interface{}), expected) { + found = true + break + } + } else { + t.Fatal("expected each http_request_duration_seconds histogram metric element to be a map") + } + } + + if !found { + t.Fatalf("expected to find metrics %v but found no match", expected) + } +} + func TestQueryPostBasic(t *testing.T) { f := newFixture(t) f.server, _ = New(). diff --git a/test/e2e/distributedtracing/distributedtracing_test.go b/test/e2e/distributedtracing/distributedtracing_test.go index 73d795b45a..4c0b60f0c2 100644 --- a/test/e2e/distributedtracing/distributedtracing_test.go +++ b/test/e2e/distributedtracing/distributedtracing_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "github.com/open-policy-agent/opa/server" "github.com/open-policy-agent/opa/test/e2e" "github.com/open-policy-agent/opa/tracing" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -329,6 +330,80 @@ func TestClientSpan(t *testing.T) { }) } +func TestServerSpanWithSystemAuthzPolicy(t *testing.T) { + + // setup + spanExp := tracetest.NewInMemoryExporter() + options := tracing.NewOptions( + otelhttp.WithTracerProvider(trace.NewTracerProvider(trace.WithSpanProcessor(trace.NewSimpleSpanProcessor(spanExp)))), + ) + + authzPolicy := []byte(`package system.authz +default allow = false +allow { + input.path = ["health"] +}`) + + tmpfile, err := os.CreateTemp("", "authz.*.rego") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpfile.Name()) + + if _, err := tmpfile.Write(authzPolicy); err != nil { + t.Fatal(err) + } + if err := tmpfile.Close(); err != nil { + t.Fatal(err) + } + + testServerParams := e2e.NewAPIServerTestParams() + testServerParams.DistributedTracingOpts = options + testServerParams.Authorization = server.AuthorizationBasic + testServerParams.Paths = []string{"system.authz:" + tmpfile.Name()} + + e2e.WithRuntime(t, e2e.TestRuntimeOpts{}, testServerParams, func(rt *e2e.TestRuntime) { + + spanExp.Reset() + + mr, err := http.Post(rt.URL()+"/v1/data", "application/json", nil) + if err != nil { + t.Fatal(err) + } + defer mr.Body.Close() + + if mr.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected status %v but got %v", http.StatusUnauthorized, mr.StatusCode) + } + + spans := spanExp.GetSpans() + if got, expected := len(spans), 1; got != expected { + t.Fatalf("got %d span(s), expected %d", got, expected) + } + if !spans[0].SpanContext.IsValid() { + t.Fatalf("invalid span created: %#v", spans[0].SpanContext) + } + + if got, expected := spans[0].SpanKind.String(), "server"; got != expected { + t.Fatalf("Expected span kind to be %q but got %q", expected, got) + } + + expected := []attribute.KeyValue{ + attribute.String("http.host", strings.Replace(rt.URL(), "http://", "", 1)), + attribute.String("http.method", "POST"), + attribute.String("http.scheme", "http"), + attribute.String("http.server_name", server.PromHandlerAPIAuthz), + attribute.Int("http.status_code", 401), + attribute.String("http.target", "/v1/data"), + attribute.String("http.user_agent", "Go-http-client/1.1"), + attribute.Int("http.wrote_bytes", 87), + attribute.String("net.transport", "ip_tcp"), + } + compareSpanAttributes(t, expected, attribute.NewSet(spans[0].Attributes...)) + + }) +} + func compareSpanAttributes(t *testing.T, expectedAttributes []attribute.KeyValue, spanAttributes attribute.Set) { t.Helper() for _, exp := range expectedAttributes {