diff --git a/authz/grpc_authz_server_interceptors.go b/authz/grpc_authz_server_interceptors.go index ab93af13f37..3e5f598a97d 100644 --- a/authz/grpc_authz_server_interceptors.go +++ b/authz/grpc_authz_server_interceptors.go @@ -44,11 +44,11 @@ type StaticInterceptor struct { // NewStatic returns a new StaticInterceptor from a static authorization policy // JSON string. func NewStatic(authzPolicy string) (*StaticInterceptor, error) { - rbacs, err := translatePolicy(authzPolicy) + rbacs, policyName, err := translatePolicy(authzPolicy) if err != nil { return nil, err } - chainEngine, err := rbac.NewChainEngine(rbacs) + chainEngine, err := rbac.NewChainEngine(rbacs, policyName) if err != nil { return nil, err } diff --git a/authz/rbac_translator.go b/authz/rbac_translator.go index ce5c15cb976..d88797d4990 100644 --- a/authz/rbac_translator.go +++ b/authz/rbac_translator.go @@ -39,7 +39,7 @@ import ( // This is used when converting a custom config from raw JSON to a TypedStruct // The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/" -const typedURLPrefix = "grpc.authz.audit_logging/" +const typeURLPrefix = "grpc.authz.audit_logging/" type header struct { Key string @@ -62,14 +62,14 @@ type rule struct { } type auditLogger struct { - Name string `json:"name"` - Config *structpb.Struct `json:"config"` - IsOptional bool `json:"is_optional"` + Name string `json:"name"` + Config structpb.Struct `json:"config"` + IsOptional bool `json:"is_optional"` } type auditLoggingOptions struct { - AuditCondition string `json:"audit_condition"` - AuditLoggers []auditLogger `json:"audit_loggers"` + AuditCondition string `json:"audit_condition"` + AuditLoggers []*auditLogger `json:"audit_loggers"` } // Represents the SDK authorization policy provided by user. @@ -302,14 +302,13 @@ func (options *auditLoggingOptions) toProtos() (allow *v3rbacpb.RBAC_AuditLoggin deny.AuditCondition = toDenyCondition(v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition(rbacCondition)) } - for i := range options.AuditLoggers { - config := &options.AuditLoggers[i] - if config.Config == nil { - return nil, nil, fmt.Errorf("AuditLogger Config field cannot be nil") + for i, config := range options.AuditLoggers { + if config.Name == "" { + return nil, nil, fmt.Errorf("missing required field: name in audit_logging_options.audit_loggers[%v]", i) } typedStruct := &v1xdsudpatypepb.TypedStruct{ - TypeUrl: typedURLPrefix + config.Name, - Value: config.Config, + TypeUrl: typeURLPrefix + config.Name, + Value: &config.Config, } customConfig, err := anypb.New(typedStruct) if err != nil { @@ -355,30 +354,30 @@ func toDenyCondition(condition v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition) // translatePolicy translates SDK authorization policy in JSON format to two // Envoy RBAC polices (deny followed by allow policy) or only one Envoy RBAC -// allow policy. If the input policy cannot be parsed or is invalid, an error -// will be returned. -func translatePolicy(policyStr string) ([]*v3rbacpb.RBAC, error) { +// allow policy. Also returns the overall policy name. If the input policy +// cannot be parsed or is invalid, an error will be returned. +func translatePolicy(policyStr string) ([]*v3rbacpb.RBAC, string, error) { policy := &authorizationPolicy{} d := json.NewDecoder(bytes.NewReader([]byte(policyStr))) d.DisallowUnknownFields() if err := d.Decode(policy); err != nil { - return nil, fmt.Errorf("failed to unmarshal policy: %v", err) + return nil, "", fmt.Errorf("failed to unmarshal policy: %v", err) } if policy.Name == "" { - return nil, fmt.Errorf(`"name" is not present`) + return nil, "", fmt.Errorf(`"name" is not present`) } if len(policy.AllowRules) == 0 { - return nil, fmt.Errorf(`"allow_rules" is not present`) + return nil, "", fmt.Errorf(`"allow_rules" is not present`) } allowLogger, denyLogger, err := policy.AuditLoggingOptions.toProtos() if err != nil { - return nil, err + return nil, "", err } rbacs := make([]*v3rbacpb.RBAC, 0, 2) if len(policy.DenyRules) > 0 { denyPolicies, err := parseRules(policy.DenyRules, policy.Name) if err != nil { - return nil, fmt.Errorf(`"deny_rules" %v`, err) + return nil, "", fmt.Errorf(`"deny_rules" %v`, err) } denyRBAC := &v3rbacpb.RBAC{ Action: v3rbacpb.RBAC_DENY, @@ -389,8 +388,8 @@ func translatePolicy(policyStr string) ([]*v3rbacpb.RBAC, error) { } allowPolicies, err := parseRules(policy.AllowRules, policy.Name) if err != nil { - return nil, fmt.Errorf(`"allow_rules" %v`, err) + return nil, "", fmt.Errorf(`"allow_rules" %v`, err) } allowRBAC := &v3rbacpb.RBAC{Action: v3rbacpb.RBAC_ALLOW, Policies: allowPolicies, AuditLoggingOptions: allowLogger} - return append(rbacs, allowRBAC), nil + return append(rbacs, allowRBAC), policy.Name, nil } diff --git a/authz/rbac_translator_test.go b/authz/rbac_translator_test.go index fed0ef5c9d3..23b6fb669e9 100644 --- a/authz/rbac_translator_test.go +++ b/authz/rbac_translator_test.go @@ -36,9 +36,10 @@ import ( func TestTranslatePolicy(t *testing.T) { tests := map[string]struct { - authzPolicy string - wantErr string - wantPolicies []*v3rbacpb.RBAC + authzPolicy string + wantErr string + wantPolicies []*v3rbacpb.RBAC + wantPolicyName string }{ "valid policy": { authzPolicy: `{ @@ -210,6 +211,7 @@ func TestTranslatePolicy(t *testing.T) { AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{}, }, }, + wantPolicyName: "authz", }, "allow authenticated": { authzPolicy: `{ @@ -798,6 +800,101 @@ func TestTranslatePolicy(t *testing.T) { }, }, }, + "missing custom config audit logger": { + authzPolicy: `{ + "name": "authz", + "allow_rules": [ + { + "name": "allow_authenticated", + "source": { + "principals":["*", ""] + } + }], + "deny_rules": [ + { + "name": "deny_policy_1", + "source": { + "principals":[ + "spiffe://foo.abc" + ] + } + }], + "audit_logging_options": { + "audit_condition": "ON_DENY", + "audit_loggers": [ + { + "name": "stdout_logger", + "is_optional": false + } + ] + } + }`, + wantPolicies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_DENY, + Policies: map[string]*v3rbacpb.Policy{ + "authz_deny_policy_1": { + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_OrIds{OrIds: &v3rbacpb.Principal_Set{ + Ids: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Authenticated_{ + Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{ + MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "spiffe://foo.abc"}, + }}, + }}, + }, + }}}, + }, + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{Name: "stdout_logger", TypedConfig: anyPbHelper(t, map[string]interface{}{}, "stdout_logger")}, + IsOptional: false, + }, + }, + }, + }, + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "authz_allow_authenticated": { + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_OrIds{OrIds: &v3rbacpb.Principal_Set{ + Ids: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Authenticated_{ + Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{ + MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{SafeRegex: &v3matcherpb.RegexMatcher{Regex: ".+"}}, + }}, + }}, + {Identifier: &v3rbacpb.Principal_Authenticated_{ + Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{ + MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: ""}, + }}, + }}, + }, + }}}, + }, + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{Name: "stdout_logger", TypedConfig: anyPbHelper(t, map[string]interface{}{}, "stdout_logger")}, + IsOptional: false, + }, + }, + }, + }, + }, + }, "unknown field": { authzPolicy: `{"random": 123}`, wantErr: "failed to unmarshal policy", @@ -897,7 +994,7 @@ func TestTranslatePolicy(t *testing.T) { }`, wantErr: `failed to unmarshal policy`, }, - "missing custom config audit logger": { + "missing audit logger name": { authzPolicy: `{ "name": "authz", "allow_rules": [ @@ -907,37 +1004,32 @@ func TestTranslatePolicy(t *testing.T) { "principals":["*", ""] } }], - "deny_rules": [ - { - "name": "deny_policy_1", - "source": { - "principals":[ - "spiffe://foo.abc" - ] - } - }], "audit_logging_options": { - "audit_condition": "ON_DENY", + "audit_condition": "NONE", "audit_loggers": [ { - "name": "stdout_logger", + "name": "", + "config": {}, "is_optional": false } ] } }`, - wantErr: "AuditLogger Config field cannot be nil", + wantErr: `missing required field: name`, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - gotPolicies, gotErr := translatePolicy(test.authzPolicy) + gotPolicies, gotPolicyName, gotErr := translatePolicy(test.authzPolicy) if gotErr != nil && !strings.HasPrefix(gotErr.Error(), test.wantErr) { t.Fatalf("unexpected error\nwant:%v\ngot:%v", test.wantErr, gotErr) } if diff := cmp.Diff(gotPolicies, test.wantPolicies, protocmp.Transform()); diff != "" { t.Fatalf("unexpected policy\ndiff (-want +got):\n%s", diff) } + if test.wantPolicyName != "" && gotPolicyName != test.wantPolicyName { + t.Fatalf("unexpected policy name\nwant:%v\ngot:%v", test.wantPolicyName, gotPolicyName) + } }) } } @@ -946,7 +1038,7 @@ func anyPbHelper(t *testing.T, in map[string]interface{}, name string) *anypb.An t.Helper() pb, err := structpb.NewStruct(in) typedStruct := &v1xdsudpatypepb.TypedStruct{ - TypeUrl: typedURLPrefix + name, + TypeUrl: typeURLPrefix + name, Value: pb, } if err != nil { diff --git a/internal/xds/rbac/converter.go b/internal/xds/rbac/converter.go new file mode 100644 index 00000000000..db22fd5a9e0 --- /dev/null +++ b/internal/xds/rbac/converter.go @@ -0,0 +1,98 @@ +/* + * Copyright 2023 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package rbac + +import ( + "encoding/json" + "fmt" + "strings" + + v1xdsudpatypepb "github.com/cncf/xds/go/udpa/type/v1" + v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3" + v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" + "google.golang.org/grpc/authz/audit" + "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/structpb" +) + +const udpaTypedStuctType = "type.googleapis.com/udpa.type.v1.TypedStruct" +const xdsTypedStuctType = "type.googleapis.com/xds.type.v3.TypedStruct" + +func buildLogger(loggerConfig *v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig) (audit.Logger, error) { + if loggerConfig.GetAuditLogger().GetTypedConfig() == nil { + return nil, fmt.Errorf("missing required field: TypedConfig") + } + customConfig, loggerName, err := getCustomConfig(loggerConfig.AuditLogger.TypedConfig) + if err != nil { + return nil, err + } + if loggerName == "" { + return nil, fmt.Errorf("field TypedConfig.TypeURL cannot be an empty string") + } + factory := audit.GetLoggerBuilder(loggerName) + if factory == nil { + if loggerConfig.IsOptional { + return nil, nil + } + return nil, fmt.Errorf("no builder registered for %v", loggerName) + } + auditLoggerConfig, err := factory.ParseLoggerConfig(customConfig) + if err != nil { + return nil, fmt.Errorf("custom config could not be parsed by registered factory. error: %v", err) + } + auditLogger := factory.Build(auditLoggerConfig) + return auditLogger, nil +} + +func getCustomConfig(config *anypb.Any) (json.RawMessage, string, error) { + switch config.GetTypeUrl() { + case udpaTypedStuctType: + typedStruct := &v1xdsudpatypepb.TypedStruct{} + if err := config.UnmarshalTo(typedStruct); err != nil { + return nil, "", fmt.Errorf("failed to unmarshal resource: %v", err) + } + return convertCustomConfig(typedStruct.TypeUrl, typedStruct.Value) + case xdsTypedStuctType: + typedStruct := &v3xdsxdstypepb.TypedStruct{} + if err := config.UnmarshalTo(typedStruct); err != nil { + return nil, "", fmt.Errorf("failed to unmarshal resource: %v", err) + } + return convertCustomConfig(typedStruct.TypeUrl, typedStruct.Value) + } + return nil, "", fmt.Errorf("custom config not implemented for type [%v]", config.GetTypeUrl()) +} + +func convertCustomConfig(typeURL string, s *structpb.Struct) (json.RawMessage, string, error) { + // The gRPC policy name will be the "type name" part of the value of the + // type_url field in the TypedStruct. We get this by using the part after + // the last / character. Can assume a valid type_url from the control plane. + urls := strings.Split(typeURL, "/") + if len(urls) == 0 { + return nil, "", fmt.Errorf("error converting custom audit logger %v for %v: typeURL must have a url-like format with the typeName being the value after the last /", typeURL, s) + } + name := urls[len(urls)-1] + + rawJSON := []byte("{}") + var err error + if s != nil { + rawJSON, err = json.Marshal(s) + if err != nil { + return nil, "", fmt.Errorf("error converting custom audit logger %v for %v: %v", typeURL, s, err) + } + } + return rawJSON, name, nil +} diff --git a/internal/xds/rbac/converter_test.go b/internal/xds/rbac/converter_test.go new file mode 100644 index 00000000000..253b9db2d50 --- /dev/null +++ b/internal/xds/rbac/converter_test.go @@ -0,0 +1,114 @@ +/* + * Copyright 2023 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package rbac + +import ( + "strings" + "testing" + + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" + "google.golang.org/grpc/authz/audit" + "google.golang.org/protobuf/types/known/anypb" +) + +func (s) TestBuildLoggerErrors(t *testing.T) { + tests := []struct { + name string + loggerConfig *v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig + expectedLogger audit.Logger + expectedError string + }{ + { + name: "nil typed config", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + TypedConfig: nil, + }, + }, + expectedError: "missing required field: TypedConfig", + }, + { + name: "Unsupported Type", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: &anypb.Any{}, + }, + }, + expectedError: "custom config not implemented for type ", + }, + { + name: "Empty name", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, ""), + }, + }, + expectedError: "field TypedConfig.TypeURL cannot be an empty string", + }, + { + name: "No registered logger", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "UnregisteredLogger", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "UnregisteredLogger"), + }, + IsOptional: false, + }, + expectedError: "no builder registered for UnregisteredLogger", + }, + { + name: "fail to parse custom config", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": "BADVALUE", "xyz": "123"}, "fail to parse custom config_TestAuditLoggerCustomConfig")}, + IsOptional: false, + }, + expectedError: "custom config could not be parsed", + }, + { + name: "no registered logger but optional passes", + loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "UnregisteredLogger", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "no registered logger but optional passes_UnregisteredLogger"), + }, + IsOptional: true, + }, + expectedLogger: nil, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + b := TestAuditLoggerCustomConfigBuilder{testName: test.name} + audit.RegisterLoggerBuilder(&b) + logger, err := buildLogger(test.loggerConfig) + if err != nil && !strings.HasPrefix(err.Error(), test.expectedError) { + t.Fatalf("expected error: %v. got error: %v", test.expectedError, err) + } else { + if logger != test.expectedLogger { + t.Fatalf("expected logger: %v. got logger: %v", test.expectedLogger, logger) + } + } + + }) + } + +} diff --git a/internal/xds/rbac/rbac_engine.go b/internal/xds/rbac/rbac_engine.go index a212579c63e..63237affe23 100644 --- a/internal/xds/rbac/rbac_engine.go +++ b/internal/xds/rbac/rbac_engine.go @@ -30,6 +30,7 @@ import ( v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" "google.golang.org/grpc" + "google.golang.org/grpc/authz/audit" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/grpclog" @@ -51,10 +52,10 @@ type ChainEngine struct { // NewChainEngine returns a chain of RBAC engines, used to make authorization // decisions on incoming RPCs. Returns a non-nil error for invalid policies. -func NewChainEngine(policies []*v3rbacpb.RBAC) (*ChainEngine, error) { +func NewChainEngine(policies []*v3rbacpb.RBAC, policyName string) (*ChainEngine, error) { engines := make([]*engine, 0, len(policies)) for _, policy := range policies { - engine, err := newEngine(policy) + engine, err := newEngine(policy, policyName) if err != nil { return nil, err } @@ -94,13 +95,16 @@ func (cre *ChainEngine) IsAuthorized(ctx context.Context) error { switch { case engine.action == v3rbacpb.RBAC_ALLOW && !ok: cre.logRequestDetails(rpcData) + engine.doAuditLogging(rpcData, matchingPolicyName, false) return status.Errorf(codes.PermissionDenied, "incoming RPC did not match an allow policy") case engine.action == v3rbacpb.RBAC_DENY && ok: cre.logRequestDetails(rpcData) + engine.doAuditLogging(rpcData, matchingPolicyName, false) return status.Errorf(codes.PermissionDenied, "incoming RPC matched a deny policy %q", matchingPolicyName) } // Every policy in the engine list must be queried. Thus, iterate to the // next policy. + engine.doAuditLogging(rpcData, matchingPolicyName, true) } // If the incoming RPC gets through all of the engines successfully (i.e. // doesn't not match an allow or match a deny engine), the RPC is authorized @@ -110,14 +114,18 @@ func (cre *ChainEngine) IsAuthorized(ctx context.Context) error { // engine is used for matching incoming RPCs to policies. type engine struct { - policies map[string]*policyMatcher + // TODO(gtcooke94) - differentiate between `policyName`, `policies`, and `rules` + policyName string + policies map[string]*policyMatcher // action must be ALLOW or DENY. - action v3rbacpb.RBAC_Action + action v3rbacpb.RBAC_Action + auditLoggers []audit.Logger + auditCondition v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition } -// newEngine creates an RBAC Engine based on the contents of policy. Returns a +// newEngine creates an RBAC Engine based on the contents of a policy. Returns a // non-nil error if the policy is invalid. -func newEngine(config *v3rbacpb.RBAC) (*engine, error) { +func newEngine(config *v3rbacpb.RBAC, policyName string) (*engine, error) { a := config.GetAction() if a != v3rbacpb.RBAC_ALLOW && a != v3rbacpb.RBAC_DENY { return nil, fmt.Errorf("unsupported action %s", config.Action) @@ -131,18 +139,47 @@ func newEngine(config *v3rbacpb.RBAC) (*engine, error) { } policies[name] = matcher } + + auditLoggers, auditCondition, err := parseAuditOptions(config.GetAuditLoggingOptions()) + if err != nil { + return nil, err + } return &engine{ - policies: policies, - action: a, + policyName: policyName, + policies: policies, + action: a, + auditLoggers: auditLoggers, + auditCondition: auditCondition, }, nil } +func parseAuditOptions(opts *v3rbacpb.RBAC_AuditLoggingOptions) ([]audit.Logger, v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition, error) { + if opts == nil { + return nil, v3rbacpb.RBAC_AuditLoggingOptions_NONE, nil + } + var auditLoggers []audit.Logger + for _, logger := range opts.LoggerConfigs { + auditLogger, err := buildLogger(logger) + if err != nil { + return nil, v3rbacpb.RBAC_AuditLoggingOptions_NONE, err + } + if auditLogger == nil { + // This occurs when the audit logger is not registered but also + // marked optional. + continue + } + auditLoggers = append(auditLoggers, auditLogger) + } + return auditLoggers, opts.GetAuditCondition(), nil + +} + // findMatchingPolicy determines if an incoming RPC matches a policy. On a // successful match, it returns the name of the matching policy and a true bool // to specify that there was a matching policy found. It returns false in // the case of not finding a matching policy. -func (r *engine) findMatchingPolicy(rpcData *rpcData) (string, bool) { - for policy, matcher := range r.policies { +func (e *engine) findMatchingPolicy(rpcData *rpcData) (string, bool) { + for policy, matcher := range e.policies { if matcher.match(rpcData) { return policy, true } @@ -238,3 +275,43 @@ type rpcData struct { // handshake. certs []*x509.Certificate } + +func (e *engine) doAuditLogging(rpcData *rpcData, rule string, authorized bool) { + // In the RBAC world, we need to have a SPIFFE ID as the principal for this + // to be meaningful + principal := "" + if rpcData.peerInfo != nil && rpcData.peerInfo.AuthInfo != nil && rpcData.peerInfo.AuthInfo.AuthType() == "tls" { + // If AuthType = tls, then we can cast AuthInfo to TLSInfo. + tlsInfo := rpcData.peerInfo.AuthInfo.(credentials.TLSInfo) + if tlsInfo.SPIFFEID != nil { + principal = tlsInfo.SPIFFEID.String() + } + } + + //TODO(gtcooke94) check if we need to log before creating the event + event := &audit.Event{ + FullMethodName: rpcData.fullMethod, + Principal: principal, + PolicyName: e.policyName, + MatchedRule: rule, + Authorized: authorized, + } + for _, logger := range e.auditLoggers { + switch e.auditCondition { + case v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY: + if !authorized { + logger.Log(event) + } + case v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW: + if authorized { + logger.Log(event) + } + case v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY_AND_ALLOW: + logger.Log(event) + } + } +} + +// This is used when converting a custom config from raw JSON to a TypedStruct. +// The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/". +const typeURLPrefix = "grpc.authz.audit_logging/" diff --git a/internal/xds/rbac/rbac_engine_test.go b/internal/xds/rbac/rbac_engine_test.go index 19bc4e8ca89..32c357f4953 100644 --- a/internal/xds/rbac/rbac_engine_test.go +++ b/internal/xds/rbac/rbac_engine_test.go @@ -21,10 +21,15 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "encoding/json" + "fmt" "net" "net/url" + "reflect" "testing" + v1xdsudpatypepb "github.com/cncf/xds/go/udpa/type/v1" + v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" @@ -32,12 +37,15 @@ import ( v3typepb "github.com/envoyproxy/go-control-plane/envoy/type/v3" wrapperspb "github.com/golang/protobuf/ptypes/wrappers" "google.golang.org/grpc" + "google.golang.org/grpc/authz/audit" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/structpb" ) type s struct { @@ -62,9 +70,10 @@ func (a *addr) String() string { return a.ipAddress } // raise errors. func (s) TestNewChainEngine(t *testing.T) { tests := []struct { - name string - policies []*v3rbacpb.RBAC - wantErr bool + name string + policies []*v3rbacpb.RBAC + wantErr bool + policyName string }{ { name: "SuccessCaseAnyMatchSingular", @@ -424,16 +433,256 @@ func (s) TestNewChainEngine(t *testing.T) { }, }, }, + { + name: "SimpleAuditLogger", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "SimpleAuditLogger_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + }, + }, + { + name: "AuditLoggerCustomConfig", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": 123, "xyz": "123"}, "AuditLoggerCustomConfig_TestAuditLoggerCustomConfig")}, + IsOptional: false, + }, + }, + }, + }, + }, + policyName: "test_policy", + }, + { + name: "AuditLoggerCustomConfigXdsTypedStruct", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + TypedConfig: createXDSTypedStruct(t, map[string]interface{}{"abc": 123, "xyz": "123"}, "AuditLoggerCustomConfigXdsTypedStruct_TestAuditLoggerCustomConfig")}, + IsOptional: false, + }, + }, + }, + }, + }, + policyName: "test_policy", + }, + { + name: "Missing Optional AuditLogger doesn't fail", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "UnsupportedLogger", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "Missing Optional AuditLogger doesn't fail_UnsupportedLogger")}, + IsOptional: true, + }, + }, + }, + }, + }, + }, + { + name: "Missing Non-Optional AuditLogger fails", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "UnsupportedLogger", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "Missing Non-Optional AuditLogger fails_UnsupportedLogger")}, + IsOptional: false, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "Cannot_parse_missing_CustomConfig", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + }, + IsOptional: false, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "Cannot_parse_bad_CustomConfig", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": "BADVALUE", "xyz": "123"}, "Cannot_parse_bad_CustomConfig_TestAuditLoggerCustomConfig")}, + IsOptional: false, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "Cannot_parse_missing_typedConfig_name", + policies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "anyone": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerCustomConfig", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": 123, "xyz": "123"}, "")}, + IsOptional: false, + }, + }, + }, + }, + }, + wantErr: true, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if _, err := NewChainEngine(test.policies); (err != nil) != test.wantErr { + b := TestAuditLoggerBufferBuilder{testName: test.name} + audit.RegisterLoggerBuilder(&b) + b2 := TestAuditLoggerCustomConfigBuilder{testName: test.name} + audit.RegisterLoggerBuilder(&b2) + if _, err := NewChainEngine(test.policies, test.policyName); (err != nil) != test.wantErr { t.Fatalf("NewChainEngine(%+v) returned err: %v, wantErr: %v", test.policies, err, test.wantErr) } }) } } +type rbacQuery struct { + rpcData *rpcData + wantStatusCode codes.Code + wantAuditEvents []*audit.Event +} + // TestChainEngine tests the chain of RBAC Engines by configuring the chain of // engines in a certain way in different scenarios. After configuring the chain // of engines in a certain way, this test pings the chain of engines with @@ -446,10 +695,8 @@ func (s) TestChainEngine(t *testing.T) { tests := []struct { name string rbacConfigs []*v3rbacpb.RBAC - rbacQueries []struct { - rpcData *rpcData - wantStatusCode codes.Code - } + rbacQueries []rbacQuery + policyName string }{ // SuccessCaseAnyMatch tests a single RBAC Engine instantiated with // a config with a policy with any rules for both permissions and @@ -471,10 +718,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ { rpcData: &rpcData{ fullMethod: "some method", @@ -505,10 +749,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This RPC should match with the local host fan policy. Thus, // this RPC should be allowed to proceed. { @@ -571,10 +812,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call should match with the service admin // policy. { @@ -659,10 +897,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call should match with the not-secret-content policy. { rpcData: &rpcData{ @@ -701,10 +936,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call should match with the certain-direct-remote-ip policy. { rpcData: &rpcData{ @@ -745,10 +977,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call should match with the certain-remote-ip policy. { rpcData: &rpcData{ @@ -785,10 +1014,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call shouldn't match with the // certain-destination-ip policy, as the test listens on local // host. @@ -836,10 +1062,7 @@ func (s) TestChainEngine(t *testing.T) { Action: v3rbacpb.RBAC_DENY, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This RPC should match with the allow policy, and shouldn't // match with the deny and thus should be allowed to proceed. { @@ -903,10 +1126,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ // This incoming RPC Call should match with the service admin // policy. No authentication info is provided, so the // authenticated matcher should match to the string matcher on @@ -956,10 +1176,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ { rpcData: &rpcData{ fullMethod: "some method", @@ -992,10 +1209,7 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - rbacQueries: []struct { - rpcData *rpcData - wantStatusCode codes.Code - }{ + rbacQueries: []rbacQuery{ { rpcData: &rpcData{ fullMethod: "some method", @@ -1007,85 +1221,709 @@ func (s) TestChainEngine(t *testing.T) { }, }, }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - // Instantiate the chainedRBACEngine with different configurations that are - // interesting to test and to query. - cre, err := NewChainEngine(test.rbacConfigs) - if err != nil { - t.Fatalf("Error constructing RBAC Engine: %v", err) - } - // Query the created chain of RBAC Engines with different args to see - // if the chain of RBAC Engines configured as such works as intended. - for _, data := range test.rbacQueries { - func() { - // Construct the context with three data points that have enough - // information to represent incoming RPC's. This will be how a - // user uses this API. A user will have to put MD, PeerInfo, and - // the connection the RPC is sent on in the context. - ctx := metadata.NewIncomingContext(context.Background(), data.rpcData.md) - - // Make a TCP connection with a certain destination port. The - // address/port of this connection will be used to populate the - // destination ip/port in RPCData struct. This represents what - // the user of ChainEngine will have to place into - // context, as this is only way to get destination ip and port. - lis, err := net.Listen("tcp", "localhost:0") - if err != nil { - t.Fatalf("Error listening: %v", err) - } - defer lis.Close() - connCh := make(chan net.Conn, 1) - go func() { - conn, err := lis.Accept() - if err != nil { - t.Errorf("Error accepting connection: %v", err) - return - } - connCh <- conn - }() - _, err = net.Dial("tcp", lis.Addr().String()) - if err != nil { - t.Fatalf("Error dialing: %v", err) - } - conn := <-connCh - defer conn.Close() - getConnection = func(context.Context) net.Conn { - return conn - } - ctx = peer.NewContext(ctx, data.rpcData.peerInfo) - stream := &ServerTransportStreamWithMethod{ - method: data.rpcData.fullMethod, - } - - ctx = grpc.NewContextWithServerTransportStream(ctx, stream) - err = cre.IsAuthorized(ctx) - if gotCode := status.Code(err); gotCode != data.wantStatusCode { - t.Fatalf("IsAuthorized(%+v, %+v) returned (%+v), want(%+v)", ctx, data.rpcData.fullMethod, gotCode, data.wantStatusCode) - } - }() - } - }) - } -} - -type ServerTransportStreamWithMethod struct { - method string -} - -func (sts *ServerTransportStreamWithMethod) Method() string { - return sts.method -} - -func (sts *ServerTransportStreamWithMethod) SetHeader(md metadata.MD) error { - return nil -} - -func (sts *ServerTransportStreamWithMethod) SendHeader(md metadata.MD) error { - return nil -} - -func (sts *ServerTransportStreamWithMethod) SetTrailer(md metadata.MD) error { - return nil + // AllowAndDenyPolicy tests a policy with an allow (on path) and + // deny (on port) policy chained together. This represents how a user + // configured interceptor would use this, and also is a potential + // configuration for a dynamic xds interceptor. Further, it tests that + // the audit logger works properly in each scenario. + { + name: "AuditLoggingAllowAndDenyPolicy_ON_ALLOW", + policyName: "test_policy", + rbacConfigs: []*v3rbacpb.RBAC{ + { + Policies: map[string]*v3rbacpb.Policy{ + "localhost-fan": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_UrlPath{UrlPath: &v3matcherpb.PathMatcher{Rule: &v3matcherpb.PathMatcher_Path{Path: &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "localhost-fan-page"}}}}}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + Action: v3rbacpb.RBAC_DENY, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_NONE, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_ALLOW_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + { + Policies: map[string]*v3rbacpb.Policy{ + "certain-source-ip": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_DirectRemoteIp{DirectRemoteIp: &v3corepb.CidrRange{AddressPrefix: "0.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: uint32(10)}}}}, + }, + }, + }, + Action: v3rbacpb.RBAC_ALLOW, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_ALLOW_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + }, + rbacQueries: []rbacQuery{ + // This RPC should match with the allow policy, and shouldn't + // match with the deny and thus should be allowed to proceed. + { + rpcData: &rpcData{ + fullMethod: "", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + AuthInfo: credentials.TLSInfo{ + State: tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{ + { + URIs: []*url.URL{ + { + Scheme: "spiffe", + Host: "cluster.local", + Path: "/ns/default/sa/admin", + }, + }, + }, + }, + }, + SPIFFEID: &url.URL{ + Scheme: "spiffe", + Host: "cluster.local", + Path: "/ns/default/sa/admin", + }, + }, + }, + }, + wantStatusCode: codes.OK, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "", + Principal: "spiffe://cluster.local/ns/default/sa/admin", + PolicyName: "test_policy", + MatchedRule: "certain-source-ip", + Authorized: true, + }, + }, + }, + // This RPC should match with both the allow policy and deny policy + // and thus shouldn't be allowed to proceed as matched with deny. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + // This RPC shouldn't match with either policy, and thus + // shouldn't be allowed to proceed as didn't match with allow. + { + rpcData: &rpcData{ + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + // This RPC shouldn't match with allow, match with deny, and + // thus shouldn't be allowed to proceed. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + }, + }, + { + name: "AuditLoggingAllowAndDenyPolicy_ON_DENY", + policyName: "test_policy", + rbacConfigs: []*v3rbacpb.RBAC{ + { + Policies: map[string]*v3rbacpb.Policy{ + "localhost-fan": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_UrlPath{UrlPath: &v3matcherpb.PathMatcher{Rule: &v3matcherpb.PathMatcher_Path{Path: &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "localhost-fan-page"}}}}}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + Action: v3rbacpb.RBAC_DENY, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_DENY_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + { + Policies: map[string]*v3rbacpb.Policy{ + "certain-source-ip": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_DirectRemoteIp{DirectRemoteIp: &v3corepb.CidrRange{AddressPrefix: "0.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: uint32(10)}}}}, + }, + }, + }, + Action: v3rbacpb.RBAC_ALLOW, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_DENY_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + }, + rbacQueries: []rbacQuery{ + // This RPC should match with the allow policy, and shouldn't + // match with the deny and thus should be allowed to proceed. + // Audit logging matches with nothing. + { + rpcData: &rpcData{ + fullMethod: "", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.OK, + }, + // This RPC should match with both the allow policy and deny policy + // and thus shouldn't be allowed to proceed as matched with deny. + // Audit logging matches with deny and short circuits. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + AuthInfo: credentials.TLSInfo{ + State: tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{ + { + URIs: []*url.URL{ + { + Host: "cluster.local", + Path: "/ns/default/sa/admin", + }, + }, + }, + }, + }, + }, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "localhost-fan-page", + PolicyName: "test_policy", + MatchedRule: "localhost-fan", + Authorized: false, + }, + }, + }, + // This RPC shouldn't match with either policy, and thus + // shouldn't be allowed to proceed as didn't match with allow. + // Audit logging matches with the allow policy. + { + rpcData: &rpcData{ + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "", + PolicyName: "test_policy", + MatchedRule: "", + Authorized: false, + }, + }, + }, + // This RPC shouldn't match with allow, match with deny, and + // thus shouldn't be allowed to proceed. + // Audit logging will have the deny logged. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "localhost-fan-page", + PolicyName: "test_policy", + MatchedRule: "localhost-fan", + Authorized: false, + }, + }, + }, + }, + }, + { + name: "AuditLoggingAllowAndDenyPolicy_NONE", + policyName: "test_policy", + rbacConfigs: []*v3rbacpb.RBAC{ + { + Policies: map[string]*v3rbacpb.Policy{ + "localhost-fan": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_UrlPath{UrlPath: &v3matcherpb.PathMatcher{Rule: &v3matcherpb.PathMatcher_Path{Path: &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "localhost-fan-page"}}}}}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + Action: v3rbacpb.RBAC_DENY, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_NONE, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_NONE_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + { + Policies: map[string]*v3rbacpb.Policy{ + "certain-source-ip": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_DirectRemoteIp{DirectRemoteIp: &v3corepb.CidrRange{AddressPrefix: "0.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: uint32(10)}}}}, + }, + }, + }, + Action: v3rbacpb.RBAC_ALLOW, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_NONE, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_NONE_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + }, + rbacQueries: []rbacQuery{ + // This RPC should match with the allow policy, and shouldn't + // match with the deny and thus should be allowed to proceed. + // Audit logging is NONE. + { + rpcData: &rpcData{ + fullMethod: "", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.OK, + }, + // This RPC should match with both the allow policy and deny policy + // and thus shouldn't be allowed to proceed as matched with deny. + // Audit logging is NONE. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + // This RPC shouldn't match with either policy, and thus + // shouldn't be allowed to proceed as didn't match with allow. + // Audit logging is NONE. + { + rpcData: &rpcData{ + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + // This RPC shouldn't match with allow, match with deny, and + // thus shouldn't be allowed to proceed. + // Audit logging is NONE. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + }, + }, + }, + { + name: "AuditLoggingAllowAndDenyPolicy_ON_DENY_AND_ALLOW", + policyName: "test_policy", + rbacConfigs: []*v3rbacpb.RBAC{ + { + Policies: map[string]*v3rbacpb.Policy{ + "localhost-fan": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_UrlPath{UrlPath: &v3matcherpb.PathMatcher{Rule: &v3matcherpb.PathMatcher_Path{Path: &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "localhost-fan-page"}}}}}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Any{Any: true}}, + }, + }, + }, + Action: v3rbacpb.RBAC_DENY, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_DENY_AND_ALLOW_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + { + Policies: map[string]*v3rbacpb.Policy{ + "certain-source-ip": { + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_DirectRemoteIp{DirectRemoteIp: &v3corepb.CidrRange{AddressPrefix: "0.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: uint32(10)}}}}, + }, + }, + }, + Action: v3rbacpb.RBAC_ALLOW, + AuditLoggingOptions: &v3rbacpb.RBAC_AuditLoggingOptions{ + AuditCondition: v3rbacpb.RBAC_AuditLoggingOptions_ON_DENY_AND_ALLOW, + LoggerConfigs: []*v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{ + {AuditLogger: &v3corepb.TypedExtensionConfig{ + Name: "TestAuditLoggerBuffer", + TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "AuditLoggingAllowAndDenyPolicy_ON_DENY_AND_ALLOW_TestAuditLoggerBuffer")}, + IsOptional: false, + }, + }, + }, + }, + }, + rbacQueries: []rbacQuery{ + // This RPC should match with the allow policy, and shouldn't + // match with the deny and thus should be allowed to proceed. + // Audit logging matches with nothing. + { + rpcData: &rpcData{ + fullMethod: "", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.OK, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "", + PolicyName: "test_policy", + MatchedRule: "certain-source-ip", + Authorized: true, + }, + }, + }, + // This RPC should match with both the allow policy and deny policy + // and thus shouldn't be allowed to proceed as matched with deny. + // Audit logging matches with deny and short circuits. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "0.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "localhost-fan-page", + PolicyName: "test_policy", + MatchedRule: "localhost-fan", + Authorized: false, + }, + }, + }, + // This RPC shouldn't match with either policy, and thus + // shouldn't be allowed to proceed as didn't match with allow. + // Audit logging matches with the allow policy. + { + rpcData: &rpcData{ + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "", + PolicyName: "test_policy", + MatchedRule: "", + Authorized: false, + }, + }, + }, + // This RPC shouldn't match with allow, match with deny, and + // thus shouldn't be allowed to proceed. + // Audit logging will have the deny logged. + { + rpcData: &rpcData{ + fullMethod: "localhost-fan-page", + peerInfo: &peer.Peer{ + Addr: &addr{ipAddress: "10.0.0.0"}, + }, + }, + wantStatusCode: codes.PermissionDenied, + wantAuditEvents: []*audit.Event{ + { + FullMethodName: "localhost-fan-page", + PolicyName: "test_policy", + MatchedRule: "localhost-fan", + Authorized: false, + }, + }, + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + b := TestAuditLoggerBufferBuilder{testName: test.name} + audit.RegisterLoggerBuilder(&b) + b2 := TestAuditLoggerCustomConfigBuilder{testName: test.name} + audit.RegisterLoggerBuilder(&b2) + + // Instantiate the chainedRBACEngine with different configurations that are + // interesting to test and to query. + cre, err := NewChainEngine(test.rbacConfigs, test.policyName) + if err != nil { + t.Fatalf("Error constructing RBAC Engine: %v", err) + } + // Query the created chain of RBAC Engines with different args to see + // if the chain of RBAC Engines configured as such works as intended. + for _, data := range test.rbacQueries { + func() { + // Construct the context with three data points that have enough + // information to represent incoming RPC's. This will be how a + // user uses this API. A user will have to put MD, PeerInfo, and + // the connection the RPC is sent on in the context. + ctx := metadata.NewIncomingContext(context.Background(), data.rpcData.md) + + // Make a TCP connection with a certain destination port. The + // address/port of this connection will be used to populate the + // destination ip/port in RPCData struct. This represents what + // the user of ChainEngine will have to place into context, + // as this is only way to get destination ip and port. + lis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("Error listening: %v", err) + } + defer lis.Close() + connCh := make(chan net.Conn, 1) + go func() { + conn, err := lis.Accept() + if err != nil { + t.Errorf("Error accepting connection: %v", err) + return + } + connCh <- conn + }() + _, err = net.Dial("tcp", lis.Addr().String()) + if err != nil { + t.Fatalf("Error dialing: %v", err) + } + conn := <-connCh + defer conn.Close() + getConnection = func(context.Context) net.Conn { + return conn + } + ctx = peer.NewContext(ctx, data.rpcData.peerInfo) + stream := &ServerTransportStreamWithMethod{ + method: data.rpcData.fullMethod, + } + + ctx = grpc.NewContextWithServerTransportStream(ctx, stream) + err = cre.IsAuthorized(ctx) + if gotCode := status.Code(err); gotCode != data.wantStatusCode { + t.Fatalf("IsAuthorized(%+v, %+v) returned (%+v), want(%+v)", ctx, data.rpcData.fullMethod, gotCode, data.wantStatusCode) + } + if !reflect.DeepEqual(b.auditEvents, data.wantAuditEvents) { + t.Fatalf("Unexpected audit event for query:%v", data) + } + + // This builder's auditEvents can be shared for several queries, make sure it's empty. + b.auditEvents = nil + }() + } + }) + } +} + +type ServerTransportStreamWithMethod struct { + method string +} + +func (sts *ServerTransportStreamWithMethod) Method() string { + return sts.method +} + +func (sts *ServerTransportStreamWithMethod) SetHeader(md metadata.MD) error { + return nil +} + +func (sts *ServerTransportStreamWithMethod) SendHeader(md metadata.MD) error { + return nil +} + +func (sts *ServerTransportStreamWithMethod) SetTrailer(md metadata.MD) error { + return nil +} + +// An audit logger that will log to the auditEvents slice. +type TestAuditLoggerBuffer struct { + auditEvents *[]*audit.Event +} + +func (logger *TestAuditLoggerBuffer) Log(e *audit.Event) { + *(logger.auditEvents) = append(*(logger.auditEvents), e) +} + +// Builds TestAuditLoggerBuffer. +type TestAuditLoggerBufferBuilder struct { + auditEvents []*audit.Event + testName string +} + +// The required config for TestAuditLoggerBuffer. +type TestAuditLoggerBufferConfig struct { + audit.LoggerConfig +} + +func (b *TestAuditLoggerBufferBuilder) ParseLoggerConfig(configJSON json.RawMessage) (config audit.LoggerConfig, err error) { + return TestAuditLoggerBufferConfig{}, nil +} + +func (b *TestAuditLoggerBufferBuilder) Build(config audit.LoggerConfig) audit.Logger { + return &TestAuditLoggerBuffer{auditEvents: &b.auditEvents} +} + +func (b *TestAuditLoggerBufferBuilder) Name() string { + return b.testName + "_TestAuditLoggerBuffer" +} + +// An audit logger to test using a custom config. +type TestAuditLoggerCustomConfig struct{} + +func (logger *TestAuditLoggerCustomConfig) Log(*audit.Event) {} + +// Build TestAuditLoggerCustomConfig. This builds a TestAuditLoggerCustomConfig +// logger that uses a custom config. +type TestAuditLoggerCustomConfigBuilder struct { + testName string +} + +// The custom config for the TestAuditLoggerCustomConfig logger. +type TestAuditLoggerCustomConfigConfig struct { + audit.LoggerConfig + Abc int + Xyz string +} + +// Parses TestAuditLoggerCustomConfigConfig. Hard-coded to match with it's test +// case above. +func (b TestAuditLoggerCustomConfigBuilder) ParseLoggerConfig(configJSON json.RawMessage) (audit.LoggerConfig, error) { + c := TestAuditLoggerCustomConfigConfig{} + err := json.Unmarshal(configJSON, &c) + if err != nil { + return nil, fmt.Errorf("could not parse custom config: %v", err) + } + return c, nil +} + +func (b *TestAuditLoggerCustomConfigBuilder) Build(config audit.LoggerConfig) audit.Logger { + return &TestAuditLoggerCustomConfig{} +} + +func (b *TestAuditLoggerCustomConfigBuilder) Name() string { + return b.testName + "_TestAuditLoggerCustomConfig" +} + +// Builds custom configs for audit logger RBAC protos. +func createUDPATypedStruct(t *testing.T, in map[string]interface{}, name string) *anypb.Any { + t.Helper() + pb, err := structpb.NewStruct(in) + if err != nil { + t.Fatalf("createUDPATypedStructFailed during structpb.NewStruct: %v", err) + } + typedURL := "" + if name != "" { + typedURL = typeURLPrefix + name + } + typedStruct := &v1xdsudpatypepb.TypedStruct{ + TypeUrl: typedURL, + Value: pb, + } + customConfig, err := anypb.New(typedStruct) + if err != nil { + t.Fatalf("createUDPATypedStructFailed during anypb.New: %v", err) + } + return customConfig +} + +// Builds custom configs for audit logger RBAC protos. +func createXDSTypedStruct(t *testing.T, in map[string]interface{}, name string) *anypb.Any { + t.Helper() + pb, err := structpb.NewStruct(in) + if err != nil { + t.Fatalf("createXDSTypedStructFailed during structpb.NewStruct: %v", err) + } + typedStruct := &v3xdsxdstypepb.TypedStruct{ + TypeUrl: typeURLPrefix + name, + Value: pb, + } + customConfig, err := anypb.New(typedStruct) + if err != nil { + t.Fatalf("createXDSTypedStructFailed during anypb.New: %v", err) + } + return customConfig } diff --git a/xds/internal/httpfilter/rbac/rbac.go b/xds/internal/httpfilter/rbac/rbac.go index 209283c3bf5..277fcfc5927 100644 --- a/xds/internal/httpfilter/rbac/rbac.go +++ b/xds/internal/httpfilter/rbac/rbac.go @@ -126,7 +126,10 @@ func parseConfig(rbacCfg *rpb.RBAC) (httpfilter.FilterConfig, error) { return config{}, nil } - ce, err := rbac.NewChainEngine([]*v3rbacpb.RBAC{rbacCfg.GetRules()}) + // TODO(gregorycooke) - change the call chain to here so we have the filter + // name to input here instead of an empty string. It will come from here: + // https://github.com/grpc/grpc-go/blob/eff0942e95d93112921414aee758e619ec86f26f/xds/internal/xdsclient/xdsresource/unmarshal_lds.go#L199 + ce, err := rbac.NewChainEngine([]*v3rbacpb.RBAC{rbacCfg.GetRules()}, "") if err != nil { // "At this time, if the RBAC.action is Action.LOG then the policy will be // completely ignored, as if RBAC was not configurated." - A41