From d18948f0a2238d04fec49536b44f04f4066d799d Mon Sep 17 00:00:00 2001 From: Mateusz Jankowski Date: Fri, 10 Apr 2026 09:10:00 +0200 Subject: [PATCH] Add denial hint classifier to improve the messaging for metrics with AuthZ denial --- auth/rpcauth/rpcauth.go | 46 +++++++++++++- auth/rpcauth/rpcauth_test.go | 91 +++++++++++++++++++++++++++ cmd/sansshell-server/server/server.go | 14 +++++ server/server.go | 18 +++++- 4 files changed, 167 insertions(+), 2 deletions(-) diff --git a/auth/rpcauth/rpcauth.go b/auth/rpcauth/rpcauth.go index 106c982f..ca7b0947 100644 --- a/auth/rpcauth/rpcauth.go +++ b/auth/rpcauth/rpcauth.go @@ -46,6 +46,29 @@ var ( Description: "number of authorization failure due to policy evaluation error"} ) +// DenialHintClassifier maps denial hint text to a low-cardinality label +// for the denial_hint metric attribute. Classifiers are checked in order; +// the first one whose Matches function returns true wins. +type DenialHintClassifier struct { + // Matches returns true if the joined denial hints belong to this category. + Matches func(joinedHints string) bool + // Label is the low-cardinality value emitted as the denial_hint attribute. + Label string +} + +// classifyDenialHints joins all hints and runs them through the classifiers +// in order. First match wins. Returns defaultDenialHintLabel if none match. +func classifyDenialHints(hints []string, classifiers []DenialHintClassifier) string { + const defaultDenialHintLabel = "other" + joined := strings.Join(hints, "\n") + for _, c := range classifiers { + if c.Matches(joined) { + return c.Label + } + } + return defaultDenialHintLabel +} + type AuthzPolicy interface { Eval(ctx context.Context, input *RPCAuthInput) (bool, error) DenialHints(ctx context.Context, input *RPCAuthInput) ([]string, error) @@ -83,6 +106,10 @@ type rpcAuthorizerImpl struct { // Additional authorization hooks invoked before policy evaluation. hooks []RPCAuthzHook + + // Ordered classifiers for mapping denial hints to metric labels. + // First match wins; if empty or none match, defaultDenialHintLabel is used. + hintClassifiers []DenialHintClassifier } // A RPCAuthzHook is invoked on populated RpcAuthInput prior to policy @@ -104,6 +131,17 @@ func NewRPCAuthorizer(policy AuthzPolicy, authzHooks ...RPCAuthzHook) RPCAuthori } } +// NewRPCAuthorizerWithHintClassifiers is like NewRPCAuthorizer but also +// accepts denial hint classifiers for the denial_hint metric attribute. +// Classifiers are checked in order; first match wins. +func NewRPCAuthorizerWithHintClassifiers(policy AuthzPolicy, authzHooks []RPCAuthzHook, classifiers []DenialHintClassifier) RPCAuthorizer { + return &rpcAuthorizerImpl{ + policy: policy, + hooks: authzHooks, + hintClassifiers: classifiers, + } +} + // Eval will evalulate the supplied input against the authorization policy, returning // nil iff policy evaulation was successful, and the request is permitted, or // an appropriate status.Error otherwise. Any input hooks will be executed @@ -160,7 +198,13 @@ func (g *rpcAuthorizerImpl) Eval(ctx context.Context, input *RPCAuthInput) error } logger.Info("authz policy evaluation result", "authorizationResult", result, "input", redactedInput, "denialHints", hints) if !result { - errRegister := recorder.Counter(ctx, authzDeniedPolicyCounter, 1, attribute.String("method", input.Method)) + deniedAttrs := []attribute.KeyValue{ + attribute.String("method", input.Method), + } + if len(hints) > 0 { + deniedAttrs = append(deniedAttrs, attribute.String("denial_hint", classifyDenialHints(hints, g.hintClassifiers))) + } + errRegister := recorder.Counter(ctx, authzDeniedPolicyCounter, 1, deniedAttrs...) if errRegister != nil { logger.V(1).Error(errRegister, "failed to add counter "+authzDeniedPolicyCounter.Name) } diff --git a/auth/rpcauth/rpcauth_test.go b/auth/rpcauth/rpcauth_test.go index dbbd5d10..9ad569b8 100644 --- a/auth/rpcauth/rpcauth_test.go +++ b/auth/rpcauth/rpcauth_test.go @@ -45,6 +45,49 @@ type IntExtension struct { Value int `json:"value"` } +func TestClassifyDenialHints(t *testing.T) { + classifiers := []DenialHintClassifier{ + {Label: "ip_not_allowed", Matches: func(s string) bool { return strings.Contains(s, "not in the allowed list") }}, + {Label: "no_signed_cert", Matches: func(s string) bool { return strings.Contains(s, "does not yet have a signed certificate") }}, + {Label: "deployment_mismatch", Matches: func(s string) bool { + return strings.Contains(s, "does not match") && strings.Contains(s, "deployment") + }}, + } + + for _, tc := range []struct { + name string + hints []string + want string + }{ + {"single ip hint", []string{"Peer IP 1.2.3.4 is not in the allowed list for this postgres host"}, "ip_not_allowed"}, + {"ip substring", []string{"not in the allowed list"}, "ip_not_allowed"}, + {"single cert hint", []string{"VM does not yet have a signed certificate; only /Os.HostManagement/ RPCs are allowed"}, "no_signed_cert"}, + {"deployment mismatch", []string{"caller deployment does not match host deployment"}, "deployment_mismatch"}, + {"CPS deployment mismatch", []string{"CPS caller deployment 'foo' does not match FDB host deployment parent 'bar'"}, "deployment_mismatch"}, + {"unknown", []string{"some unknown denial reason"}, "other"}, + {"empty hint", []string{""}, "other"}, + {"ip wins over deployment mismatch", []string{"caller deployment does not match host deployment", "not in the allowed list"}, "ip_not_allowed"}, + {"cert wins over deployment mismatch", []string{"caller deployment does not match host deployment", "does not yet have a signed certificate"}, "no_signed_cert"}, + {"deployment mismatch wins over other", []string{"something unknown", "caller deployment does not match host deployment"}, "deployment_mismatch"}, + {"all other", []string{"unknown reason 1", "unknown reason 2"}, "other"}, + {"ip wins when all present", []string{"something unknown", "caller deployment does not match host", "does not yet have a signed certificate", "not in the allowed list"}, "ip_not_allowed"}, + } { + t.Run(tc.name, func(t *testing.T) { + got := classifyDenialHints(tc.hints, classifiers) + if got != tc.want { + t.Errorf("classifyDenialHints(%v) = %q, want %q", tc.hints, got, tc.want) + } + }) + } + + t.Run("no classifiers returns other", func(t *testing.T) { + got := classifyDenialHints([]string{"not in the allowed list"}, nil) + if got != "other" { + t.Errorf("classifyDenialHints with nil classifiers = %q, want %q", got, "other") + } + }) +} + func TestAuthzHook(t *testing.T) { ctx := context.Background() var logs string @@ -398,6 +441,54 @@ func TestAuthzHook(t *testing.T) { } } +func TestEvalDenialHintAttribute(t *testing.T) { + ctx := context.Background() + logger := funcr.NewJSON(func(obj string) {}, funcr.Options{Verbosity: 2}) + ctx = logr.NewContext(ctx, logger) + + for _, tc := range []struct { + name string + hints []string + wantHint string + }{ + { + name: "ip not allowed hint", + hints: []string{"Peer IP 10.0.0.1 is not in the allowed list for this postgres host"}, + wantHint: "ip_not_allowed", + }, + { + name: "no signed cert hint", + hints: []string{"VM does not yet have a signed certificate; only /Os.HostManagement/ RPCs are allowed"}, + wantHint: "no_signed_cert", + }, + { + name: "deployment mismatch hint", + hints: []string{"caller deployment does not match host deployment"}, + wantHint: "deployment_mismatch", + }, + { + name: "unknown hint", + hints: []string{"something unexpected"}, + wantHint: "other", + }, + } { + t.Run(tc.name, func(t *testing.T) { + policy := NewMockAuthzPolicy( + func(_ context.Context, _ *RPCAuthInput) (bool, error) { return false, nil }, + func(_ context.Context, _ *RPCAuthInput) ([]string, error) { return tc.hints, nil }, + ) + authz := NewRPCAuthorizer(policy) + err := authz.Eval(ctx, &RPCAuthInput{Method: "/Test/Method"}) + if status.Code(err) != codes.PermissionDenied { + t.Fatalf("expected PermissionDenied, got %v", err) + } + if !strings.Contains(err.Error(), tc.hints[0]) { + t.Errorf("error %q should contain hint %q", err.Error(), tc.hints[0]) + } + }) + } +} + func TestAuthorize(t *testing.T) { req := &emptypb.Empty{} info := &grpc.UnaryServerInfo{ diff --git a/cmd/sansshell-server/server/server.go b/cmd/sansshell-server/server/server.go index 06c0b790..94e859e7 100644 --- a/cmd/sansshell-server/server/server.go +++ b/cmd/sansshell-server/server/server.go @@ -68,6 +68,7 @@ type runState struct { streamInterceptors []grpc.StreamServerInterceptor statsHandler stats.Handler authzHooks []rpcauth.RPCAuthzHook + hintClassifiers []rpcauth.DenialHintClassifier services []func(*grpc.Server) refreshCredsOnSIGHUP bool @@ -173,6 +174,16 @@ func WithAuthzHook(hook rpcauth.RPCAuthzHook) Option { }) } +// WithDenialHintClassifiers sets the ordered classifiers that map denial +// hint text to low-cardinality metric labels on the authz_denied_policy +// counter. First match wins. If none are set, all denials get label "other". +func WithDenialHintClassifiers(classifiers ...rpcauth.DenialHintClassifier) Option { + return optionFunc(func(_ context.Context, r *runState) error { + r.hintClassifiers = append(r.hintClassifiers, classifiers...) + return nil + }) +} + // WithRawServerOption allows one access to the RPC Server object. Generally this is done to add additional // registration functions for RPC services to be done before starting the server. func WithRawServerOption(s func(*grpc.Server)) Option { @@ -406,6 +417,9 @@ func extractCommonOptionsFromRunState(rs *runState) []server.Option { for _, a := range rs.authzHooks { serverOpts = append(serverOpts, server.WithAuthzHook(a)) } + if len(rs.hintClassifiers) > 0 { + serverOpts = append(serverOpts, server.WithDenialHintClassifiers(rs.hintClassifiers...)) + } for _, u := range rs.unaryInterceptors { serverOpts = append(serverOpts, server.WithUnaryInterceptor(u)) } diff --git a/server/server.go b/server/server.go index cbe75314..79157f82 100644 --- a/server/server.go +++ b/server/server.go @@ -44,6 +44,7 @@ type serveSetup struct { policy rpcauth.AuthzPolicy logger logr.Logger authzHooks []rpcauth.RPCAuthzHook + hintClassifiers []rpcauth.DenialHintClassifier unaryInterceptors []grpc.UnaryServerInterceptor streamInterceptors []grpc.StreamServerInterceptor statsHandler stats.Handler @@ -99,6 +100,16 @@ func WithAuthzHook(hook rpcauth.RPCAuthzHook) Option { }) } +// WithDenialHintClassifiers sets the ordered classifiers that map denial +// hint text to low-cardinality metric labels on the authz_denied_policy +// counter. First match wins. If none are set, all denials get label "other". +func WithDenialHintClassifiers(classifiers ...rpcauth.DenialHintClassifier) Option { + return optionFunc(func(_ context.Context, s *serveSetup) error { + s.hintClassifiers = append(s.hintClassifiers, classifiers...) + return nil + }) +} + // WithUnaryInterceptor adds an additional unary interceptor installed after telemetry and authz. func WithUnaryInterceptor(unary grpc.UnaryServerInterceptor) Option { return optionFunc(func(_ context.Context, s *serveSetup) error { @@ -213,7 +224,12 @@ func BuildServer(opts ...Option) (*grpc.Server, error) { return nil, fmt.Errorf("rpc authorizer was not provided") } - authz := rpcauth.NewRPCAuthorizer(ss.policy, ss.authzHooks...) + var authz rpcauth.RPCAuthorizer + if len(ss.hintClassifiers) > 0 { + authz = rpcauth.NewRPCAuthorizerWithHintClassifiers(ss.policy, ss.authzHooks, ss.hintClassifiers) + } else { + authz = rpcauth.NewRPCAuthorizer(ss.policy, ss.authzHooks...) + } unary := ss.unaryInterceptors unary = append(unary,