Skip to content

Commit

Permalink
Partial revert of negative regexps in RBAC labels
Browse files Browse the repository at this point in the history
This change was not backwards compatible - variable interpolation should
work in node_labels.

This commit partially reverts
#4253 and
#4430
  • Loading branch information
Andrew Lytvynov authored and russjones committed Oct 16, 2020
1 parent 8d8d39c commit 91fab7c
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 91 deletions.
8 changes: 0 additions & 8 deletions docs/4.4/enterprise/ssh-rbac.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,7 @@ spec:
'environment': ['test', 'staging']
# regular expressions are also supported, for example the equivalent
# of the list example above can be expressed as:
'environment': '{{regexp.match("^test|staging$")}}'
# or using the simpler legacy syntax:
'environment': '^test|staging$'
# negative regular expressions can be used to avoid strict deny rules:
'environment': '{{regexp.not_match("prod")}}'
# defines roles that this user can can request.
# needed for teleport's request workflow
Expand Down Expand Up @@ -261,11 +257,7 @@ spec:
'environment': ['test', 'staging']
# regular expressions are also supported, for example the equivalent
# of the list example above can be expressed as:
'environment': '{{regexp.match("^test|staging$")}}'
# or using the simpler legacy syntax:
'environment': '^test|staging$'
# negative regular expressions can be used to avoid strict deny rules:
'environment': '{{regexp.not_match("prod")}}'
```
Expand Down
30 changes: 5 additions & 25 deletions lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,18 +710,6 @@ func (r *RoleV3) CheckAndSetDefaults() error {
if key == Wildcard && !(len(val) == 1 && val[0] == Wildcard) {
return trace.BadParameter("selector *:<val> is not supported")
}
for _, l := range val {
if _, err := parse.NewMatcher(l); err != nil {
return trace.Wrap(err)
}
}
}
for _, val := range r.Spec.Deny.NodeLabels {
for _, l := range val {
if _, err := parse.NewMatcher(l); err != nil {
return trace.Wrap(err)
}
}
}
for i := range r.Spec.Allow.Rules {
err := r.Spec.Allow.Rules[i].CheckAndSetDefaults()
Expand Down Expand Up @@ -1597,19 +1585,11 @@ func MatchLabels(selector Labels, target map[string]string) (bool, string, error
}

if !utils.SliceContainsStr(selectorValues, Wildcard) {
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
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
}
}
}
Expand Down
58 changes: 0 additions & 58 deletions lib/services/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,64 +749,6 @@ 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 {

Expand Down
2 changes: 2 additions & 0 deletions lib/utils/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// TODO(awly): combine Expression and Matcher. It should be possible to write:
// `{{regexp.match(email.local(external.trait_name))}}`
package parse

import (
Expand Down
27 changes: 27 additions & 0 deletions lib/utils/replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,32 @@ 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(`\$[^\$]+`)

0 comments on commit 91fab7c

Please sign in to comment.