Machine ID: Include Bot Instance ID in audit logs (#43786)

* 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

* Add BotInstanceID to audit events

* Fix borked conflict resolution

* Fix further borked conflict resolution

* Add test for cert create/join

---------

Co-authored-by: Tim Buckley <[email protected]>

api/proto/teleport/legacy/types/events/events.proto

@@ -111,6 +111,13 @@ message UserMetadata {
   // UserKind indicates what type of user this is, e.g. a human or Machine ID
   // bot user.
   UserKind UserKind = 10 [(gogoproto.jsontag) = "user_kind,omitempty"];
+
+  // BotName is the name of the Bot if this action is associated with one.
+  string BotName = 11 [(gogoproto.jsontag) = "bot_name,omitempty"];
+
+  // BotInstanceID is the ID of the Bot Instance if this action is associated
+  // with one.
+  string BotInstanceID = 12 [(gogoproto.jsontag) = "bot_instance_id,omitempty"];
 }
 
 // Server is a server metadata
@@ -3846,6 +3853,8 @@ message BotJoin {
     (gogoproto.embed) = true,
     (gogoproto.jsontag) = ""
   ];
+  // BotInstanceID is the ID of the bot instance which has joined or renewed.
+  string BotInstanceID = 9 [(gogoproto.jsontag) = "bot_instance_id,omitempty"];
 }
 
 // InstanceJoin records an instance join event.
@@ -4575,6 +4584,9 @@ message Identity {
   // DeviceExtensions holds the device trust device extensions for the identity,
   // if any.
   DeviceExtensions DeviceExtensions = 28 [(gogoproto.jsontag) = "device_extensions,omitempty"];
+  // BotInstanceID indicates the name of the Machine ID bot instance this
+  // identity was issued to, if any.
+  string BotInstanceID = 29 [(gogoproto.jsontag) = "bot_instance_id,omitempty"];
 }
 
 // RouteToApp contains parameters for application access certificate requests.

lib/auth/bot.go

