From 2aca6581dc175dbb3728828cb6d593ae28578683 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Sat, 15 Aug 2020 20:32:46 -0700 Subject: [PATCH 1/2] security/authorization: add the util function to compile string cel expr --- security/authorization/engine/engine.go | 2 +- security/authorization/util/util.go | 63 ++++++++++++ security/authorization/util/util_test.go | 123 +++++++++++++++++++++++ 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 security/authorization/util/util.go create mode 100644 security/authorization/util/util_test.go diff --git a/security/authorization/engine/engine.go b/security/authorization/engine/engine.go index e964c89b6821..d598ff409fa6 100644 --- a/security/authorization/engine/engine.go +++ b/security/authorization/engine/engine.go @@ -33,7 +33,7 @@ import ( "google.golang.org/protobuf/proto" ) -var logger = grpclog.Component("channelz") +var logger = grpclog.Component("authorization") var stringAttributeMap = map[string]func(*AuthorizationArgs) (string, error){ "request.url_path": (*AuthorizationArgs).getRequestURLPath, diff --git a/security/authorization/util/util.go b/security/authorization/util/util.go new file mode 100644 index 000000000000..8aadad3862ba --- /dev/null +++ b/security/authorization/util/util.go @@ -0,0 +1,63 @@ +/* + * + * Copyright 2020 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 engine + +import ( + "errors" + + expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + "google.golang.org/protobuf/proto" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/checker/decls" +) + +func compileCel(env *cel.Env, expr string) (*cel.Ast, error) { + ast, iss := env.Parse(expr) + // Report syntactic errors, if present. + if iss.Err() != nil { + return nil, iss.Err() + } + // Type-check the expression for correctness. + checked, iss := env.Check(ast) + if iss.Err() != nil { + return nil, iss.Err() + } + // Check the result type is a Boolean. + if !proto.Equal(checked.ResultType(), decls.Bool) { + return nil, errors.New("failed to compile CEL string: get non-bool value") + } + return checked, nil +} + +func compileStringToCheckedExpr(expr string, declarations []*expr.Decl) (*expr.CheckedExpr, error) { + env, err := cel.NewEnv(cel.Declarations(declarations...)) + if err != nil { + return nil, err + } + checked, err := compileCel(env, expr) + if err != nil { + return nil, err + } + checkedExpr, err := cel.AstToCheckedExpr(checked) + if err != nil { + return nil, err + } + return checkedExpr, nil +} diff --git a/security/authorization/util/util_test.go b/security/authorization/util/util_test.go new file mode 100644 index 000000000000..5e32424581ee --- /dev/null +++ b/security/authorization/util/util_test.go @@ -0,0 +1,123 @@ +/* + * + * Copyright 2020 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 engine + +import ( + "testing" + + expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + "google.golang.org/grpc/internal/grpctest" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/checker/decls" +) + +type s struct { + grpctest.Tester +} + +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) +} + +func (s) TestStringConvert(t *testing.T) { + declarations := []*expr.Decl{ + decls.NewIdent("request.url_path", decls.String, nil), + decls.NewIdent("request.host", decls.String, nil), + decls.NewIdent("connection.uri_san_peer_certificate", decls.String, nil)} + env, err := cel.NewEnv() + if err != nil { + t.Fatalf("Failed to create the CEL environment") + } + for _, test := range []struct { + desc string + wantEvalOutcome bool + wantParsingError bool + wantEvalError bool + expr string + authzArgs map[string]interface{} + }{ + { + desc: "Test 1 single primitive match", + wantEvalOutcome: true, + expr: "request.url_path.startsWith('/pkg.service/test')", + authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test"}, + }, + { + desc: "Test 2 single compare match", + wantEvalOutcome: true, + expr: "connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", + authzArgs: map[string]interface{}{"connection.uri_san_peer_certificate": "cluster/ns/default/sa/admin"}, + }, + { + desc: "Test 3 single primitive no match", + wantEvalOutcome: false, + expr: "request.url_path.startsWith('/pkg.service/test')", + authzArgs: map[string]interface{}{"request.url_path": "/source/pkg.service/test"}, + }, + { + desc: "Test 4 primitive and compare match", + wantEvalOutcome: true, + expr: "request.url_path == '/pkg.service/test' && connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", + authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test", + "connection.uri_san_peer_certificate": "cluster/ns/default/sa/admin"}, + }, + { + desc: "Test 5 parse error field not present in environment", + wantParsingError: true, + expr: "request.source_path.startsWith('/pkg.service/test')", + authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test"}, + }, + { + desc: "Test 6 eval error argument not included in environment", + wantEvalError: true, + expr: "request.url_path.startsWith('/pkg.service/test')", + authzArgs: map[string]interface{}{"request.source_path": "/pkg.service/test"}, + }, + } { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + checked, err := compileStringToCheckedExpr(test.expr, declarations) + want, got := test.wantParsingError, err != nil + if got != want { + t.Fatalf("Error mismatch in conversion, test.wantParsingError =%v, got %v", want, got) + } + if test.wantParsingError { + return + } + ast := cel.CheckedExprToAst(checked) + program, err := env.Program(ast) + if err != nil { + t.Fatalf("Failed to create CEL Program: %v", err) + } + eval, _, err := program.Eval(test.authzArgs) + want, got = test.wantEvalError, err != nil + if got != want { + t.Fatalf("Error mismatch in evaluation, test.wantEvalError =%v, got %v", want, got) + } + if test.wantEvalError { + return + } + if eval.Value() != test.wantEvalOutcome { + t.Fatalf("Error in evaluating converted CheckedExpr: want %v, got %v", test.wantEvalOutcome, eval.Value()) + } + }) + } +} From b6fcd418faa0a9a0ce59ef0385d6cea4db023f59 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 18 Aug 2020 10:17:11 -0700 Subject: [PATCH 2/2] fix comments --- security/authorization/util/util_test.go | 26 +++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/security/authorization/util/util_test.go b/security/authorization/util/util_test.go index 5e32424581ee..981a00864d58 100644 --- a/security/authorization/util/util_test.go +++ b/security/authorization/util/util_test.go @@ -40,7 +40,8 @@ func (s) TestStringConvert(t *testing.T) { declarations := []*expr.Decl{ decls.NewIdent("request.url_path", decls.String, nil), decls.NewIdent("request.host", decls.String, nil), - decls.NewIdent("connection.uri_san_peer_certificate", decls.String, nil)} + decls.NewIdent("connection.uri_san_peer_certificate", decls.String, nil), + } env, err := cel.NewEnv() if err != nil { t.Fatalf("Failed to create the CEL environment") @@ -54,38 +55,38 @@ func (s) TestStringConvert(t *testing.T) { authzArgs map[string]interface{} }{ { - desc: "Test 1 single primitive match", + desc: "single primitive match", wantEvalOutcome: true, expr: "request.url_path.startsWith('/pkg.service/test')", authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test"}, }, { - desc: "Test 2 single compare match", + desc: "single compare match", wantEvalOutcome: true, expr: "connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", authzArgs: map[string]interface{}{"connection.uri_san_peer_certificate": "cluster/ns/default/sa/admin"}, }, { - desc: "Test 3 single primitive no match", + desc: "single primitive no match", wantEvalOutcome: false, expr: "request.url_path.startsWith('/pkg.service/test')", authzArgs: map[string]interface{}{"request.url_path": "/source/pkg.service/test"}, }, { - desc: "Test 4 primitive and compare match", + desc: "primitive and compare match", wantEvalOutcome: true, expr: "request.url_path == '/pkg.service/test' && connection.uri_san_peer_certificate == 'cluster/ns/default/sa/admin'", authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test", "connection.uri_san_peer_certificate": "cluster/ns/default/sa/admin"}, }, { - desc: "Test 5 parse error field not present in environment", + desc: "parse error field not present in environment", wantParsingError: true, expr: "request.source_path.startsWith('/pkg.service/test')", authzArgs: map[string]interface{}{"request.url_path": "/pkg.service/test"}, }, { - desc: "Test 6 eval error argument not included in environment", + desc: "eval error argument not included in environment", wantEvalError: true, expr: "request.url_path.startsWith('/pkg.service/test')", authzArgs: map[string]interface{}{"request.source_path": "/pkg.service/test"}, @@ -93,11 +94,9 @@ func (s) TestStringConvert(t *testing.T) { } { test := test t.Run(test.desc, func(t *testing.T) { - t.Parallel() checked, err := compileStringToCheckedExpr(test.expr, declarations) - want, got := test.wantParsingError, err != nil - if got != want { - t.Fatalf("Error mismatch in conversion, test.wantParsingError =%v, got %v", want, got) + if (err != nil) != test.wantParsingError { + t.Fatalf("Error mismatch in conversion, wantParsingError =%v, got %v", test.wantParsingError, err != nil) } if test.wantParsingError { return @@ -108,9 +107,8 @@ func (s) TestStringConvert(t *testing.T) { t.Fatalf("Failed to create CEL Program: %v", err) } eval, _, err := program.Eval(test.authzArgs) - want, got = test.wantEvalError, err != nil - if got != want { - t.Fatalf("Error mismatch in evaluation, test.wantEvalError =%v, got %v", want, got) + if (err != nil) != test.wantEvalError { + t.Fatalf("Error mismatch in evaluation, wantEvalError =%v, got %v", test.wantEvalError, err != nil) } if test.wantEvalError { return