Skip to content

Commit

Permalink
security/authorization: add integration tests (#3809)
Browse files Browse the repository at this point in the history 
* security/authorization: integration tests for using CEL engine library
  • Loading branch information
Elizabeth Zou authored Aug 26, 2020
1 parent c4ba4cc commit cc8e63c
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 62 deletions.
7 changes: 4 additions & 3 deletions security/authorization/engine/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"strconv"

pb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v2"
cel "github.com/google/cel-go/cel"
"github.com/google/cel-go/cel"
"github.com/google/cel-go/checker/decls"
types "github.com/google/cel-go/common/types"
interpreter "github.com/google/cel-go/interpreter"
"github.com/google/cel-go/common/types"
"github.com/google/cel-go/interpreter"
expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/metadata"
Expand Down Expand Up @@ -324,6 +324,7 @@ func NewAuthorizationEngine(allow, deny *pb.RBAC) (*AuthorizationEngine, error)
decls.NewVar("destination.address", decls.String),
decls.NewVar("destination.port", decls.Int),
decls.NewVar("connection.uri_san_peer_certificate", decls.String),
decls.NewVar("source.principal", decls.String),
),
)
if err != nil {
Expand Down
242 changes: 192 additions & 50 deletions security/authorization/engine/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,30 @@ import (
"testing"

pb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v2"
cel "github.com/google/cel-go/cel"
"github.com/google/cel-go/cel"
"github.com/google/cel-go/checker/decls"
"github.com/google/cel-go/common/types"
"github.com/google/cel-go/common/types/ref"
interpreter "github.com/google/cel-go/interpreter"
"github.com/google/cel-go/interpreter"
"github.com/google/go-cmp/cmp"
expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/internal/grpctest"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"
)

type programMock struct {
type s struct {
grpctest.Tester
}

type fakeProgram struct {
out ref.Val
err error
}

func (mock programMock) Eval(vars interface{}) (ref.Val, *cel.EvalDetails, error) {
return mock.out, nil, mock.err
func (fake fakeProgram) Eval(vars interface{}) (ref.Val, *cel.EvalDetails, error) {
return fake.out, nil, fake.err
}

type valMock struct {
Expand Down Expand Up @@ -66,12 +75,24 @@ func (mock valMock) Value() interface{} {
return mock.val
}

type addrMock struct {
addr string
}

func (mock addrMock) Network() string {
return "tcp"
}

func (mock addrMock) String() string {
return mock.addr
}

var (
emptyActivation = interpreter.EmptyActivation()
unsuccessfulProgram = programMock{out: nil, err: status.Errorf(codes.InvalidArgument, "Unsuccessful program evaluation")}
errProgram = programMock{out: valMock{"missing attributes"}, err: status.Errorf(codes.InvalidArgument, "Successful program evaluation to an error result -- missing attributes")}
trueProgram = programMock{out: valMock{true}, err: nil}
falseProgram = programMock{out: valMock{false}, err: nil}
unsuccessfulProgram = fakeProgram{out: nil, err: status.Errorf(codes.InvalidArgument, "Unsuccessful program evaluation")}
errProgram = fakeProgram{out: valMock{"missing attributes"}, err: status.Errorf(codes.InvalidArgument, "Successful program evaluation to an error result -- missing attributes")}
trueProgram = fakeProgram{out: valMock{true}, err: nil}
falseProgram = fakeProgram{out: valMock{false}, err: nil}

allowMatchEngine = &policyEngine{action: pb.RBAC_ALLOW, programs: map[string]cel.Program{
"allow match policy1": unsuccessfulProgram,
Expand All @@ -92,59 +113,52 @@ var (
}}
)

func TestNewAuthorizationEngine(t *testing.T) {
func Test(t *testing.T) {
grpctest.RunSubTests(t, s{})
}

func (s) TestNewAuthorizationEngine(t *testing.T) {
tests := map[string]struct {
allow *pb.RBAC
deny *pb.RBAC
wantErr string
wantErr bool
errStr string
}{
"too few rbacs": {
allow: nil,
deny: nil,
wantErr: "at least one of allow, deny must be non-nil",
wantErr: true,
errStr: "Expected error: at least one of allow, deny must be non-nil",
},
"one rbac allow": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW},
deny: nil,
wantErr: "",
errStr: "Expected 1 ALLOW RBAC to be successful",
allow: &pb.RBAC{Action: pb.RBAC_ALLOW},
errStr: "Expected 1 ALLOW RBAC to be successful",
},
"one rbac deny": {
allow: nil,
deny: &pb.RBAC{Action: pb.RBAC_DENY},
wantErr: "",
errStr: "Expected 1 DENY RBAC to be successful",
deny: &pb.RBAC{Action: pb.RBAC_DENY},
errStr: "Expected 1 DENY RBAC to be successful",
},
"two rbacs": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW},
deny: &pb.RBAC{Action: pb.RBAC_DENY},
wantErr: "",
errStr: "Expected 2 RBACs (DENY + ALLOW) to be successful",
allow: &pb.RBAC{Action: pb.RBAC_ALLOW},
deny: &pb.RBAC{Action: pb.RBAC_DENY},
errStr: "Expected 2 RBACs (DENY + ALLOW) to be successful",
},
"wrong rbac actions": {
allow: &pb.RBAC{Action: pb.RBAC_DENY},
deny: nil,
wantErr: "allow must have action ALLOW, deny must have action DENY",
wantErr: true,
errStr: "Expected error: allow must have action ALLOW, deny must have action DENY",
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
_, gotErr := NewAuthorizationEngine(tc.allow, tc.deny)
if tc.wantErr == "" && gotErr == nil {
return
}
if gotErr == nil || gotErr.Error() != tc.wantErr {
t.Errorf(tc.errStr)
if (gotErr != nil) != tc.wantErr {
t.Fatal(tc.errStr)
}
})
}
}

func TestGetDecision(t *testing.T) {
func (s) TestGetDecision(t *testing.T) {
tests := map[string]struct {
engine *policyEngine
match bool
Expand All @@ -157,7 +171,6 @@ func TestGetDecision(t *testing.T) {
},
"ALLOW engine fail": {
engine: &policyEngine{action: pb.RBAC_ALLOW, programs: map[string]cel.Program{}},
match: false,
want: DecisionDeny,
},
"DENY engine match": {
Expand All @@ -167,21 +180,20 @@ func TestGetDecision(t *testing.T) {
},
"DENY engine fail": {
engine: &policyEngine{action: pb.RBAC_DENY, programs: map[string]cel.Program{}},
match: false,
want: DecisionAllow,
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
if got := getDecision(tc.engine, tc.match); got != tc.want {
t.Errorf("Expected %v, instead got %v", tc.want, got)
t.Fatalf("getDecision(%v, %v) = (%v), want (%v)", tc.engine, tc.match, got, tc.want)
}
})
}
}

func TestPolicyEngineEvaluate(t *testing.T) {
func (s) TestPolicyEngineEvaluate(t *testing.T) {
tests := map[string]struct {
engine *policyEngine
activation interpreter.Activation
Expand Down Expand Up @@ -218,54 +230,184 @@ func TestPolicyEngineEvaluate(t *testing.T) {
t.Run(name, func(t *testing.T) {
gotDecision, gotPolicyNames := tc.engine.evaluate(tc.activation)
sort.Strings(gotPolicyNames)
if gotDecision != tc.wantDecision || !reflect.DeepEqual(gotPolicyNames, tc.wantPolicyNames) {
t.Errorf("Expected (%v, %v), instead got (%v, %v)", tc.wantDecision, tc.wantPolicyNames, gotDecision, gotPolicyNames)
if gotDecision != tc.wantDecision || !cmp.Equal(gotPolicyNames, tc.wantPolicyNames) {
t.Fatalf("policyEngine.evaluate(%v, %v) = (%v, %v), want (%v, %v)", tc.engine, tc.activation, gotDecision, gotPolicyNames, tc.wantDecision, tc.wantPolicyNames)
}
})
}
}

func TestAuthorizationEngineEvaluate(t *testing.T) {
func (s) TestAuthorizationEngineEvaluate(t *testing.T) {
tests := map[string]struct {
engine *AuthorizationEngine
authArgs *AuthorizationArgs
wantAuthDecision *AuthorizationDecision
wantErr error
}{
"allow match": {
engine: &AuthorizationEngine{allow: allowMatchEngine},
authArgs: &AuthorizationArgs{},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"allow match policy2"}},
wantErr: nil,
},
"deny fail": {
engine: &AuthorizationEngine{deny: denyFailEngine},
authArgs: &AuthorizationArgs{},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{}},
wantErr: nil,
},
"first engine unknown": {
engine: &AuthorizationEngine{allow: allowMatchEngine, deny: denyUnknownEngine},
authArgs: &AuthorizationArgs{},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"deny unknown policy2", "deny unknown policy3"}},
wantErr: nil,
},
"second engine match": {
engine: &AuthorizationEngine{allow: allowMatchEngine, deny: denyFailEngine},
authArgs: &AuthorizationArgs{},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"allow match policy2"}},
wantErr: nil,
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
gotAuthDecision, gotErr := tc.engine.Evaluate(tc.authArgs)
sort.Strings(gotAuthDecision.policyNames)
if tc.wantErr != nil && (gotErr == nil || gotErr.Error() != tc.wantErr.Error()) {
t.Errorf("Expected error to be %v, instead got %v", tc.wantErr, gotErr)
} else if tc.wantErr == nil && (gotErr != nil || gotAuthDecision.decision != tc.wantAuthDecision.decision || !reflect.DeepEqual(gotAuthDecision.policyNames, tc.wantAuthDecision.policyNames)) {
t.Errorf("Expected authorization decision to be (%v, %v), instead got (%v, %v)", tc.wantAuthDecision.decision, tc.wantAuthDecision.policyNames, gotAuthDecision.decision, gotAuthDecision.policyNames)
if gotErr != nil || gotAuthDecision.decision != tc.wantAuthDecision.decision || !cmp.Equal(gotAuthDecision.policyNames, tc.wantAuthDecision.policyNames) {
t.Fatalf("AuthorizationEngine.Evaluate(%v, %v) = (%v, %v), want (%v, %v)", tc.engine, tc.authArgs, gotAuthDecision, gotErr, tc.wantAuthDecision, nil)
}
})
}
}

