Skip to content

Commit

Permalink
Create Bot Instances during initial bot join (#43577)
Browse files Browse the repository at this point in the history
* Create Bot Instances during initial bot join

This creates new instances for bots when they initially join the
cluster, and persists instance IDs in new certificate fields on join
and during renewal.

Note that this does not yet handle instance reuse for non-token join
methods.

Additionally, bot instance creation is locked behind a
`BOT_INSTANCE_EXPERIMENT` flag; it must be set to `1` to enable
creation.

* Proto cleanup, and update bot auth records on cert renewal

This makes various (admittedly breaking) protobuf changes, including
removing the TTL field (calculating resource expiry based on cert
requests), removing public key fingerprints, and changing the data
type of the generation counter to match the preexisting internal
datatype. These changes _should_ be safe as no consumers of the proto
API currently exist.

Additionally, this also updates bot authentications on renewal.

* Fix proto lints

* Fix misleading doc comment in the bot instance experiment

* Create bot instances for old bots on join; other fixes

This now creates bot instances for bots whose certs are missing the
BotInstanceID field. Additionally, it fixes two related bugs:
expiration dates are extended on renewal, the generated UUID
is properly appended to certs on initial join, and instances are
only created or updated when the experiment is enabled.

* Add a minimal test for bot instance creation on initial join

* Validate bot instance state in generation counter checks

* Remove outdated TODO comment and fix test lints

* Add an expiration change check to the generation test
  • Loading branch information
timothyb89 committed Aug 8, 2024
1 parent a319f62 commit 2e614ed
Show file tree
Hide file tree
Showing 17 changed files with 566 additions and 167 deletions.
198 changes: 87 additions & 111 deletions api/gen/proto/go/teleport/machineid/v1/bot_instance.pb.go

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions api/proto/teleport/machineid/v1/bot_instance.proto
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@ message BotInstanceSpec {
string bot_name = 1;
// The unique identifier for this instance.
string instance_id = 2;
// The desired expiration offset for this bot instance. The expiration will be
// calculated from the current time at creation or authentication plus this
// value. A nil `ttl` will not expire.
google.protobuf.Duration ttl = 3;

reserved 3;
reserved "ttl";
}

// BotInstanceStatusHeartbeat contains information self-reported by an instance
Expand Down Expand Up @@ -104,8 +103,9 @@ message BotInstanceStatusAuthentication {
int32 generation = 5;
// The public key of the Bot instance.
bytes public_key = 6;
// The fingerprint of the public key of the Bot instance.
string fingerprint = 7;

reserved 7;
reserved "fingerprint";
}

// BotInstanceStatus holds the status of a BotInstance.
Expand Down
4 changes: 4 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ const (
// CertExtensionBotName indicates the name of the Machine ID bot this
// certificate was issued to, if any.
CertExtensionBotName = "bot-name@goteleport.com"
// CertExtensionBotInstanceID indicates the unique identifier of this
// Machine ID bot instance, if any. This identifier is persisted through
// certificate renewals.
CertExtensionBotInstanceID = "bot-instance-id@goteleport.com"

// CertCriticalOptionSourceAddress is a critical option that defines IP addresses (in CIDR notation)
// from which this certificate is accepted for authentication.
Expand Down
5 changes: 5 additions & 0 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,9 @@ type certRequest struct {
deviceExtensions DeviceExtensions
// botName is the name of the bot requesting this cert, if any
botName string
// botInstanceID is the unique identifier of the bot instance associated
// with this cert, if any
botInstanceID string
}

// check verifies the cert request is valid.
Expand Down Expand Up @@ -2790,6 +2793,7 @@ func generateCert(ctx context.Context, a *Server, req certRequest, caType types.
Renewable: req.renewable,
Generation: req.generation,
BotName: req.botName,
BotInstanceID: req.botInstanceID,
CertificateExtensions: req.checker.CertificateExtensions(),
AllowedResourceIDs: requestedResourcesStr,
ConnectionDiagnosticID: req.connectionDiagnosticID,
Expand Down Expand Up @@ -2885,6 +2889,7 @@ func generateCert(ctx context.Context, a *Server, req certRequest, caType types.
Renewable: req.renewable,
Generation: req.generation,
BotName: req.botName,
BotInstanceID: req.botInstanceID,
AllowedResourceIDs: req.checker.GetAllowedResourceIDs(),
PrivateKeyPolicy: attestedKeyPolicy,
ConnectionDiagnosticID: req.connectionDiagnosticID,
Expand Down
12 changes: 12 additions & 0 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -3153,6 +3153,12 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
connectionDiagnosticID: req.ConnectionDiagnosticID,
attestationStatement: keys.AttestationStatementFromProto(req.AttestationStatement),
botName: getBotName(user),

// 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,
}
if user.GetName() != a.context.User.GetName() {
certReq.impersonator = a.context.User.GetName()
Expand Down Expand Up @@ -3205,6 +3211,12 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
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.updateBotInstance(ctx, &certReq); err != nil {
return nil, trace.Wrap(err)
}
}

certs, err := a.authServer.generateUserCert(ctx, certReq)
Expand Down
147 changes: 146 additions & 1 deletion lib/auth/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,17 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/gravitational/teleport/api/client/proto"
headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
machineidv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1"
"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/auth/machineid/machineidv1"
experiment "github.com/gravitational/teleport/lib/auth/machineid/machineidv1/bot_instance_experiment"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
Expand Down Expand Up @@ -168,14 +174,131 @@ func (a *Server) validateGenerationLabel(ctx context.Context, username string, c
return nil
}

// updateBotInstance updates the bot instance associated with the context
// identity, if any.
func (a *ServerWithRoles) updateBotInstance(ctx context.Context, req *certRequest) error {
ident := a.context.Identity.GetIdentity()

if !experiment.Enabled() {
// Only attempt to update bot instances if the experiment is enabled.
return nil
}

if ident.BotName == "" {
// Only applies to bot identities
return nil
}

authRecord := &machineidv1pb.BotInstanceStatusAuthentication{
AuthenticatedAt: timestamppb.New(a.authServer.GetClock().Now()),
PublicKey: req.publicKey,

// TODO: for now, this copy of the certificate generation only is
// informational. Future changes will transition to trusting (and
// verifying) this value in lieu of the old generation label on bot
// users.
Generation: int32(req.generation),

// Note: This auth path can only ever be for token joins; all other join
// types effectively rejoin every renewal. Other fields will be unset
// (metadata, join token name, etc).
JoinMethod: string(types.JoinMethodToken),
}

// An empty bot instance most likely means a bot is rejoining after an
// upgrade, so a new bot instance should be generated.
if ident.BotInstanceID == "" {
log.WithFields(logrus.Fields{
"bot_name": ident.BotName,
}).Info("bot has no instance ID, a new instance will be generated")

instanceID, err := uuid.NewRandom()
if err != nil {
return trace.Wrap(err)
}

expires := a.authServer.GetClock().Now().Add(req.ttl + machineidv1.ExpiryMargin)

bi := newBotInstance(&machineidv1pb.BotInstanceSpec{
BotName: ident.BotName,
InstanceId: instanceID.String(),
}, authRecord, expires)

if _, err := a.authServer.BotInstance.CreateBotInstance(ctx, bi); err != nil {
return trace.Wrap(err)
}

// Add the new ID to the cert request
req.botInstanceID = instanceID.String()

return nil
}

_, err := a.authServer.BotInstance.PatchBotInstance(ctx, ident.BotName, ident.BotInstanceID, func(bi *machineidv1pb.BotInstance) (*machineidv1pb.BotInstance, error) {
if bi.Status == nil {
bi.Status = &machineidv1pb.BotInstanceStatus{}
}

// Update the record's expiration timestamp based on the request TTL
// plus an expiry margin.
bi.Metadata.Expires = timestamppb.New(a.authServer.GetClock().Now().Add(req.ttl + machineidv1.ExpiryMargin))

// If we're at or above the limit, remove enough of the front elements
// to make room for the new one at the end.
if len(bi.Status.LatestAuthentications) >= machineidv1.AuthenticationHistoryLimit {
toRemove := len(bi.Status.LatestAuthentications) - machineidv1.AuthenticationHistoryLimit + 1
bi.Status.LatestAuthentications = bi.Status.LatestAuthentications[toRemove:]
}

// An initial auth record should have been added during initial join,
// but if not, add it now.
if bi.Status.InitialAuthentication == nil {
log.WithFields(logrus.Fields{
"bot_name": ident.BotName,
"bot_instance_id": ident.BotInstanceID,
}).Warn("bot instance is missing its initial authentication record, a new one will be added")
bi.Status.InitialAuthentication = authRecord
}

bi.Status.LatestAuthentications = append(bi.Status.LatestAuthentications, authRecord)

return bi, nil
})

return trace.Wrap(err)
}

// newBotInstance constructs a new bot instance from a spec and initial authentication
func newBotInstance(
spec *machineidv1pb.BotInstanceSpec,
initialAuth *machineidv1pb.BotInstanceStatusAuthentication,
expires time.Time,
) *machineidv1pb.BotInstance {
return &machineidv1pb.BotInstance{
Kind: types.KindBotInstance,
Version: types.V1,
Metadata: &headerv1.Metadata{
Expires: timestamppb.New(expires),
},
Spec: spec,
Status: &machineidv1pb.BotInstanceStatus{
InitialAuthentication: initialAuth,
LatestAuthentications: []*machineidv1pb.BotInstanceStatusAuthentication{initialAuth},
},
}
}

// generateInitialBotCerts is used to generate bot certs and overlaps
// significantly with `generateUserCerts()`. However, it omits a number of
// options (impersonation, access requests, role requests, actual cert renewal,
// and most UserCertsRequest options that don't relate to bots) and does not
// care if the current identity is Nop. This function does not validate the
// current identity at all; the caller is expected to validate that the client
// is allowed to issue the (possibly renewable) certificates.
func (a *Server) generateInitialBotCerts(ctx context.Context, botName, username, loginIP string, pubKey []byte, expires time.Time, renewable bool) (*proto.Certs, error) {
func (a *Server) generateInitialBotCerts(
ctx context.Context, botName, username, loginIP string, pubKey []byte,
expires time.Time, renewable bool, initialAuth *machineidv1pb.BotInstanceStatusAuthentication,
) (*proto.Certs, error) {
var err error

// Extract the user and role set for whom the certificate will be generated.
Expand Down Expand Up @@ -216,6 +339,27 @@ func (a *Server) generateInitialBotCerts(ctx context.Context, botName, username,
var generation uint64
if renewable {
generation = 1
initialAuth.Generation = 1
}

var botInstanceID string
if experiment.Enabled() {
uuid, err := uuid.NewRandom()
if err != nil {
return nil, trace.Wrap(err)
}

bi := newBotInstance(&machineidv1pb.BotInstanceSpec{
BotName: botName,
InstanceId: uuid.String(),
}, initialAuth, expires.Add(machineidv1.ExpiryMargin))

_, err = a.BotInstance.CreateBotInstance(ctx, bi)
if err != nil {
return nil, trace.Wrap(err)
}

botInstanceID = uuid.String()
}

// Generate certificate
Expand All @@ -230,6 +374,7 @@ func (a *Server) generateInitialBotCerts(ctx context.Context, botName, username,
generation: generation,
loginIP: loginIP,
botName: botName,
botInstanceID: botInstanceID,
}

if err := a.validateGenerationLabel(ctx, userState.GetName(), &certReq, 0); err != nil {
Expand Down
Loading

0 comments on commit 2e614ed

Please sign in to comment.