Skip to content

Commit

Permalink
Add negative RBAC label matches
Browse files Browse the repository at this point in the history
Normal `node_labels` matches are positive only - you must provide an
exact label or a regexp that matches all valid labels.

This is problematic when you want to grant access to "any label value
except X". You could put "X" in the `deny` section of a role, but this
prevents any other roles from granting access to "X". For example, users
may have many dynamically-named environments and one "prod", and they
want to only grant access to "prod" via workflow API.

This commit adds support for `+regexp.not` function, for example
`+regexp.not(prod)`. The specific prefix was chosen because a valid
regexp can't start with `+`.
  • Loading branch information
Andrew Lytvynov committed Aug 26, 2020
1 parent dc05b40 commit c52a4ea
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 4 deletions.
5 changes: 5 additions & 0 deletions docs/4.3/enterprise/ssh_rbac.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ spec:
# regular expressions are also supported, for example the equivalent
# of the list example above can be expressed as:
'environment': '^test|staging$'
# negative matches use the '+not' prefix, this works with both exact
# matches and regular expressions.
# this is useful when another role may grant extended access and you
# don't want to exclude it entirely with a 'deny' rule below.
'environment': '+not production'
# defines roles that this user can can request.
# needed for teleport's request workflow
Expand Down
19 changes: 15 additions & 4 deletions lib/utils/replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,19 @@ func ReplaceRegexp(expression string, replaceWith string, input string) (string,

// SliceMatchesRegex checks if input matches any of the expressions. The
// match is always evaluated as a regex either an exact match or regexp.
//
// If any expression starts with "+not ", the match is negated.
func SliceMatchesRegex(input string, expressions []string) (bool, error) {
for _, expression := range expressions {
// Handle the "+not " prefix: flip the desired match outcome and remove
// it from the expression.
wantMatch := true
if strings.HasPrefix(expression, "+regexp.not(") && strings.HasSuffix(expression, ")") {
wantMatch = false
expression = strings.TrimPrefix(expression, "+regexp.not(")
expression = strings.TrimSuffix(expression, ")")
}

if !strings.HasPrefix(expression, "^") || !strings.HasSuffix(expression, "$") {
// replace glob-style wildcards with regexp wildcards
// for plain strings, and quote all characters that could
Expand All @@ -62,10 +73,10 @@ func SliceMatchesRegex(input string, expressions []string) (bool, error) {
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) {
// Since the expression is always surrounded by ^ and $ this is an
// exact match for either a plain string (for example ^hello$) or for a
// regexp (for example ^hel*o$).
if expr.MatchString(input) == wantMatch {
return true, nil
}
}
Expand Down
105 changes: 105 additions & 0 deletions lib/utils/replace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package utils

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestSliceMatchesRegexp(t *testing.T) {
t.Parallel()

tests := []struct {
desc string
input string
expressions []string
wantMatch bool
assertErr assert.ErrorAssertionFunc
}{
{
desc: "exact match",
input: "foo",
expressions: []string{`foo`},
wantMatch: true,
assertErr: assert.NoError,
},
{
desc: "exact match, multiple expressions",
input: "foo",
expressions: []string{`bar`, `foo`, `baz`},
wantMatch: true,
assertErr: assert.NoError,
},
{
desc: "no match",
input: "foo",
expressions: []string{`bar`},
wantMatch: false,
assertErr: assert.NoError,
},
{
desc: "wildcard match",
input: "foo",
expressions: []string{`f*`},
wantMatch: true,
assertErr: assert.NoError,
},
{
desc: "wildcard no match",
input: "foo",
expressions: []string{`b*`},
wantMatch: false,
assertErr: assert.NoError,
},
{
desc: "regexp match",
input: "foo",
expressions: []string{`^f.*$`},
wantMatch: true,
assertErr: assert.NoError,
},
{
desc: "regexp no match",
input: "foo",
expressions: []string{`^bar$`},
wantMatch: false,
assertErr: assert.NoError,
},
{
desc: "invalid regexp",
input: "foo",
expressions: []string{`^?+$`},
wantMatch: false,
assertErr: assert.Error,
},
{
desc: "negated regexp match",
input: "foo",
expressions: []string{`+regexp.not(bar)`},
wantMatch: true,
assertErr: assert.NoError,
},
{
desc: "negated regexp no match",
input: "bar",
expressions: []string{`+regexp.not(bar)`},
wantMatch: false,
assertErr: assert.NoError,
},
{
desc: "incomplete negated regexp no match",
input: "foo",
expressions: []string{`+regexp.not(bar`},
wantMatch: false,
assertErr: assert.NoError,
},
}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
match, err := SliceMatchesRegex(tt.input, tt.expressions)
tt.assertErr(t, err)
assert.Equal(t, match, tt.wantMatch)
})
}
}

0 comments on commit c52a4ea

Please sign in to comment.