Skip to content

Commit

Permalink
Machine ID: Validate generation counter using bot instances (#44583)
Browse files Browse the repository at this point in the history
* Show bot instance ID in tbot log output

This tweaks the "fetched new bot identity" message to show the bot
name and instance ID as embedded in the bot's certificate.

Example:

```
2024-07-23T15:51:20-06:00 INFO [TBOT:IDEN] Fetched new bot identity identity:tpm-test, id=5a2865d3-d3dc-4eaa-853c-5377a1fe83f6 | valid: after=2024-07-23T21:50:20Z, before=2024-07-23T21:56:19Z, duration=5m59s | kind=tls, renewable=false, disallow-reissue=false, roles=[bot-tpm-test], principals=[-teleport-internal-join], generation=0 tbot/service_bot_identity.go:223
```

* Machine ID: Validate generation counter using bot instances

This change moves the generation counter from bot user labels to a
field in `BotInstanceStatusAuthentication`. This allows for tracking
of individual generations regardless of join method, allows for
multiple `token`-type joins per bot, and opens the door for multi-use
`token`-type tokens in the future.

For now, the new behavior remains behind the `BOT_INSTANCE_EXPERIMENT`
feature flag, and generation counter handling is largely unchanged
when it is unset.

* Update legacy generation counter for downgrade compatibility

This updates the legacy user label generation counter to support some
downgrade compatibility if a user either disables the bot instance
experiment or downgrades to a Teleport version predating it.

Also includes some logger cleanup, and adds a warning when a
generation counter mismatch is detected but not enforced.

* Set initial generation value for legacy mode

`validateGenerationLabel` expects a nonzero generation counter value
for renewable certs to populate the counter, so this provides it
again.

* Add new generation counter tests

This adds various new tests to validate new per-instance generation
counter behavior.

Also includes backcompat fixes to import old generation counters, and
a small fix to ensure downgrades don't leave bots in a bad state.

* Fix imports

* Tweak TODO message
  • Loading branch information
timothyb89 authored Jul 30, 2024
1 parent 8e019fc commit 769c435
Show file tree
Hide file tree
Showing 7 changed files with 2,221 additions and 1,784 deletions.
5 changes: 5 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5285,6 +5285,11 @@ message RegisterUsingTokenRequest {
// service and is ignored otherwise; bots should otherwise rejoin with their
// existing client certificate to prove their instance identity.
string BotInstanceID = 13 [(gogoproto.jsontag) = "bot_instance_id"];
// BotGeneration is a trusted generation counter value for Machine ID bots,
// provided to Auth by the Join Service when bots rejoin via a streamed/gRPC
// join method. Rejoining bots supply this value via a client certificate
// extension; it is ignored from other sources.
int32 BotGeneration = 14 [(gogoproto.jsontag) = "bot_generation"];
}

// RecoveryCodes holds a user's recovery code information. Recovery codes allows users to regain
Expand Down
3,425 changes: 1,729 additions & 1,696 deletions api/types/types.pb.go

Large diffs are not rendered by default.

63 changes: 45 additions & 18 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import (
"github.com/gravitational/teleport/entitlements"
"github.com/gravitational/teleport/lib/auth/authclient"
"github.com/gravitational/teleport/lib/auth/clusterconfig/clusterconfigv1"
experiment "github.com/gravitational/teleport/lib/auth/machineid/machineidv1/bot_instance_experiment"
"github.com/gravitational/teleport/lib/auth/okta"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/backend"
Expand Down Expand Up @@ -584,21 +585,33 @@ func (a *ServerWithRoles) RegisterUsingToken(ctx context.Context, req *types.Reg
}
}

// Similarly, do not trust bot instance IDs in the request unless from the
// proxy (via the join service). They will be derived from the client
// certificate otherwise.
if !isProxy && req.BotInstanceID != "" {
log.WithFields(logrus.Fields{
"bot_instance_id": req.BotInstanceID,
}).Warnf("Untrusted client attempted to provide a bot instance ID, this will be ignored")
// Similarly, do not trust bot instance IDs or generation values in the
// request unless from a component with the proxy role (e.g. the join
// service). They will be derived from the client certificate otherwise.
if !isProxy {
if req.BotInstanceID != "" {
log.WithFields(logrus.Fields{
"bot_instance_id": req.BotInstanceID,
}).Warnf("Untrusted client attempted to provide a bot instance ID, this will be ignored")

req.BotInstanceID = ""
}

req.BotInstanceID = ""
if req.BotGeneration > 0 {
log.WithFields(logrus.Fields{
"bot_generation": req.BotGeneration,
}).Warnf("Untrusted client attempted to provide a bot generation, this will be ignored")

req.BotGeneration = 0
}
}

// If the identity has a BotInstanceID included, copy it onto the request -
// but only if one wasn't already passed along via the proxy.
// If the identity has a BotInstanceID or BotGeneration included, copy it
// onto the request - but only if one wasn't already passed along via the
// proxy.
ident := a.context.Identity.GetIdentity()
req.BotInstanceID = cmp.Or(req.BotInstanceID, ident.BotInstanceID)
req.BotGeneration = cmp.Or(req.BotGeneration, int32(ident.Generation))

// tokens have authz mechanism on their own, no need to check
return a.authServer.RegisterUsingToken(ctx, req)
Expand Down Expand Up @@ -3309,13 +3322,21 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
},
connectionDiagnosticID: req.ConnectionDiagnosticID,
botName: getBotName(user),
}

if experiment.Enabled() {
// Always pass through a bot instance ID if available. Legacy bots
// joining without an instance ID may have one generated when
// `updateBotInstance()` is called below, and this (empty) value will be
// overridden.
botInstanceID: a.context.Identity.GetIdentity().BotInstanceID,
// Note that this copy needs to be tied to the experiment for downgrade
// compatibility. If a cluster is downgraded the ID needs to be dropped
// from certs so a new one can be generated, otherwise the generation
// counter in the stored identity will mismatch when auth is upgraded
// again.
certReq.botInstanceID = a.context.Identity.GetIdentity().BotInstanceID
}

if user.GetName() != a.context.User.GetName() {
certReq.impersonator = a.context.User.GetName()
} else if isRoleImpersonation(req) {
Expand Down Expand Up @@ -3364,14 +3385,20 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
// If the cert is renewable, process any certificate generation counter.
if certReq.renewable {
currentIdentityGeneration := a.context.Identity.GetIdentity().Generation
if err := a.authServer.validateGenerationLabel(ctx, user.GetName(), &certReq, currentIdentityGeneration); err != nil {
return nil, trace.Wrap(err)
}

// Update the bot instance based on this authentication. This may create
// a new bot instance record if the identity is missing an instance ID.
if err := a.authServer.updateBotInstance(ctx, &certReq, certReq.botName, certReq.botInstanceID, nil); err != nil {
return nil, trace.Wrap(err)
if experiment.Enabled() {
// Update the bot instance based on this authentication. This may create
// a new bot instance record if the identity is missing an instance ID.
if err := a.authServer.updateBotInstance(
ctx, &certReq, user.GetName(), certReq.botName,
certReq.botInstanceID, nil, int32(currentIdentityGeneration),
); err != nil {
return nil, trace.Wrap(err)
}
} else {
if err := a.authServer.validateGenerationLabel(ctx, user.GetName(), &certReq, currentIdentityGeneration); err != nil {
return nil, trace.Wrap(err)
}
}
}

Expand Down
Loading

0 comments on commit 769c435

Please sign in to comment.