From 9e4dc0adabac2b83bec60cccaa61b2600bf7297d Mon Sep 17 00:00:00 2001 From: Ashitha Santhosh Date: Fri, 15 Oct 2021 16:45:20 -0700 Subject: [PATCH 1/4] authz: support empty principals in SDK and fixes to rbac authenticated matcher. --- authz/rbac_translator.go | 22 ++-- authz/rbac_translator_test.go | 26 ++++ authz/sdk_end2end_test.go | 175 ++++++++++++++++++++++++++ internal/xds/rbac/matchers.go | 12 +- internal/xds/rbac/rbac_engine.go | 5 + internal/xds/rbac/rbac_engine_test.go | 14 +++ 6 files changed, 240 insertions(+), 14 deletions(-) diff --git a/authz/rbac_translator.go b/authz/rbac_translator.go index 039d76bc99d..08ba9054d07 100644 --- a/authz/rbac_translator.go +++ b/authz/rbac_translator.go @@ -39,7 +39,7 @@ type header struct { } type peer struct { - Principals []string + Principals *[]string `json:",omitempty"` } type request struct { @@ -155,14 +155,20 @@ func parsePrincipalNames(principalNames []string) []*v3rbacpb.Principal { } func parsePeer(source peer) (*v3rbacpb.Principal, error) { - if len(source.Principals) > 0 { - return principalOr(parsePrincipalNames(source.Principals)), nil + if source.Principals == nil { + return &v3rbacpb.Principal{ + Identifier: &v3rbacpb.Principal_Any{ + Any: true, + }, + }, nil } - return &v3rbacpb.Principal{ - Identifier: &v3rbacpb.Principal_Any{ - Any: true, - }, - }, nil + if len(*source.Principals) == 0 { + return &v3rbacpb.Principal{ + Identifier: &v3rbacpb.Principal_Authenticated_{ + Authenticated: &v3rbacpb.Principal_Authenticated{}, + }}, nil + } + return principalOr(parsePrincipalNames(*source.Principals)), nil } func parsePaths(paths []string) []*v3rbacpb.Permission { diff --git a/authz/rbac_translator_test.go b/authz/rbac_translator_test.go index 9a883e9d78d..9b88362ea90 100644 --- a/authz/rbac_translator_test.go +++ b/authz/rbac_translator_test.go @@ -205,6 +205,32 @@ func TestTranslatePolicy(t *testing.T) { }, }, }, + "empty principal field": { + authzPolicy: `{ + "name": "authz", + "allow_rules": [{ + "name": "allow_authenticated", + "source": {"principals":[]} + }] + }`, + wantPolicies: []*v3rbacpb.RBAC{ + { + Action: v3rbacpb.RBAC_ALLOW, + Policies: map[string]*v3rbacpb.Policy{ + "authz_allow_authenticated": { + Principals: []*v3rbacpb.Principal{ + {Identifier: &v3rbacpb.Principal_Authenticated_{ + Authenticated: &v3rbacpb.Principal_Authenticated{}, + }}, + }, + Permissions: []*v3rbacpb.Permission{ + {Rule: &v3rbacpb.Permission_Any{Any: true}}, + }, + }, + }, + }, + }, + }, "unknown field": { authzPolicy: `{"random": 123}`, wantErr: "failed to unmarshal policy", diff --git a/authz/sdk_end2end_test.go b/authz/sdk_end2end_test.go index 093b2bb437d..1b1aabb5e97 100644 --- a/authz/sdk_end2end_test.go +++ b/authz/sdk_end2end_test.go @@ -20,6 +20,8 @@ package authz_test import ( "context" + "crypto/tls" + "crypto/x509" "io" "io/ioutil" "net" @@ -30,10 +32,12 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/authz" "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/status" pb "google.golang.org/grpc/test/grpc_testing" + "google.golang.org/grpc/testdata" ) type testServer struct { @@ -257,6 +261,45 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, + "DeniesRpcRequestWithPrincipalsFieldOnUnauthenticatedConnection": { + authzPolicy: `{ + "name": "authz", + "allow_rules": + [ + { + "name": "allow_TestServiceCalls", + "source": { + "principals": + [ + "foo" + ] + }, + "request": { + "paths": + [ + "/grpc.testing.TestService/*" + ] + } + } + ] + }`, + wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), + }, + "DeniesRpcRequestWithEmptyPrincipalsOnUnauthenticatedConnection": { + authzPolicy: `{ + "name": "authz", + "allow_rules": + [ + { + "name": "allow_authenticated", + "source": { + "principals": [] + } + } + ] + }`, + wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), + }, } func (s) TestSDKStaticPolicyEnd2End(t *testing.T) { @@ -315,6 +358,138 @@ func (s) TestSDKStaticPolicyEnd2End(t *testing.T) { } } +func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnTlsAuthenticatedConnection(t *testing.T) { + authzPolicy := `{ + "name": "authz", + "allow_rules": + [ + { + "name": "allow_authenticated", + "source": { + "principals": [] + } + } + ] + }` + // Start a gRPC server with SDK unary server interceptor. + i, _ := authz.NewStatic(authzPolicy) + creds, err := credentials.NewServerTLSFromFile(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem")) + if err != nil { + t.Fatalf("failed to generate credentials: %v", err) + } + s := grpc.NewServer( + grpc.Creds(creds), + grpc.ChainUnaryInterceptor(i.UnaryInterceptor)) + defer s.Stop() + pb.RegisterTestServiceServer(s, &testServer{}) + + lis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("error listening: %v", err) + } + go s.Serve(lis) + + // Establish a connection to the server. + creds, err = credentials.NewClientTLSFromFile(testdata.Path("x509/server_ca_cert.pem"), "x.test.example.com") + if err != nil { + t.Fatalf("failed to load credentials: %v", err) + } + clientConn, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(creds)) + if err != nil { + t.Fatalf("grpc.Dial(%v) failed: %v", lis.Addr().String(), err) + } + defer clientConn.Close() + client := pb.NewTestServiceClient(clientConn) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Verifying authorization decision. + _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) + if got := status.Convert(err); got.Code() != codes.OK { + t.Fatalf("error want:{%v} got:{%v}", codes.OK, got.Err()) + } +} + +func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnMtlsAuthenticatedConnection(t *testing.T) { + authzPolicy := `{ + "name": "authz", + "allow_rules": + [ + { + "name": "allow_authenticated", + "source": { + "principals": [] + } + } + ] + }` + // Start a gRPC server with SDK unary server interceptor. + i, _ := authz.NewStatic(authzPolicy) + cert, err := tls.LoadX509KeyPair(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(x509/server1_cert.pem, x509/server1_key.pem) failed: %v", err) + } + ca, err := ioutil.ReadFile(testdata.Path("x509/client_ca_cert.pem")) + if err != nil { + t.Fatalf("ioutil.ReadFile(x509/client_ca_cert.pem) failed: %v", err) + } + certPool := x509.NewCertPool() + if !certPool.AppendCertsFromPEM(ca) { + t.Fatal("failed to append certificates") + } + creds := credentials.NewTLS(&tls.Config{ + ClientAuth: tls.RequireAndVerifyClientCert, + Certificates: []tls.Certificate{cert}, + ClientCAs: certPool, + }) + s := grpc.NewServer( + grpc.Creds(creds), + grpc.ChainUnaryInterceptor(i.UnaryInterceptor)) + defer s.Stop() + pb.RegisterTestServiceServer(s, &testServer{}) + + lis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("error listening: %v", err) + } + go s.Serve(lis) + + // Establish a connection to the server. + cert, err = tls.LoadX509KeyPair(testdata.Path("x509/client1_cert.pem"), testdata.Path("x509/client1_key.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(x509/client1_cert.pem, x509/client1_key.pem) failed: %v", err) + } + ca, err = ioutil.ReadFile(testdata.Path("x509/server_ca_cert.pem")) + if err != nil { + t.Fatalf("ioutil.ReadFile(x509/server_ca_cert.pem) failed: %v", err) + } + roots := x509.NewCertPool() + if !roots.AppendCertsFromPEM(ca) { + t.Fatal("failed to append certificates") + } + creds = credentials.NewTLS(&tls.Config{ + Certificates: []tls.Certificate{cert}, + RootCAs: roots, + ServerName: "x.test.example.com", + }) + clientConn, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(creds)) + if err != nil { + t.Fatalf("grpc.Dial(%v) failed: %v", lis.Addr().String(), err) + } + defer clientConn.Close() + client := pb.NewTestServiceClient(clientConn) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Verifying authorization decision. + _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) + if got := status.Convert(err); got.Code() != codes.OK { + t.Fatalf("error want:{%v} got:{%v}", codes.OK, got.Err()) + } +} + func (s) TestSDKFileWatcherEnd2End(t *testing.T) { for name, test := range sdkTests { t.Run(name, func(t *testing.T) { diff --git a/internal/xds/rbac/matchers.go b/internal/xds/rbac/matchers.go index 28dabf46591..6129a292d23 100644 --- a/internal/xds/rbac/matchers.go +++ b/internal/xds/rbac/matchers.go @@ -395,13 +395,13 @@ func newAuthenticatedMatcher(authenticatedMatcherConfig *v3rbacpb.Principal_Auth } func (am *authenticatedMatcher) match(data *rpcData) bool { - // Represents this line in the RBAC documentation = "If unset, it applies to - // any user that is authenticated" (see package-level comments). An - // authenticated downstream in a stateful TLS connection will have to - // provide a certificate to prove their identity. Thus, you can simply check - // if there is a certificate present. + if data.authType != "tls" { + // Connection is not authenticated. + return false + } if am.stringMatcher == nil { - return len(data.certs) != 0 + // Allows any authenticated user. + return true } // "If there is no client certificate (thus no SAN nor Subject), check if "" // (empty string) matches. If it matches, the principal_name is said to diff --git a/internal/xds/rbac/rbac_engine.go b/internal/xds/rbac/rbac_engine.go index a25f9cfdeef..9dba9793c62 100644 --- a/internal/xds/rbac/rbac_engine.go +++ b/internal/xds/rbac/rbac_engine.go @@ -187,10 +187,12 @@ func newRPCData(ctx context.Context) (*rpcData, error) { return nil, fmt.Errorf("error parsing local address: %v", err) } + var authType string var peerCertificates []*x509.Certificate if pi.AuthInfo != nil { tlsInfo, ok := pi.AuthInfo.(credentials.TLSInfo) if ok { + authType = pi.AuthInfo.AuthType() peerCertificates = tlsInfo.State.PeerCertificates } } @@ -200,6 +202,7 @@ func newRPCData(ctx context.Context) (*rpcData, error) { peerInfo: pi, fullMethod: mn, destinationPort: uint32(dp), + authType: authType, localAddr: conn.LocalAddr(), certs: peerCertificates, }, nil @@ -217,6 +220,8 @@ type rpcData struct { // destinationPort is the port that the RPC is being sent to on the // server. destinationPort uint32 + // authType is the type of authentication. + authType string // localAddr is the address that the RPC is being sent to. localAddr net.Addr // certs are the certificates presented by the peer during a TLS diff --git a/internal/xds/rbac/rbac_engine_test.go b/internal/xds/rbac/rbac_engine_test.go index 17832458209..e2e5d98c2a8 100644 --- a/internal/xds/rbac/rbac_engine_test.go +++ b/internal/xds/rbac/rbac_engine_test.go @@ -916,6 +916,20 @@ func (s) TestChainEngine(t *testing.T) { fullMethod: "some method", 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.OK, From 42b6ff69f0a2b6231056b991de9dae8850c628c1 Mon Sep 17 00:00:00 2001 From: Ashitha Santhosh Date: Mon, 18 Oct 2021 18:13:05 -0700 Subject: [PATCH 2/4] Minor formatting --- authz/rbac_translator.go | 2 +- internal/xds/rbac/rbac_engine.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/authz/rbac_translator.go b/authz/rbac_translator.go index 08ba9054d07..89275d78dd1 100644 --- a/authz/rbac_translator.go +++ b/authz/rbac_translator.go @@ -39,7 +39,7 @@ type header struct { } type peer struct { - Principals *[]string `json:",omitempty"` + Principals *[]string } type request struct { diff --git a/internal/xds/rbac/rbac_engine.go b/internal/xds/rbac/rbac_engine.go index 9dba9793c62..35b82747a2d 100644 --- a/internal/xds/rbac/rbac_engine.go +++ b/internal/xds/rbac/rbac_engine.go @@ -202,8 +202,8 @@ func newRPCData(ctx context.Context) (*rpcData, error) { peerInfo: pi, fullMethod: mn, destinationPort: uint32(dp), - authType: authType, localAddr: conn.LocalAddr(), + authType: authType, certs: peerCertificates, }, nil } @@ -220,10 +220,10 @@ type rpcData struct { // destinationPort is the port that the RPC is being sent to on the // server. destinationPort uint32 - // authType is the type of authentication. - authType string // localAddr is the address that the RPC is being sent to. localAddr net.Addr + // authType is the type of authentication. + authType string // certs are the certificates presented by the peer during a TLS // handshake. certs []*x509.Certificate From 6f2a92265e75acc87c687c8462d8e22fa2c7614f Mon Sep 17 00:00:00 2001 From: Ashitha Santhosh Date: Tue, 19 Oct 2021 12:00:54 -0700 Subject: [PATCH 3/4] Remove pointer from principals fields --- authz/rbac_translator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/authz/rbac_translator.go b/authz/rbac_translator.go index 89275d78dd1..4a790b1a702 100644 --- a/authz/rbac_translator.go +++ b/authz/rbac_translator.go @@ -39,7 +39,7 @@ type header struct { } type peer struct { - Principals *[]string + Principals []string } type request struct { @@ -162,13 +162,13 @@ func parsePeer(source peer) (*v3rbacpb.Principal, error) { }, }, nil } - if len(*source.Principals) == 0 { + if len(source.Principals) == 0 { return &v3rbacpb.Principal{ Identifier: &v3rbacpb.Principal_Authenticated_{ Authenticated: &v3rbacpb.Principal_Authenticated{}, }}, nil } - return principalOr(parsePrincipalNames(*source.Principals)), nil + return principalOr(parsePrincipalNames(source.Principals)), nil } func parsePaths(paths []string) []*v3rbacpb.Permission { From b9a21be0f862ad72cfd8e65e192a90c16766127c Mon Sep 17 00:00:00 2001 From: Ashitha Santhosh Date: Thu, 21 Oct 2021 14:56:20 -0700 Subject: [PATCH 4/4] resolving comments --- authz/sdk_end2end_test.go | 54 +++++++++++++++----------------- internal/xds/rbac/rbac_engine.go | 2 +- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/authz/sdk_end2end_test.go b/authz/sdk_end2end_test.go index 1b1aabb5e97..79fa379bcea 100644 --- a/authz/sdk_end2end_test.go +++ b/authz/sdk_end2end_test.go @@ -73,7 +73,7 @@ var sdkTests = map[string]struct { md metadata.MD wantStatus *status.Status }{ - "DeniesRpcMatchInDenyNoMatchInAllow": { + "DeniesRPCMatchInDenyNoMatchInAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -116,7 +116,7 @@ var sdkTests = map[string]struct { md: metadata.Pairs("key-abc", "val-abc"), wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, - "DeniesRpcMatchInDenyAndAllow": { + "DeniesRPCMatchInDenyAndAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -146,7 +146,7 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, - "AllowsRpcNoMatchInDenyMatchInAllow": { + "AllowsRPCNoMatchInDenyMatchInAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -183,7 +183,7 @@ var sdkTests = map[string]struct { md: metadata.Pairs("key-xyz", "val-xyz"), wantStatus: status.New(codes.OK, ""), }, - "DeniesRpcNoMatchInDenyAndAllow": { + "DeniesRPCNoMatchInDenyAndAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -213,7 +213,7 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, - "AllowsRpcEmptyDenyMatchInAllow": { + "AllowsRPCEmptyDenyMatchInAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -242,7 +242,7 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.OK, ""), }, - "DeniesRpcEmptyDenyNoMatchInAllow": { + "DeniesRPCEmptyDenyNoMatchInAllow": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -261,7 +261,7 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, - "DeniesRpcRequestWithPrincipalsFieldOnUnauthenticatedConnection": { + "DeniesRPCRequestWithPrincipalsFieldOnUnauthenticatedConnection": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -285,7 +285,7 @@ var sdkTests = map[string]struct { }`, wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"), }, - "DeniesRpcRequestWithEmptyPrincipalsOnUnauthenticatedConnection": { + "DeniesRPCRequestWithEmptyPrincipalsOnUnauthenticatedConnection": { authzPolicy: `{ "name": "authz", "allow_rules": @@ -358,7 +358,7 @@ func (s) TestSDKStaticPolicyEnd2End(t *testing.T) { } } -func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnTlsAuthenticatedConnection(t *testing.T) { +func (s) TestSDKAllowsRPCRequestWithEmptyPrincipalsOnTLSAuthenticatedConnection(t *testing.T) { authzPolicy := `{ "name": "authz", "allow_rules": @@ -405,13 +405,12 @@ func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnTlsAuthenticatedConnection( defer cancel() // Verifying authorization decision. - _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) - if got := status.Convert(err); got.Code() != codes.OK { - t.Fatalf("error want:{%v} got:{%v}", codes.OK, got.Err()) + if _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}); err != nil { + t.Fatalf("client.UnaryCall(_, _) = %v; want nil", err) } } -func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnMtlsAuthenticatedConnection(t *testing.T) { +func (s) TestSDKAllowsRPCRequestWithEmptyPrincipalsOnMTLSAuthenticatedConnection(t *testing.T) { authzPolicy := `{ "name": "authz", "allow_rules": @@ -484,9 +483,8 @@ func (s) TestSDKAllowsRpcRequestWithEmptyPrincipalsOnMtlsAuthenticatedConnection defer cancel() // Verifying authorization decision. - _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) - if got := status.Convert(err); got.Code() != codes.OK { - t.Fatalf("error want:{%v} got:{%v}", codes.OK, got.Err()) + if _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}); err != nil { + t.Fatalf("client.UnaryCall(_, _) = %v; want nil", err) } } @@ -562,7 +560,7 @@ func retryUntil(ctx context.Context, tsc pb.TestServiceClient, want *status.Stat } func (s) TestSDKFileWatcher_ValidPolicyRefresh(t *testing.T) { - valid1 := sdkTests["DeniesRpcMatchInDenyAndAllow"] + valid1 := sdkTests["DeniesRPCMatchInDenyAndAllow"] file := createTmpPolicyFile(t, "valid_policy_refresh", []byte(valid1.authzPolicy)) i, _ := authz.NewFileWatcher(file, 100*time.Millisecond) defer i.Close() @@ -594,23 +592,23 @@ func (s) TestSDKFileWatcher_ValidPolicyRefresh(t *testing.T) { // Verifying authorization decision. _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) if got := status.Convert(err); got.Code() != valid1.wantStatus.Code() || got.Message() != valid1.wantStatus.Message() { - t.Fatalf("error want:{%v} got:{%v}", valid1.wantStatus.Err(), got.Err()) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got.Err(), valid1.wantStatus.Err()) } // Rewrite the file with a different valid authorization policy. - valid2 := sdkTests["AllowsRpcEmptyDenyMatchInAllow"] + valid2 := sdkTests["AllowsRPCEmptyDenyMatchInAllow"] if err := ioutil.WriteFile(file, []byte(valid2.authzPolicy), os.ModePerm); err != nil { t.Fatalf("ioutil.WriteFile(%q) failed: %v", file, err) } // Verifying authorization decision. if got := retryUntil(ctx, client, valid2.wantStatus); got != nil { - t.Fatalf("error want:{%v} got:{%v}", valid2.wantStatus.Err(), got) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got, valid2.wantStatus.Err()) } } func (s) TestSDKFileWatcher_InvalidPolicySkipReload(t *testing.T) { - valid := sdkTests["DeniesRpcMatchInDenyAndAllow"] + valid := sdkTests["DeniesRPCMatchInDenyAndAllow"] file := createTmpPolicyFile(t, "invalid_policy_skip_reload", []byte(valid.authzPolicy)) i, _ := authz.NewFileWatcher(file, 20*time.Millisecond) defer i.Close() @@ -642,7 +640,7 @@ func (s) TestSDKFileWatcher_InvalidPolicySkipReload(t *testing.T) { // Verifying authorization decision. _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) if got := status.Convert(err); got.Code() != valid.wantStatus.Code() || got.Message() != valid.wantStatus.Message() { - t.Fatalf("error want:{%v} got:{%v}", valid.wantStatus.Err(), got.Err()) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got.Err(), valid.wantStatus.Err()) } // Skips the invalid policy update, and continues to use the valid policy. @@ -656,12 +654,12 @@ func (s) TestSDKFileWatcher_InvalidPolicySkipReload(t *testing.T) { // Verifying authorization decision. _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) if got := status.Convert(err); got.Code() != valid.wantStatus.Code() || got.Message() != valid.wantStatus.Message() { - t.Fatalf("error want:{%v} got:{%v}", valid.wantStatus.Err(), got.Err()) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got.Err(), valid.wantStatus.Err()) } } func (s) TestSDKFileWatcher_RecoversFromReloadFailure(t *testing.T) { - valid1 := sdkTests["DeniesRpcMatchInDenyAndAllow"] + valid1 := sdkTests["DeniesRPCMatchInDenyAndAllow"] file := createTmpPolicyFile(t, "recovers_from_reload_failure", []byte(valid1.authzPolicy)) i, _ := authz.NewFileWatcher(file, 100*time.Millisecond) defer i.Close() @@ -693,7 +691,7 @@ func (s) TestSDKFileWatcher_RecoversFromReloadFailure(t *testing.T) { // Verifying authorization decision. _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) if got := status.Convert(err); got.Code() != valid1.wantStatus.Code() || got.Message() != valid1.wantStatus.Message() { - t.Fatalf("error want:{%v} got:{%v}", valid1.wantStatus.Err(), got.Err()) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got.Err(), valid1.wantStatus.Err()) } // Skips the invalid policy update, and continues to use the valid policy. @@ -707,17 +705,17 @@ func (s) TestSDKFileWatcher_RecoversFromReloadFailure(t *testing.T) { // Verifying authorization decision. _, err = client.UnaryCall(ctx, &pb.SimpleRequest{}) if got := status.Convert(err); got.Code() != valid1.wantStatus.Code() || got.Message() != valid1.wantStatus.Message() { - t.Fatalf("error want:{%v} got:{%v}", valid1.wantStatus.Err(), got.Err()) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got.Err(), valid1.wantStatus.Err()) } // Rewrite the file with a different valid authorization policy. - valid2 := sdkTests["AllowsRpcEmptyDenyMatchInAllow"] + valid2 := sdkTests["AllowsRPCEmptyDenyMatchInAllow"] if err := ioutil.WriteFile(file, []byte(valid2.authzPolicy), os.ModePerm); err != nil { t.Fatalf("ioutil.WriteFile(%q) failed: %v", file, err) } // Verifying authorization decision. if got := retryUntil(ctx, client, valid2.wantStatus); got != nil { - t.Fatalf("error want:{%v} got:{%v}", valid2.wantStatus.Err(), got) + t.Fatalf("client.UnaryCall(_, _) = %v; want = %v", got, valid2.wantStatus.Err()) } } diff --git a/internal/xds/rbac/rbac_engine.go b/internal/xds/rbac/rbac_engine.go index 35b82747a2d..ecb8512ac51 100644 --- a/internal/xds/rbac/rbac_engine.go +++ b/internal/xds/rbac/rbac_engine.go @@ -222,7 +222,7 @@ type rpcData struct { destinationPort uint32 // localAddr is the address that the RPC is being sent to. localAddr net.Addr - // authType is the type of authentication. + // authType is the type of authentication e.g. "tls". authType string // certs are the certificates presented by the peer during a TLS // handshake.