func (s) TestIntegration(t *testing.T) {
declarations := []*expr.Decl{
decls.NewVar("request.url_path", decls.String),
decls.NewVar("request.host", decls.String),
decls.NewVar("request.method", decls.String),
decls.NewVar("request.headers", decls.NewMapType(decls.String, decls.String)),
decls.NewVar("source.address", decls.String),
decls.NewVar("source.port", decls.Int),
decls.NewVar("destination.address", decls.String),
decls.NewVar("destination.port", decls.Int),
decls.NewVar("connection.uri_san_peer_certificate", decls.String),
decls.NewVar("source.principal", decls.String),
}

tests := map[string]struct {
allow *pb.RBAC
deny *pb.RBAC
authArgs *AuthorizationArgs
wantAuthDecision *AuthorizationDecision
}{
"ALLOW engine: DecisionAllow": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path starts with": {Condition: compileStringToExpr("request.url_path.startsWith('/pkg.service/test')", declarations)},
}},
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"url_path starts with"}},
},
"ALLOW engine: DecisionUnknown": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path and uri_san_peer_certificate": {Condition: compileStringToExpr("request.url_path == '/pkg.service/test' && connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", declarations)},
"source port": {Condition: compileStringToExpr("source.port == 8080", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:25"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"url_path and uri_san_peer_certificate"}},
},
"ALLOW engine: DecisionDeny": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path": {Condition: compileStringToExpr("request.url_path == '/pkg.service/test'", declarations)},
}},
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{}},
},
"DENY engine: DecisionAllow": {
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"url_path and uri_san_peer_certificate": {Condition: compileStringToExpr("request.url_path == '/pkg.service/test' && connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", declarations)},
}},
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{}},
},
"DENY engine: DecisionUnknown": {
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"destination address": {Condition: compileStringToExpr("destination.address == '192.0.3.1'", declarations)},
"source port": {Condition: compileStringToExpr("source.port == 8080", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:25"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"destination address"}},
},
"DENY engine: DecisionDeny": {
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"destination address": {Condition: compileStringToExpr("destination.address == '192.0.3.1'", declarations)},
"source address or source port": {Condition: compileStringToExpr("source.address == '192.0.4.1' || source.port == 8080", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{"source address or source port"}},
},
"DENY ALLOW engine: DecisionDeny from DENY policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path starts with": {Condition: compileStringToExpr("request.url_path.startsWith('/pkg.service/test')", declarations)},
}},
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"destination address": {Condition: compileStringToExpr("destination.address == '192.0.3.1'", declarations)},
"source address or source port": {Condition: compileStringToExpr("source.address == '192.0.4.1' || source.port == 8080", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{"source address or source port"}},
},
"DENY ALLOW engine: DecisionUnknown from DENY policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path starts with": {Condition: compileStringToExpr("request.url_path.startsWith('/pkg.service/test')", declarations)},
}},
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"destination address": {Condition: compileStringToExpr("destination.address == '192.0.3.1'", declarations)},
"source port and destination port": {Condition: compileStringToExpr("source.port == 8080 && destination.port == 1234", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"destination address", "source port and destination port"}},
},
"DENY ALLOW engine: DecisionAllow from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"method or url_path starts with": {Condition: compileStringToExpr("request.method == 'POST' || request.url_path.startsWith('/pkg.service/test')", declarations)},
}},
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"source address": {Condition: compileStringToExpr("source.address == '192.0.3.1'", declarations)},
"source port and url_path": {Condition: compileStringToExpr("source.port == 8080 && request.url_path == 'pkg.service/test'", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"method or url_path starts with"}},
},
"DENY ALLOW engine: DecisionUnknown from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path starts with and method": {Condition: compileStringToExpr("request.url_path.startsWith('/pkg.service/test') && request.method == 'POST'", declarations)},
}},
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"source address": {Condition: compileStringToExpr("source.address == '192.0.3.1'", declarations)},
"source port and url_path": {Condition: compileStringToExpr("source.port == 8080 && request.url_path == 'pkg.service/test'", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"url_path starts with and method"}},
},
"DENY ALLOW engine: DecisionDeny from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
"url_path starts with and source port": {Condition: compileStringToExpr("request.url_path.startsWith('/pkg.service/test') && source.port == 1234", declarations)},
}},
deny: &pb.RBAC{Action: pb.RBAC_DENY, Policies: map[string]*pb.Policy{
"source address": {Condition: compileStringToExpr("source.address == '192.0.3.1'", declarations)},
"source port and url_path": {Condition: compileStringToExpr("source.port == 8080 && request.url_path == 'pkg.service/test'", declarations)},
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{}},
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
engine, err := NewAuthorizationEngine(tc.allow, tc.deny)
if err != nil {
t.Fatalf("Error constructing authorization engine: %v", err)
}
gotAuthDecision, gotErr := engine.Evaluate(tc.authArgs)
sort.Strings(gotAuthDecision.policyNames)
if gotErr != nil || gotAuthDecision.decision != tc.wantAuthDecision.decision || !cmp.Equal(gotAuthDecision.policyNames, tc.wantAuthDecision.policyNames) {
t.Fatalf("NewAuthorizationEngine(%v, %v).Evaluate(%v) = (%v, %v), want (%v, %v)", tc.allow, tc.deny, tc.authArgs, gotAuthDecision, gotErr, tc.wantAuthDecision, nil)
}
})
}
Expand Down
Loading

0 comments on commit cc8e63c

Please sign in to comment.