@@ -342,13 +342,15 @@ func newBotInstance(
 // 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.
+//
+// Returns a second argument of the bot instance ID for inclusion in audit logs.
 func (a *Server) generateInitialBotCerts(
 	ctx context.Context, botName, username, loginIP string,
 	sshPubKey, tlsPubKey []byte,
 	expires time.Time, renewable bool,
 	initialAuth *machineidv1pb.BotInstanceStatusAuthentication,
 	existingInstanceID string,
-) (*proto.Certs, error) {
+) (*proto.Certs, string, error) {
 	var err error
 
 	// Extract the user and role set for whom the certificate will be generated.
@@ -360,13 +362,13 @@ func (a *Server) generateInitialBotCerts(
 	userState, err := a.GetUserOrLoginState(ctx, username)
 	if err != nil {
 		log.WithError(err).Debugf("Could not impersonate user %v. The user could not be fetched from local store.", username)
-		return nil, trace.AccessDenied("access denied")
+		return nil, "", trace.AccessDenied("access denied")
 	}
 
 	// Do not allow SSO users to be impersonated.
 	if userState.GetUserType() == types.UserTypeSSO {
 		log.Warningf("Tried to issue a renewable cert for externally managed user %v, this is not supported.", username)
-		return nil, trace.AccessDenied("access denied")
+		return nil, "", trace.AccessDenied("access denied")
 	}
 
 	// Cap the cert TTL to the MaxRenewableCertTTL.
@@ -378,11 +380,11 @@ func (a *Server) generateInitialBotCerts(
 	accessInfo := services.AccessInfoFromUserState(userState)
 	clusterName, err := a.GetClusterName()
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, "", trace.Wrap(err)
 	}
 	checker, err := services.NewAccessChecker(accessInfo, clusterName.GetClusterName(), a)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, "", trace.Wrap(err)
 	}
 
 	// renewable cert request must include a generation
@@ -412,7 +414,7 @@ func (a *Server) generateInitialBotCerts(
 			// If no existing instance ID is known, create a new one.
 			uuid, err := uuid.NewRandom()
 			if err != nil {
-				return nil, trace.Wrap(err)
+				return nil, "", trace.Wrap(err)
 			}
 
 			bi := newBotInstance(&machineidv1pb.BotInstanceSpec{
@@ -422,7 +424,7 @@ func (a *Server) generateInitialBotCerts(
 
 			_, err = a.BotInstance.CreateBotInstance(ctx, bi)
 			if err != nil {
-				return nil, trace.Wrap(err)
+				return nil, "", trace.Wrap(err)
 			}
 
 			certReq.botInstanceID = uuid.String()
@@ -433,7 +435,7 @@ func (a *Server) generateInitialBotCerts(
 			// Note: botName is derived from the provision token rather than any
 			// value sent by the client, so we can trust it.
 			if err := a.updateBotInstance(ctx, &certReq, botName, existingInstanceID, initialAuth); err != nil {
-				return nil, trace.Wrap(err)
+				return nil, "", trace.Wrap(err)
 			}
 
 			// Only set the bot instance ID if it's empty; `updateBotInstance()`
@@ -445,13 +447,13 @@ func (a *Server) generateInitialBotCerts(
 	}
 
 	if err := a.validateGenerationLabel(ctx, userState.GetName(), &certReq, 0); err != nil {
-		return nil, trace.Wrap(err)
+		return nil, "", trace.Wrap(err)
 	}
 
 	certs, err := a.generateUserCert(ctx, certReq)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, "", trace.Wrap(err)
 	}
 
-	return certs, nil
+	return certs, certReq.botInstanceID, nil
 }

lib/auth/bot_test.go

@@ -33,6 +33,8 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/digitorus/pkcs7"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/uuid"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
@@ -48,13 +50,16 @@ import (
 	machineidv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1"
 	"github.com/gravitational/teleport/api/metadata"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/lib/auth/authclient"
 	"github.com/gravitational/teleport/lib/auth/join"
 	"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/auth/state"
 	"github.com/gravitational/teleport/lib/auth/testauthority"
 	"github.com/gravitational/teleport/lib/cloud/azure"
+	libevents "github.com/gravitational/teleport/lib/events"
+	"github.com/gravitational/teleport/lib/events/eventstest"
 	"github.com/gravitational/teleport/lib/fixtures"
 	"github.com/gravitational/teleport/lib/kubernetestoken"
 	"github.com/gravitational/teleport/lib/reversetunnelclient"
@@ -221,6 +226,9 @@ func TestRegisterBotInstance(t *testing.T) {
 	})
 
 	srv := newTestTLSServer(t)
+	// Inject mockEmitter to capture audit events
+	mockEmitter := &eventstest.MockRecorderEmitter{}
+	srv.Auth().SetEmitter(mockEmitter)
 	ctx := context.Background()
 
 	_, err := CreateRole(ctx, srv.Auth(), "example", types.RoleSpecV6{})
@@ -287,6 +295,76 @@ func TestRegisterBotInstance(t *testing.T) {
 	// only that record.)
 	require.Len(t, botInstance.GetStatus().LatestAuthentications, 1)
 	require.EqualExportedValues(t, ia, botInstance.GetStatus().LatestAuthentications[0])
+
+	// Validate that expected audit events were emitted...
+	auditEvents := mockEmitter.Events()
+	var joinEvent *events.BotJoin
+	for _, event := range auditEvents {
+		evt, ok := event.(*events.BotJoin)
+		if ok {
+			joinEvent = evt
+			break
+		}
+	}
+	require.NotNil(t, joinEvent)
+	require.Empty(t,
+		cmp.Diff(joinEvent, &events.BotJoin{
+			Metadata: events.Metadata{
+				Type: libevents.BotJoinEvent,
+				Code: libevents.BotJoinCode,
+			},
+			Status: events.Status{
+				Success: true,
+			},
+			UserName:  "bot-test",
+			BotName:   "test",
+			Method:    string(types.JoinMethodToken),
+			TokenName: token.GetSafeName(),
+			ConnectionMetadata: events.ConnectionMetadata{
+				RemoteAddr: "127.0.0.1",
+			},
+			BotInstanceID: ident.BotInstanceID,
+		},
+			// There appears to be a bug with cmp.Diff and nil event.Struct that
+			// causes a panic so let's just ignore it.
+			cmpopts.IgnoreFields(events.BotJoin{}, "Attributes"),
+			cmpopts.IgnoreFields(events.Metadata{}, "Time"),
+			cmpopts.EquateEmpty(),
+		),
+	)
+
+	var certIssueEvent *events.CertificateCreate
+	for _, event := range auditEvents {
+		evt, ok := event.(*events.CertificateCreate)
+		if ok {
+			certIssueEvent = evt
+			break
+		}
+	}
+	require.NotNil(t, certIssueEvent)
+	require.Empty(t,
+		cmp.Diff(certIssueEvent, &events.CertificateCreate{
+			Metadata: events.Metadata{
+				Type: libevents.CertificateCreateEvent,
+				Code: libevents.CertificateCreateCode,
+			},
+			CertificateType: "user",
+			Identity: &events.Identity{
+				User:             "bot-test",
+				Roles:            []string{"bot-test"},
+				RouteToCluster:   "localhost",
+				ClientIP:         "127.0.0.1",
+				TeleportCluster:  "localhost",
+				PrivateKeyPolicy: "none",
+				BotName:          "test",
+				BotInstanceID:    ident.BotInstanceID,
+			},
+		},
+			cmpopts.IgnoreFields(events.Metadata{}, "Time"),
+			cmpopts.IgnoreFields(events.Identity{}, "Logins", "Expires"),
+			cmpopts.EquateEmpty(),
+		),
+	)
 }
 
 // TestRegisterBotCertificateGenerationStolen simulates a stolen renewable

lib/auth/join.go

@@ -398,14 +398,22 @@ func (a *Server) generateCertsBot(
 		}
 	}
 
-	certs, err := a.generateInitialBotCerts(
-		ctx, botName, machineidv1.BotResourceName(botName), req.RemoteAddr,
-		req.PublicSSHKey, req.PublicTLSKey, expires, renewable, auth,
+	certs, botInstanceID, err := a.generateInitialBotCerts(
+		ctx,
+		botName,
+		machineidv1.BotResourceName(botName),
+		req.RemoteAddr,
+		req.PublicSSHKey,
+		req.PublicTLSKey,
+		expires,
+		renewable,
+		auth,
 		req.BotInstanceID,
 	)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	joinEvent.BotInstanceID = botInstanceID
 
 	if shouldDeleteToken {
 		// delete ephemeral bot join tokens so they can't be re-used
@@ -417,8 +425,7 @@ func (a *Server) generateCertsBot(
 	}
 
 	// Emit audit event for bot join.
-	log.Infof("Bot %q has joined the cluster.", botName)
-
+	log.Infof("Bot %q (instance: %s) has joined the cluster.", botName, botInstanceID)
 	if err := a.emitter.EmitAuditEvent(ctx, joinEvent); err != nil {
 		log.WithError(err).Warn("Failed to emit bot join event.")
 	}

lib/srv/ctx.go

@@ -1237,6 +1237,8 @@ func (id *IdentityContext) GetUserMetadata() apievents.UserMetadata {
 		AccessRequests: id.ActiveRequests,
 		TrustedDevice:  eventDeviceMetadataFromCert(id.Certificate),
 		UserKind:       userKind,
+		BotName:        id.BotName,
+		BotInstanceID:  id.BotInstanceID,
 	}
 }
 

lib/srv/ctx_test.go

@@ -232,6 +232,22 @@ func TestIdentityContext_GetUserMetadata(t *testing.T) {
 				UserKind: apievents.UserKind_USER_KIND_HUMAN,
 			},
 		},
+		{
+			name: "bot metadata",
+			idCtx: IdentityContext{
+				TeleportUser:  "bot-alpaca",
+				Login:         "alpaca1",
+				BotName:       "alpaca",
+				BotInstanceID: "123-123-123",
+			},
+			want: apievents.UserMetadata{
+				User:          "bot-alpaca",
+				Login:         "alpaca1",
+				UserKind:      apievents.UserKind_USER_KIND_BOT,
+				BotName:       "alpaca",
+				BotInstanceID: "123-123-123",
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

lib/tlsca/ca.go

@@ -354,6 +354,8 @@ func (id *Identity) GetEventIdentity() events.Identity {
 		AllowedResourceIDs:      events.ResourceIDs(id.AllowedResourceIDs),
 		PrivateKeyPolicy:        string(id.PrivateKeyPolicy),
 		DeviceExtensions:        devExts,
+		BotName:                 id.BotName,
+		BotInstanceID:           id.BotInstanceID,
 	}
 }
 
@@ -1135,6 +1137,8 @@ func (id Identity) GetUserMetadata() events.UserMetadata {
 		AccessRequests:    id.ActiveRequests,
 		UserKind:          userKind,
 		TrustedDevice:     device,
+		BotName:           id.BotName,
+		BotInstanceID:     id.BotInstanceID,
 	}
 }
 

lib/tlsca/ca_test.go

@@ -430,24 +430,15 @@ func TestIdentity_GetUserMetadata(t *testing.T) {
 		{
 			name: "user metadata for bot",
 			identity: Identity{
-				Username:     "alpaca",
-				Impersonator: "llama",
-				RouteToApp: RouteToApp{
-					AWSRoleARN:        "awsrolearn",
-					AzureIdentity:     "azureidentity",
-					GCPServiceAccount: "gcpaccount",
-				},
-				ActiveRequests: []string{"accessreq1", "accessreq2"},
-				BotName:        "foo",
+				Username:      "bot-alpaca",
+				BotName:       "alpaca",
+				BotInstanceID: "123-123",
 			},
 			want: apievents.UserMetadata{
-				User:              "alpaca",
-				Impersonator:      "llama",
-				AWSRoleARN:        "awsrolearn",
-				AccessRequests:    []string{"accessreq1", "accessreq2"},
-				AzureIdentity:     "azureidentity",
-				GCPServiceAccount: "gcpaccount",
-				UserKind:          apievents.UserKind_USER_KIND_BOT,
+				User:          "bot-alpaca",
+				UserKind:      apievents.UserKind_USER_KIND_BOT,
+				BotName:       "alpaca",
+				BotInstanceID: "123-123",
 			},
 		},
 		{