Skip to content

Commit

Permalink
authz: fix regex expression match (grpc#5035)
Browse files Browse the repository at this point in the history
* Fixes regex expression matching.

* Adds tests

* Updates FulMatchWithRegex and regex string for presence match.

* Add tests for FullMatchWithRegex

* Update regex to allow whitespace characters
  • Loading branch information
ashithasantosh authored Dec 9, 2021
1 parent fd4e3bd commit 5d90b32
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 14 deletions.
8 changes: 5 additions & 3 deletions authz/rbac_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ func getStringMatcher(value string) *v3matcherpb.StringMatcher {
switch {
case value == "*":
return &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{},
MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{
SafeRegex: &v3matcherpb.RegexMatcher{Regex: ".+"}},
}
case strings.HasSuffix(value, "*"):
prefix := strings.TrimSuffix(value, "*")
Expand All @@ -117,8 +118,9 @@ func getHeaderMatcher(key, value string) *v3routepb.HeaderMatcher {
switch {
case value == "*":
return &v3routepb.HeaderMatcher{
Name: key,
HeaderMatchSpecifier: &v3routepb.HeaderMatcher_SafeRegexMatch{},
Name: key,
HeaderMatchSpecifier: &v3routepb.HeaderMatcher_SafeRegexMatch{
SafeRegexMatch: &v3matcherpb.RegexMatcher{Regex: ".+"}},
}
case strings.HasSuffix(value, "*"):
prefix := strings.TrimSuffix(value, "*")
Expand Down
2 changes: 1 addition & 1 deletion authz/rbac_translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestTranslatePolicy(t *testing.T) {
Ids: []*v3rbacpb.Principal{
{Identifier: &v3rbacpb.Principal_Authenticated_{
Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{
MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{},
MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{SafeRegex: &v3matcherpb.RegexMatcher{Regex: ".+"}},
}},
}},
},
Expand Down
40 changes: 34 additions & 6 deletions authz/sdk_end2end_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ var sdkTests = map[string]struct {
"request": {
"paths":
[
"/grpc.testing.TestService/UnaryCall",
"/grpc.testing.TestService/StreamingInputCall"
"/grpc.testing.TestService/*"
],
"headers":
[
Expand All @@ -122,23 +121,23 @@ var sdkTests = map[string]struct {
"allow_rules":
[
{
"name": "allow_TestServiceCalls",
"name": "allow_all",
"request": {
"paths":
[
"/grpc.testing.TestService/*"
"*"
]
}
}
],
"deny_rules":
[
{
"name": "deny_TestServiceCalls",
"name": "deny_all",
"request": {
"paths":
[
"/grpc.testing.TestService/*"
"*"
]
}
}
Expand Down Expand Up @@ -300,6 +299,35 @@ var sdkTests = map[string]struct {
}`,
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
},
"DeniesRPCRequestNoMatchInAllowFailsPresenceMatch": {
authzPolicy: `{
"name": "authz",
"allow_rules":
[
{
"name": "allow_TestServiceCalls",
"request": {
"paths":
[
"/grpc.testing.TestService/*"
],
"headers":
[
{
"key": "key-abc",
"values":
[
"*"
]
}
]
}
}
]
}`,
md: metadata.Pairs("key-abc", ""),
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
},
}

func (s) TestSDKStaticPolicyEnd2End(t *testing.T) {
Expand Down
11 changes: 7 additions & 4 deletions internal/grpcutil/regex.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ package grpcutil

import "regexp"

// FullMatchWithRegex returns whether the full string matches the regex provided.
func FullMatchWithRegex(re *regexp.Regexp, string string) bool {
// FullMatchWithRegex returns whether the full text matches the regex provided.
func FullMatchWithRegex(re *regexp.Regexp, text string) bool {
if len(text) == 0 {
return re.MatchString(text)
}
re.Longest()
rem := re.FindString(string)
return len(rem) == len(string)
rem := re.FindString(text)
return len(rem) == len(text)
}
12 changes: 12 additions & 0 deletions internal/grpcutil/regex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ func TestFullMatchWithRegex(t *testing.T) {
string: "ab",
want: true,
},
{
name: "match all",
regexStr: ".*",
string: "",
want: true,
},
{
name: "matches non-empty strings",
regexStr: ".+",
string: "",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 5d90b32

Please sign in to comment.