From 45cf28b0fed9d57291d5eb0e54f627a68ff9739a Mon Sep 17 00:00:00 2001 From: Andrew Lytvynov Date: Thu, 17 Sep 2020 13:18:45 -0700 Subject: [PATCH] Migrate services.MatchLabels to parse.Matcher This should be backwards-compatible plus add the {{regexp.match(...)}} and {{regexp.not_match(...)}} functions. --- lib/services/role.go | 22 ++++++++----- lib/services/role_test.go | 58 +++++++++++++++++++++++++++++++++++ lib/services/user.go | 2 +- lib/utils/parse/parse.go | 12 ++++---- lib/utils/parse/parse_test.go | 4 +-- lib/utils/replace.go | 27 ---------------- 6 files changed, 82 insertions(+), 43 deletions(-) diff --git a/lib/services/role.go b/lib/services/role.go index b8d3532c19d72..790a19ba6c426 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -385,7 +385,7 @@ func ApplyTraits(r Role, traits map[string][]string) Role { // at least one value in case if return value is nil func applyValueTraits(val string, traits map[string][]string) ([]string, error) { // Extract the variable from the role variable. - variable, err := parse.Variable(val) + variable, err := parse.NewExpression(val) if err != nil { return nil, trace.Wrap(err) } @@ -687,7 +687,7 @@ func (r *RoleV3) CheckAndSetDefaults() error { for _, condition := range []RoleConditionType{Allow, Deny} { for _, login := range r.GetLogins(condition) { if strings.Contains(login, "{{") || strings.Contains(login, "}}") { - _, err := parse.Variable(login) + _, err := parse.NewExpression(login) if err != nil { return trace.BadParameter("invalid login found: %v", login) } @@ -1585,11 +1585,19 @@ func MatchLabels(selector Labels, target map[string]string) (bool, string, error } if !utils.SliceContainsStr(selectorValues, Wildcard) { - result, err := utils.SliceMatchesRegex(targetVal, selectorValues) - if err != nil { - return false, "", trace.Wrap(err) - } else if !result { - return false, fmt.Sprintf("no value match: got '%v' want: '%v'", targetVal, selectorValues), nil + matched := false + for _, expr := range selectorValues { + m, err := parse.NewMatcher(expr) + if err != nil { + return false, "", trace.Wrap(err) + } + if m.Match(targetVal) { + matched = true + break + } + } + if !matched { + return false, fmt.Sprintf("no value match: got %q want: %q", targetVal, selectorValues), nil } } } diff --git a/lib/services/role_test.go b/lib/services/role_test.go index faf90e07dc743..022b23a528d85 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -749,6 +749,64 @@ func (s *RoleSuite) TestCheckAccess(c *C) { {server: serverC2, login: "admin", hasAccess: true}, }, }, + { + name: "role matches a regexp label", + roles: []RoleV3{ + RoleV3{ + Metadata: Metadata{ + Name: "name1", + Namespace: defaults.Namespace, + }, + Spec: RoleSpecV3{ + Options: RoleOptions{ + MaxSessionTTL: Duration(20 * time.Hour), + }, + Allow: RoleConditions{ + Logins: []string{"admin"}, + NodeLabels: Labels{"role": []string{`{{regexp.match("worker.*")}}`}}, + Namespaces: []string{defaults.Namespace, namespaceC}, + }, + }, + }, + }, + checks: []check{ + {server: serverA, login: "root", hasAccess: false}, + {server: serverA, login: "admin", hasAccess: false}, + {server: serverB, login: "root", hasAccess: false}, + {server: serverB, login: "admin", hasAccess: true}, + {server: serverC, login: "root", hasAccess: false}, + {server: serverC, login: "admin", hasAccess: false}, + }, + }, + { + name: "role matches a negative regexp label", + roles: []RoleV3{ + RoleV3{ + Metadata: Metadata{ + Name: "name1", + Namespace: defaults.Namespace, + }, + Spec: RoleSpecV3{ + Options: RoleOptions{ + MaxSessionTTL: Duration(20 * time.Hour), + }, + Allow: RoleConditions{ + Logins: []string{"admin"}, + NodeLabels: Labels{"role": []string{`{{regexp.not_match("db.*")}}`}}, + Namespaces: []string{defaults.Namespace, namespaceC}, + }, + }, + }, + }, + checks: []check{ + {server: serverA, login: "root", hasAccess: false}, + {server: serverA, login: "admin", hasAccess: false}, + {server: serverB, login: "root", hasAccess: false}, + {server: serverB, login: "admin", hasAccess: true}, + {server: serverC, login: "root", hasAccess: false}, + {server: serverC, login: "admin", hasAccess: false}, + }, + }, } for i, tc := range testCases { diff --git a/lib/services/user.go b/lib/services/user.go index dc28f98996aaf..99ede69238a5d 100644 --- a/lib/services/user.go +++ b/lib/services/user.go @@ -491,7 +491,7 @@ func (u *UserV1) Check() error { return trace.BadParameter("user name cannot be empty") } for _, login := range u.AllowedLogins { - e, err := parse.Variable(login) + e, err := parse.NewExpression(login) if err == nil && e.Namespace() != parse.LiteralNamespace { return trace.BadParameter("role variables not allowed in allowed logins") } diff --git a/lib/utils/parse/parse.go b/lib/utils/parse/parse.go index d44bb6beed7ee..fd1a0cfcf967c 100644 --- a/lib/utils/parse/parse.go +++ b/lib/utils/parse/parse.go @@ -112,10 +112,10 @@ var reVariable = regexp.MustCompile( `(?P[^}{]*)$`, ) -// Variable parses expressions like {{external.foo}} or {{internal.bar}}, or a -// literal value like "prod". Call Interpolate on the returned Expression to -// get the final value based on traits or other dynamic values. -func Variable(variable string) (*Expression, error) { +// NewExpression parses expressions like {{external.foo}} or {{internal.bar}}, +// or a literal value like "prod". Call Interpolate on the returned Expression +// to get the final value based on traits or other dynamic values. +func NewExpression(variable string) (*Expression, error) { match := reVariable.FindStringSubmatch(variable) if len(match) == 0 { if strings.Contains(variable, "{{") || strings.Contains(variable, "}}") { @@ -165,7 +165,7 @@ type Matcher interface { Match(in string) bool } -// Match parses a matcher expression. Currently supported expressions: +// NewMatcher parses a matcher expression. Currently supported expressions: // - string literal: `foo` // - wildcard expression: `*` or `foo*bar` // - regexp expression: `^foo$` @@ -175,7 +175,7 @@ type Matcher interface { // // These expressions do not support variable interpolation (e.g. // `{{internal.logins}}`), like Expression does. -func Match(value string) (Matcher, error) { +func NewMatcher(value string) (Matcher, error) { match := reVariable.FindStringSubmatch(value) if len(match) == 0 { if strings.Contains(value, "{{") || strings.Contains(value, "}}") { diff --git a/lib/utils/parse/parse_test.go b/lib/utils/parse/parse_test.go index 0751ed30ad1c7..607e63f0893bf 100644 --- a/lib/utils/parse/parse_test.go +++ b/lib/utils/parse/parse_test.go @@ -113,7 +113,7 @@ func TestVariable(t *testing.T) { for _, tt := range tests { t.Run(tt.title, func(t *testing.T) { - variable, err := Variable(tt.in) + variable, err := NewExpression(tt.in) if tt.err != nil { assert.IsType(t, tt.err, err) return @@ -269,7 +269,7 @@ func TestMatch(t *testing.T) { for _, tt := range tests { t.Run(tt.title, func(t *testing.T) { - matcher, err := Match(tt.in) + matcher, err := NewMatcher(tt.in) if tt.err != nil { assert.IsType(t, tt.err, err, err) return diff --git a/lib/utils/replace.go b/lib/utils/replace.go index a32c83e3b8c84..4d68ae7ed0f9d 100644 --- a/lib/utils/replace.go +++ b/lib/utils/replace.go @@ -46,32 +46,5 @@ func ReplaceRegexp(expression string, replaceWith string, input string) (string, return expr.ReplaceAllString(input, replaceWith), nil } -// SliceMatchesRegex checks if input matches any of the expressions. The -// match is always evaluated as a regex either an exact match or regexp. -func SliceMatchesRegex(input string, expressions []string) (bool, error) { - for _, expression := range expressions { - if !strings.HasPrefix(expression, "^") || !strings.HasSuffix(expression, "$") { - // replace glob-style wildcards with regexp wildcards - // for plain strings, and quote all characters that could - // be interpreted in regular expression - expression = "^" + GlobToRegexp(expression) + "$" - } - - expr, err := regexp.Compile(expression) - if err != nil { - return false, trace.BadParameter(err.Error()) - } - - // Since the expression is always surrounded by ^ and $ this is an exact - // match for either a a plain string (for example ^hello$) or for a regexp - // (for example ^hel*o$). - if expr.MatchString(input) { - return true, nil - } - } - - return false, nil -} - var replaceWildcard = regexp.MustCompile(`(\\\*)`) var reExpansion = regexp.MustCompile(`\$[^\$]+`)