* Benchmark authz.Context.LockTargets
* Make LockTarget comparable
* Add a test against the addition of non-simple fields to LockTarget
* Clarify comment about gogoproto options
* Fix TestSSHAccessLockTargets tests
* Clarify the comparable assertion
* Add hostAction helper to authorize actions for resources owned by a specific host service.
* Remove broad write privileges from agent system roles, instead only giving permission to upsert/delete agent-owned resources.
* Add `mobile_device` kind for `mobile_device.create_enroll_token` permission
* Update preset-roles.json
* Add `mobile_device.create_enroll_token` to user ACL
* Make boombuler/barcode a direct dep
It's used in teleport.e to generate the QR code that's going to be
consumed by the mobile app.
* Enable defaultAllowRules to return roles which need `buildType`
* Backfill device-admin role
* Add `mobile_device.create_enroll_token` to admin role
* feat(certs): thread beam_id through identity certificates
Carries the beam identifier from a delegation session's BeamIDLabel into
issued SSH/TLS certificates and back out via the cert extensions/ASN.1 OID,
exposes it on tlsca/sshca Identity and the decision PDP identities, propagates
it through scoped cert reissue (auth_with_roles), and surfaces it on event
UserMetadata for attribution.
* handle missing event cases
* prevent overriding beams_id cert extension with roles cert extensions
* handle code review comment
* Convert existing non-gogo codegen to the Hybrid API
Contributes to https://github.com/gravitational/teleport/issues/66776.
All existing protos explicitly set to API_OPEN have been change to
API_HBYRID. The new codegen was performed via make grpc. There are no
other functional changes to the code to start consuming the Hybrid API
those will come later. The intent is to get all Hybrid codegen in and
backported to ease the transition.
* Initial migration to the Opaque API
Contributes to https://github.com/gravitational/teleport/issues/66776.
All of the changes here are mechanical conversions generated from
`open2opaque rewrite -levels=green ./...`. There will be a follow up
to this in teleport.e which does the same. Once all changes have been
merged the process will be repeated with -levels=yellow followed by
-levels=red.
See https://protobuf.dev/reference/go/opaque-migration/ for more
details.
Replace the functions in the scopes package to infer if scoped features
are enabled with a concrete struct that is injected to consumers. This
permits tests to toggle scopes features without having to specify environment
variables.
* Add disconnect expired cert and lock mode for scoped role
Update IAC docs
* respond to code review
Add a scoped authorizer in the forwarder_test
fix up proto comments
Update forwarder.go
* fix forwarder setting nil disconnectExpiredCert
* Update validate lock to accept empty string - will be treated as best_effort down the line
add comment
Teleport Auth server allows clients that do not present a certificate to access parts of its API. This deliberate bypass exists to support joining methods or password logins where a user or server does not have a certificate yet but has a secret that can be exchanged for a certificate.
In order to keep the system compatible and reduce the permutations required, Teleport assigned these unauthenticated connections a role called `Nop`. This role has no permissions and the only reason why it exists is so we can still build a context and checker.
This role was historically embedded in a structure called `BuiltinRole`. This struct implements the `Identity` interface and by its documentation is used by teleport services that belong to this particular cluster.
Some of the Auth server handlers are restricted to server identities and the check consists in something like:
```go
authCtx, err := s.authorizer.Authorize(ctx)
if err != nil {
return nil, trace.Wrap(err)
}
if b, ok := authCtx.Identity.(authz.BuiltinRole); !ok || !b.IsServer() {
return nil, trace.AccessDenied("only server identities can verify validated MFA challenges")
}
```
As seen above, part of the verification consists in type asserting the `authCtx.Identity` to confirm it belongs to a local server and then we check if the role belongs to an actual server.
In our codebase, we have a function called `IsLocalOrRemoteService` with the following body:
```go
// IsLocalOrRemoteService checks if the identity is either a local or remote service.
func IsLocalOrRemoteService(authContext Context) bool {
switch authContext.UnmappedIdentity.(type) {
case BuiltinRole, RemoteBuiltinRole:
return true
default:
return false
}
}
```
Given that unauthenticated users were wrapped in `BuiltinRole`, this function would return true for them. This caused the encrypted recordings service to authorize unauthenticated clients to access the upload handlers. This was the only access check applied, so the bug allowed any client to upload and replace files in the backend storage.
This change fixes that by enforcing proper RBAC checks in the encrypted recordings `CreateUpload`, `UploadPart` and `CompleteUpload` handlers. It also forbids `RemoteBuiltinRole` (remote/leaf cluster identity) from uploading files to root clusters.
This change also moves RoleNop out of the `SystemRole` type and into a new distinct type, `UnauthenticatedRole`, so the compiler prevents it from being used anywhere a real authenticated system role is expected. As a defence-in-depth measure, the middleware now rejects any TLS certificate that claims the Nop role in its Groups or SystemRoles fields.
This PR fixes a separate issue on top of the existing one where certificate rotation of encrypted sessions was available to unauthenticated users.
The protection introduced was `authCtx.AuthorizeAdminAction()`, which verifies if someone has completed the MFA challenge (if required). That was the intended idea, but given how Teleport handles unauthenticated users, they were marked with AdminActionAuthNotRequired, which bypassed MFA checks. Because of that, any client - authenticated or unauthenticated - with the ability to complete the MFA challenge (or if MFA was not required) could rotate the encryption keys for session recordings.
This doesn't significantly affect Teleport since the private keys are never exposed externally and previous keys always remain in the backend object, allowing old recordings to still be decrypted. The main concern is if the keys were rotated so many times that the backend item size exceeds the 400KB limit on DynamoDB. While this would cause the rotation to fail beforehand, it would also prevent any further rotations from succeeding.
This PR introduces proper RBAC for modifying types.KindRecordingEncryption and ensures that unauthenticated users are always assigned AdminActionAuthNotRequired.
Fixes https://github.com/gravitational/teleport-private/issues/2430
Fixes https://github.com/gravitational/teleport-private/issues/2429
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Teleport RBAC policies give built-in role certificates (proxy,
discovery, okta, node, kube...) access to the session recordings
API and audit log.
For the session recordings API, the policy enforcement completely
skips the RBAC logic and if the certificate was from a built-in role,
the client verification logic would skip RBAC entirely.
```go
func (a *ServerWithRoles) StreamSessionEvents(ctx context.Context, sessionID session.ID, startIndex int64) (chan apievents.AuditEvent, chan error) {
err := a.localServerAction()
isTeleportServer := err == nil
// StreamSessionEvents can be called internally, and when that
// happens we don't want to emit an event or check for permissions.
if isTeleportServer {
return a.alog.StreamSessionEvents(ctx, sessionID, startIndex)
}
```
From the built-in roles, only node had read access to
`types.KindSession`, but given the check above all of them had access,
including cases like `okta` where no session recording exists.
This means a user with access to a token or an agent certificate can
read, play and view every single session recording.
Historically, this bypass existed for one particular reason: when an
upload is abandoned without proper termination - this happens when the
agent is restarted while interactive sessions are alive, the agent
places the session in a temporary folder. This folder is constantly
monitored by a routine called `UploadCompleter`. This routine runs on
every teleport process and is responsible for deciding when a session
was abandoned - no active session tracker and the last written data was
24h ago and mark it completed, i.e. move it to another directory, so that
the file uploader could move it to auth server for long-term storage.
Since the session was not correctly terminated it could be the case it
misses the `session.end` or respective end event. This is not
particularly important for the recording itself since it's playable as is,
but it's critical for the session recordings list.
When a user goes to Session Recordings page or does `tctl recordings ls`,
Teleport issues a read request from the audit log to search for
`session.end`, `windows.desktop.session.end`, `db.session.end` and
`app.session.end`. If the audit log doesn't contain the events but the
agent successfully uploaded the recording, the recording won't be
included in the list and will be inaccessible from `tctl` or `tsh`.
To overcome this problem,
https://github.com/gravitational/teleport/pull/14521 introduced logic to
reconstruct the session end event from the recording itself. The logic
assumes (partially incorrect) that if the session recording holds the
session end event, the same event was already present in the audit log.
After the `UploadCompleter` successfully marked the upload as completed,
it scheduled a goroutine to stream the session and see if the session end
event exists. The goroutine had to run 2m after because it was considered
a good time for the file uploader to send the session to the auth server
and auth made it available in the long term storage. To stream it, the
agent contacted Auth Server through the same API users use to play sessions.
Because of an improper filtering and logic, any agent had access to any session.
Even without knowing the session id, given that the agent also had access to read
the audit log, it was able to search for the session end events and later access
them using the `StreamSessionEvents` API.
The main reason why it was implemented like this was because from the moment the
`UploadCompleter` marked the session as completed, auth server had no way to distinguish
between a complete session vs an incomplete session. So the logic was placed into the last
piece that still retained that information.
This PR fixes both problems. It removes RW access to audit logs,
lowering it to WO - agents can write it but can't read it, and removed
any access to StreamSessionEvents.
In order to remove the `StreamSessionEvents` logic being accessible by
the agents, this PR moves the same logic to auth server. There are 3
places where this logic must exist:
- Auth upload completer: If the cluster is operating with any sync mode
instead of async, Auth is the receiving point the the streams. If
something fails, the recording will live in auth server and not the
agent.
- Upload API in Auth: When clusters operate in async mode, agents'
fileuploader uses the auth grpc API to upload the events. Since auth
receives event by event, we can easily analyse if the session already
has a session end event and fill it if necessary
- Encrypted recordings upload: If the cluster operates with encrypted
recordings enabled, agents upload the data to the encrypted recordings
gRPC service. Before this fix, if the session was incomplete we didn't
even try to build the session end event and the session was hidden.
This PR exposes the completer logic in auth server and each one of the 3
cases mentioned call that auth's server function for consistency and
predictability.
Fixes https://github.com/gravitational/teleport-private/issues/2363
This PR fixes https://github.com/gravitational/teleport/issues/60861
accidentally.
Adds a new variable `user.metadata.name` that can be used in role templates:
```yaml
allow:
node_labels:
owner: '{{user.metadata.name}}'
```
Or in expressions:
```yaml
allow:
node_labels_expression: |
labels["owner"] == user.metadata.name
```
So that you can create roles that allow access to user-owned resources such as
"Connect My Computer" nodes, or the upcoming Beams feature.
* Add `DelegationSessionService` protobuf definition
* Implement `GenerateCerts` RPC
* Prevent restricted delegation sessions from inheriting administrative privileges
Previously, the `resources` list on a delegation session would restrict regular
access to resources that weren't explicitly included, but not administrative
actions.
Now, we explicitly deny all administrative (rule-authorized) actions for
delegation sessions unless you include a wildcard in `resources`.
* Only permit list/read/readnosecrets for allow-listed resource kinds
* Fix reused extension OID
* Add a small amount of tolerance for clock skew
* Make the "expires must be in the future" message clearer
* Fix linter errors
* Access to a windows desktop implies access to the desktop service
* Add `DelegationSessionID` to the decision package's `SSHIdentity` and `TLSIdentity`
* Wrap `ExtractConditionForIdentifier` for delegation sessions too
* Prevent calling `GenerateCerts` inside a delegation session
* No need for indirection with GenerateUserCerts method now
* Fix import ordering
* Stop delegation service from returning CA certificates
Per @strideynet's request, to avoid replicating existing bad patterns around
conflating the user and host CAs.
* Error messages should start with lowercase letters
* Add `RouteToKubernetes` wrapping message
* Replace expires parameter with TTL
* Remove unnesasary error check
* Fix `AgentScope` extension type assertion
This is a pretty obscure edge case where an admin action attempt can
cause the admin server to crash.
The following have to be true:
- The request is coming from an impersonator
- The impersonating user no longer exists, but has a valid cert
- They are impersonating a user who does still exist (not a role)
- The impersonator username does not have a dot ('.') in it
- Admin action MFA is required
If all of those are true, then the request will cause a panic.
* feat: Extend `AWSRole` struct w/ `RequiresRequest`
- Differentiate between already-granted vs. requestable AWSRoles when
including requestable resources in `clusterUnifiedResourcesGet`
req
* feat: Implement Resource Constraints support for Proxy, Auth
* fixup: Fix tests
* test: Add tests for resourceaccessid-related helpers
* fixup: Address code review feedback
* test: Update identity/events tests for new `ResourceAccessID` fields
* fixup: Expand/improve godocs/comments for sentinel ResourceID handling
* fixup: Address code review feedback
* feat: Conv ResourceID->ResourceAccessID at cert decoding
- Convert ResourceIDs present on certs to ResourceAccessIDs at decoding
time, rather than at each callsite. Update tests/usages of tlsca/sshca
Identities to reflect this.
* Add ValidatedMFAChallenge backend storage service and wire to MFA gRPC service to store on create/validate MFA challenge.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Rename to just local.MFAService. More polish.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Refactor for begin SSO to use params struct.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Ensure target cluster is valid. Add more test coverage. Improve consistency.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Make consistent.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Fixed wrong refs.
* Add backend MFA service for managing MFA resources.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Extract backend MFA service.
* Address comments from peer review.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Use cmp.Diff for comparing want and got.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Apply suggestions from code review
* Apply suggestions from code review
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
* Remove non-idiomatic ctx nil check.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Remove alias in comment.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Remove fake clock.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Remove testing t param. Fix missing whitespace before package.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Simplify and use proto.Clone.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Use assert to keep tests going after failure.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Create sso.go. Remove redundant user comment. Use authz.MFAAuthData for consistency.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Verify challenge is stored after validate.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Make challenge construction more readable. Simplify tests and use helpers.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Split emit audit event into helper to simplify.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Use GetRemoteCluster to simplify.
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Clean up and polish
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
* Apply suggestions from code review
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
---------
Signed-off-by: Chris Thach <chris.thach@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Implements core health check logic for Kubernetes.
- Added functions `CheckHealth` and `GetProtocol` to `kubeDetails`
- Added a `HealthCheckManager` field to the Kubernetes `TLSServerConfig`
- Kube agent and proxy now instantiate a `HealthCheckManager`
- Added a `HealthCheckConfigReader` to interfaces `ReadKubernetesAccessPoint` and `ReadProxyAccessPoint`
- Added `HealthCheckConfig` read-only permissions for proxy and kube
- Added `HealthCheckConfig` watching for proxy and kube
- Added functions to `KubeServer` interface and `KubernetesServerV3` struct:
- `GetTargetHealth() TargetHealth`
- `SetTargetHealth(h TargetHealth)`
- `GetTargetHealthStatus() TargetHealthStatus`
- `SetTargetHealthStatus(status TargetHealthStatus)`
- Health check supports dynamic discovery of Kubernetes clusters. Calls to `startHealthCheck()` and `stopHealthCheck()` were rearranged.
- Added functions `startHeartbeatAndHealthCheck()` and `stopHeartbeatAndHealthCheck()`
- Moved call to `HealthCheckManager.Start()` outside of the Kubernetes proxy server providing a future option to reuse `HealthCheckManager` in multiple proxy services
- Removed `Status` initialization in KubernetesServerV3 `CheckAndSetDefaults()`
- Added `kubernetesLabelMatchers` to `healthCheckConfig` struct. Default presets are omitted until the entire kube health check is complete.
- Added Kubernetes matcher checking to `ValidateHealthCheckConfig()`
- Changed `ValidateHealthCheckConfig()` to allow zero total DB matchers and Kubernetes matchers.
- Changed KubernetesServerV3 `GetTargetHealthStatus()` to return `TargetHealthStatusUnknown` instead of an empty string
- Changed health check worker `getTargetHealthTimeout` default timeout to `4s` from `10s`. This potentially reduces the response time of the initial heartbeat polling call to `GetServerInfo()`.
Part of #58413
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
* chore: add user's origin cluster name to audit log
While investigating #12324, I realized that our audit logs do not
include any metadata associated with the user's provenence. This means
in root-leaf clusters, it's impossible to figure out if a user was a
remote user or a local (leaf) user by looking into the audit logs.
This PR adds a new field `user_cluster_name` which holds the Teleport
cluster name where the user is authenticated against.
In leaf clusters, this will be the root cluster name. For root clusters
or single clusters, the field will hold the local cluster name.
* handle remote users via x509 certs
* handle code review comment
* fix TestAuthenticateSSHUser test
* Device Trust: add `required-human` mode
* Add `device_trust.mode=required-human` to the test plan
* Add `IsBot` methods to TLS and SSH identities
* Fix `required-human` mode on roles
* Rename mode to `required-for-humans` for clarity
* Undo premature changes to device trust guides
* Default `deviceVerificationPassed` false to avoid accidents when adding new modes
* Handle `BotName` and `BotInstanceID` in `lib/auth/authtest`
* Fix stray reference in test
* Fix reference to `config.DestinationMemory`
* Handle empty device trust mode
* Move auth test helpers to authtest package
In an effort to remove testing symbols from the teleport binary
the auth helpers in lib/auth/helpers.go are being moved to a new
lib/auth/authtest package. This package is to be limited to only
being imported within _test.go files to prevent the helpers from
slipping back into production code.
* Convert lib/auth tests to use lib/auth/authtest
All tests within lib/auth that made use of the test helpers in
lib/auth/helpers.go now use the equivalent functionality provided
by lib/auth/authtest. To prevent import cycles the tests in lib/auth
have been moved to the auth_test package. This shift required some
extra work as a fair number of the existing tests relied on unexported
symbols. To work around this lib/auth/export_test.go has been added
for the sole purpose of exporting private types, methods, or extending
existing types with new exported received methods. This was chosen
over rewriting existing tests for time and simplicity. All future
tests should not rely on this hack and should instead stick to
testing the publicly exported items from the auth package.
* fix: allow specifying accepted usage in cert requests
* fix: restore test assertions in TestGenerateKubernetesUserCert
* fix: set remote cluster variables
* fix: restore test assertions in TestGenerateUserCertWithHardwareKeySupport
* fix: context passing to user cert generation
* fix: github connector API
* fix: pull renewable field from outer identity
* rename authtest.TestDevice authtest.Device
* fix: typos
* add clean up TODOs
* Refactor authtest helpers
* fix: convert additions to bound keypair tests
* fix: add plugin service to authtest helpers
The new package is meant to house a test friendly implementation
of modules.Modules. Due to the pervasive and global nature of
modules, this package also contains a copy of of
modules.SetTestModules. In a perfect world this function wouldn't
be needed and modules would be injected where required, however,
that would take considerable effort. The existing test modules
are temporarily left in place until the enterprise tests can be
updated to use modulestest.
Brian Joerger