Commit Graph
13 Commits
Author SHA1 Message Date
Ben Arent 09928a7f2b Cherry pick Gravitational -> GoTeleport (#4932) 2020-11-25 11:18:55 -08:00
Andrew Lytvynov 43178f34d8 Add a depth limit to RBAC expression parser (#4848)
Our current parsing code runtime grows exponentially with nested
selectors (e.g. '{{a.b.c.d.e.f}}'), mostly due to memory churn from
slice allocations. With 100,000 levels of selectors, parsing takes ~80s
on my machine.
If an attacker can submit these expressions for parsing, they can DoS
the auth server with relatively small payloads (<1MB).

All real-world expressions are <10 AST nodes deep. Add a sanity check of
1000 levels to protect against malicious inputs.

We can optimize the code later on, but it's not very useful for real
world performance.
2020-11-17 18:53:38 +00:00
Andrew Lytvynov 566b7cc457 RFD 1: user testify/require instead of testify/assert
`require` is a sister package to `assert` that terminates the test on
failure. `assert` records the failure but lets the test proceed, which
is un-intuitive.

Also update all existing tests to match.
2020-10-16 00:15:25 +00:00
Andrew Lytvynov 01516c5744 Partial revert of negative regexps in RBAC labels
This change was not backwards compatible - variable interpolation should
work in node_labels.

This commit partially reverts
https://github.com/gravitational/teleport/pull/4253 and
https://github.com/gravitational/teleport/pull/4430
2020-10-15 21:46:06 +00:00
Andrew Lytvynov 63da43245e Validate node_labels syntax at role creation time
Also improve error output and user validation.
Based on missed feedback in https://github.com/gravitational/teleport/pull/4253#pullrequestreview-499061448
2020-10-05 18:00:55 +00:00
Andrew Lytvynov 75d7fbb508 Migrate services.MatchLabels to parse.Matcher
This should be backwards-compatible plus add the {{regexp.match(...)}}
and {{regexp.not_match(...)}} functions.
2020-09-29 21:25:50 +00:00
Andrew Lytvynov 1330415d33 utils/parse: implement Matcher
Matchers use a similar syntax to Expressions, but behave differently:
- Expressions get evaluated - they interpolate some values and return a
  final string.
- Matchers check whether some string matches a value

Matchers implement the same logic as utils.SliceMatchesRegex and add 2
new functions:
- {{regexp.match("foo")}} - match input against a raw regex
- {{regexp.not_match("foo")}} - same as match, but inverts the result
2020-09-29 21:25:50 +00:00
Andrew Lytvynov bb69574e02 utils/parse: support string literal expressions
No need to handle literal expressions (e.g. without "{{foo.bar}}"
substitutions) at the higher level. Something like "foo" is a valid
expression which always returns "foo" regardless of traits.
2020-09-29 21:25:50 +00:00
Andrew Lytvynov e9b7a25d6a utils/parse: rewrite tests in the new RFD style 2020-09-29 21:25:50 +00:00
Andrew Lytvynov 617afc7e6f Fix remaining gosimple findings
List of fixed items:

```
integration/helpers.go:1279:2               gosimple  S1000: should use for range instead of for { select {} }
integration/integration_test.go:144:5       gosimple  S1009: should omit nil check; len() for nil slices is defined as zero
integration/integration_test.go:173:5       gosimple  S1009: should omit nil check; len() for nil slices is defined as zero
integration/integration_test.go:296:28      gosimple  S1019: should use make(chan error) instead
integration/integration_test.go:570:41      gosimple  S1019: should use make(chan interface{}) instead
integration/integration_test.go:685:40      gosimple  S1019: should use make(chan interface{}) instead
integration/integration_test.go:759:33      gosimple  S1019: should use make(chan string) instead
lib/auth/init_test.go:62:2                  gosimple  S1021: should merge variable declaration with assignment on next line
lib/auth/tls_test.go:1658:22                gosimple  S1024: should use time.Until instead of t.Sub(time.Now())
lib/backend/dynamo/dynamodbbk.go:420:5      gosimple  S1004: should use !bytes.Equal(expected.Key, replaceWith.Key) instead
lib/backend/dynamo/dynamodbbk.go:656:12     gosimple  S1039: unnecessary use of fmt.Sprintf
lib/backend/etcdbk/etcd.go:458:5            gosimple  S1004: should use !bytes.Equal(expected.Key, replaceWith.Key) instead
lib/backend/firestore/firestorebk.go:407:5  gosimple  S1004: should use !bytes.Equal(expected.Key, replaceWith.Key) instead
lib/backend/lite/lite.go:317:5              gosimple  S1004: should use !bytes.Equal(expected.Key, replaceWith.Key) instead
lib/backend/lite/lite.go:336:6              gosimple  S1004: should use !bytes.Equal(value, expected.Value) instead
lib/backend/memory/memory.go:365:5          gosimple  S1004: should use !bytes.Equal(expected.Key, replaceWith.Key) instead
lib/backend/memory/memory.go:376:5          gosimple  S1004: should use !bytes.Equal(existingItem.Value, expected.Value) instead
lib/backend/test/suite.go:327:10            gosimple  S1024: should use time.Until instead of t.Sub(time.Now())
lib/client/api.go:1410:9                    gosimple  S1003: should use strings.ContainsRune(name, ':') instead
lib/client/api.go:2355:32                   gosimple  S1019: should use make([]ForwardedPort, len(spec)) instead
lib/client/keyagent_test.go:85:2            gosimple  S1021: should merge variable declaration with assignment on next line
lib/client/player.go:54:33                  gosimple  S1019: should use make(chan int) instead
lib/config/configuration.go:1024:52         gosimple  S1019: should use make(services.CommandLabels) instead
lib/config/configuration.go:1025:44         gosimple  S1019: should use make(map[string]string) instead
lib/config/configuration.go:930:21          gosimple  S1003: should use strings.Contains(clf.Roles, defaults.RoleNode) instead
lib/config/configuration.go:931:22          gosimple  S1003: should use strings.Contains(clf.Roles, defaults.RoleAuthService) instead
lib/config/configuration.go:932:23          gosimple  S1003: should use strings.Contains(clf.Roles, defaults.RoleProxy) instead
lib/service/supervisor.go:387:2             gosimple  S1001: should use copy() instead of a loop
lib/tlsca/parsegen.go:140:9                 gosimple  S1034: assigning the result of this type assertion to a variable (switch generalKey := generalKey.(type)) could eliminate type assertions in switch cases
lib/utils/certs.go:140:9                    gosimple  S1034: assigning the result of this type assertion to a variable (switch generalKey := generalKey.(type)) could eliminate type assertions in switch cases
lib/utils/certs.go:167:40                   gosimple  S1010: should omit second index in slice, s[a:len(s)] is identical to s[a:]
lib/utils/certs.go:204:5                    gosimple  S1004: should use !bytes.Equal(certificateChain[0].SubjectKeyId, certificateChain[0].AuthorityKeyId) instead
lib/utils/parse/parse.go:116:45             gosimple  S1003: should use strings.Contains(variable, "}}") instead
lib/utils/parse/parse.go:116:6              gosimple  S1003: should use strings.Contains(variable, "{{") instead
lib/utils/socks/socks.go:192:10             gosimple  S1025: should use String() instead of fmt.Sprintf
lib/utils/socks/socks.go:199:10             gosimple  S1025: should use String() instead of fmt.Sprintf
lib/web/apiserver.go:1054:18                gosimple  S1024: should use time.Until instead of t.Sub(time.Now())
lib/web/apiserver.go:1954:9                 gosimple  S1039: unnecessary use of fmt.Sprintf
tool/tsh/tsh.go:1193:14                     gosimple  S1024: should use time.Until instead of t.Sub(time.Now())
```
2020-05-27 19:36:38 +00:00
Alexander Klizhentas 73ecb48232 Adds support for kubernetes_users, extend interpolation (#3404) (#3418)
This commit fixes #3369, refs #3374

It adds support for kuberenetes_users section in roles,
allowing Teleport proxy to impersonate user identities.

It also extends variable interpolation syntax by adding
suffix and prefix to variables and function `email.local`:

Example:

```yaml
kind: role
version: v3
metadata:
  name: admin
spec:
  allow:
    # extract email local part from the email claim
    logins: ['{{email.local(external.email)}}']

    # impersonate a kubernetes user with IAM prefix
    kubernetes_users: ['IAM#{{external.email}}']

  # the deny section uses the identical format as the 'allow' section.
  # the deny rules always override allow rules.
  deny: {}
```

Some notes on email.local behavior:

* This is the only function supported in the template variables for now
* In case if the email.local will encounter invalid email address,
it will interpolate to empty value, will be removed from resulting
output.

Changes in impersonation behavior:

* By default, if no kubernetes_users is set, which is a majority of cases,
  user will impersonate themselves, which is the backwards-compatible behavior.

* As long as at least one `kubernetes_users` is set, the forwarder will start
  limiting the list of users allowed by the client to impersonate.

* If the users' role set does not include actual user name, it will be rejected,
  otherwise there will be no way to exclude the user from the list).

* If the `kuberentes_users` role set includes only one user
  (quite frequently that's the real intent), teleport will default to it,
  otherwise it will refuse to select.

  This will enable the use case when `kubernetes_users` has just one field to
  link the user identity with the IAM role, for example `IAM#{{external.email}}`

* Previous versions of the forwarding proxy were denying all external
impersonation headers, this commit allows 'Impesrsonate-User' and
'Impersonate-Group' header values that are allowed by role set.

* Previous versions of the forwarding proxy ignored 'Deny' section of the roles
when applied to impersonation, this commit fixes that - roles with deny
kubernetes_users and kubernetes_groups section will not allow
impersonation of those users and groups.
2020-03-07 16:32:37 -08:00
Russell Jones df9962d572 Test cleanup.
Initialize in main utils_test.go file and name test functions match
package names.
2019-09-03 13:44:20 -07:00
Russell Jones b324616208 Added support for role variables. 2017-07-24 22:06:07 +00:00