Skip to content

Commit

Permalink
Add bot field to certificates and various usage events (#35881)
Browse files Browse the repository at this point in the history
* Add bot field to certificates and various usage events

This adds a new certificate extension field, `teleport-bot`, to
certificates issued to Machine ID bot users that can definitively
identify certificates as having been issued to a bot user.

Additionally, this uses the new `Bot` identity flag to mark certain
usage events as originating from bot users. As such, it includes a
protobuf update from Cloud [1], which pulled in some small additional
(mostly comment) changes.

[1] gravitational/cloud#7060

* Small bot flag plumbing fixes

* Convert bot flag to BotName and UserKind enum

This makes a few changes to the bot tagging approach:
* The bot name is embedded in the cert rather than just true/false
* UserKind is included in events rather than just a bot flag, to
  allow for an unspecified value for older client nodes.

* Add a quick unit test for bot cert extensions

* Fix outdated grpc

* Include bot flag on initial certs

* Log a warning and override user kind for usage records if they differ

* Fix several unit tests; add a bot metadata test case

* Fix unit tests with UserKind zero value

* Rename SSH cert extension to use standard format

Renames the `teleport-bot` extension to `bot-name@goteleport.com`,
to better follow SSH cert extension naming conventions.

* Attempt to improve unspecified userkind aggregating logic
  • Loading branch information
timothyb89 authored and ibeckermayer committed Jan 17, 2024
1 parent db732a5 commit 27dd212
Show file tree
Hide file tree
Showing 21 changed files with 1,266 additions and 859 deletions.
22 changes: 22 additions & 0 deletions api/proto/teleport/legacy/types/events/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ message SessionMetadata {
string PrivateKeyPolicy = 3 [(gogoproto.jsontag) = "private_key_policy,omitempty"];
}

// The kind of user a given username refers to. Usernames should always refer to
// a valid cluster user (even if temporary, e.g. SSO), but may be Machine ID
// bot users.
enum UserKind {
// Indicates a legacy cluster emitting events without a defined user kind.
USER_KIND_UNSPECIFIED = 0;

// Indicates the user associated with this event is human, either created
// locally or via SSO.
USER_KIND_HUMAN = 1;

// Indicates the user associated with this event is a Machine ID bot user.
USER_KIND_BOT = 2;
}

// UserMetadata is a common user event metadata
message UserMetadata {
// User is teleport user name
Expand Down Expand Up @@ -92,6 +107,10 @@ message UserMetadata {

// RequiredPrivateKeyPolicy is the private key policy enforced for this login.
string RequiredPrivateKeyPolicy = 9 [(gogoproto.jsontag) = "required_private_key_policy,omitempty"];

// 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"];
}

// Server is a server metadata
Expand Down Expand Up @@ -3872,6 +3891,9 @@ message Identity {
repeated string GCPServiceAccounts = 25 [(gogoproto.jsontag) = "gcp_service_accounts,omitempty"];
// PrivateKeyPolicy is the private key policy of the user's private key.
string PrivateKeyPolicy = 26 [(gogoproto.jsontag) = "private_key_policy,omitempty"];
// BotName indicates the name of the Machine ID bot this identity was issued
// to, if any.
string BotName = 27 [(gogoproto.jsontag) = "bot_name,omitempty"];
}

// RouteToApp contains parameters for application access certificate requests.
Expand Down
1,794 changes: 956 additions & 838 deletions api/types/events/events.pb.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,9 @@ const (
// CertExtensionDeviceCredentialID is the identifier for the credential used
// by the device to authenticate itself.
CertExtensionDeviceCredentialID = "teleport-device-credential-id"
// CertExtensionBotName indicates the name of the Machine ID bot this
// certificate was issued to, if any.
CertExtensionBotName = "bot-name@goteleport.com"

// CertCriticalOptionSourceAddress is a critical option that defines IP addresses (in CIDR notation)
// from which this certificate is accepted for authentication.
Expand Down
4 changes: 4 additions & 0 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -1775,6 +1775,8 @@ type certRequest struct {
skipAttestation bool
// deviceExtensions holds device-aware user certificate extensions.
deviceExtensions DeviceExtensions
// botName is the name of the bot requesting this cert, if any
botName string
}

// check verifies the cert request is valid.
Expand Down Expand Up @@ -2520,6 +2522,7 @@ func generateCert(a *Server, req certRequest, caType types.CertAuthType) (*proto
DisallowReissue: req.disallowReissue,
Renewable: req.renewable,
Generation: req.generation,
BotName: req.botName,
CertificateExtensions: req.checker.CertificateExtensions(),
AllowedResourceIDs: requestedResourcesStr,
ConnectionDiagnosticID: req.connectionDiagnosticID,
Expand Down Expand Up @@ -2616,6 +2619,7 @@ func generateCert(a *Server, req certRequest, caType types.CertAuthType) (*proto
DisallowReissue: req.disallowReissue,
Renewable: req.renewable,
Generation: req.generation,
BotName: req.botName,
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 @@ -2790,6 +2790,17 @@ func isRoleImpersonation(req proto.UserCertsRequest) bool {
return req.UseRoleRequests || len(req.RoleRequests) > 0
}

// getBotName returns the name of the bot embedded in the user metadata, if any.
// For non-bot users, returns "".
func getBotName(user types.User) string {
name, ok := user.GetLabel(types.BotLabel)
if ok {
return name
}

return ""
}

func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserCertsRequest, opts ...certRequestOption) (*proto.Certs, error) {
// Device trust: authorize device before issuing certificates.
authPref, err := a.authServer.GetAuthPreference(ctx)
Expand Down Expand Up @@ -3032,6 +3043,7 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
},
connectionDiagnosticID: req.ConnectionDiagnosticID,
attestationStatement: keys.AttestationStatementFromProto(req.AttestationStatement),
botName: getBotName(user),
}
if user.GetName() != a.context.User.GetName() {
certReq.impersonator = a.context.User.GetName()
Expand Down
3 changes: 2 additions & 1 deletion lib/auth/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (a *Server) validateGenerationLabel(ctx context.Context, username string, c
// 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, 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) (*proto.Certs, error) {
var err error

// Extract the user and role set for whom the certificate will be generated.
Expand Down Expand Up @@ -229,6 +229,7 @@ func (a *Server) generateInitialBotCerts(ctx context.Context, username, loginIP
includeHostCA: true,
generation: generation,
loginIP: loginIP,
botName: botName,
}

if err := a.validateGenerationLabel(ctx, userState.GetName(), &certReq, 0); err != nil {
Expand Down
71 changes: 71 additions & 0 deletions lib/auth/bot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,77 @@ func TestRegisterBotCertificateGenerationStolen(t *testing.T) {
require.NotEmpty(t, locks)
}

// TestRegisterBotCertificateExtensions ensures bot cert extensions are present.
func TestRegisterBotCertificateExtensions(t *testing.T) {
t.Parallel()
srv := newTestTLSServer(t)
ctx := context.Background()

_, err := CreateRole(ctx, srv.Auth(), "example", types.RoleSpecV6{})
require.NoError(t, err)

// Create a new bot.
client, err := srv.NewClient(TestAdmin())
require.NoError(t, err)
bot, err := client.BotServiceClient().CreateBot(ctx, &machineidv1pb.CreateBotRequest{
Bot: &machineidv1pb.Bot{
Metadata: &headerv1.Metadata{
Name: "test",
},
Spec: &machineidv1pb.BotSpec{
Roles: []string{"example"},
},
},
})
require.NoError(t, err)

token, err := types.NewProvisionTokenFromSpec("testxyzzy", time.Time{}, types.ProvisionTokenSpecV2{
Roles: types.SystemRoles{types.RoleBot},
BotName: bot.Metadata.Name,
})
require.NoError(t, err)
require.NoError(t, client.CreateToken(ctx, token))

privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)
sshPrivateKey, err := ssh.ParseRawPrivateKey(privateKey)
require.NoError(t, err)
tlsPublicKey, err := tlsca.MarshalPublicKeyFromPrivateKeyPEM(sshPrivateKey)
require.NoError(t, err)

certs, err := Register(RegisterParams{
Token: token.GetName(),
ID: IdentityID{
Role: types.RoleBot,
},
AuthServers: []utils.NetAddr{*utils.MustParseAddr(srv.Addr().String())},
PublicTLSKey: tlsPublicKey,
PublicSSHKey: publicKey,
})
require.NoError(t, err)
checkCertLoginIP(t, certs.TLS, "127.0.0.1")

tlsCert, err := tls.X509KeyPair(certs.TLS, privateKey)
require.NoError(t, err)

_, certs, _, err = renewBotCerts(ctx, srv, tlsCert, bot.Status.UserName, publicKey, privateKey)
require.NoError(t, err)

// Parse the Identity
impersonatedTLSCert, err := tlsca.ParseCertificatePEM(certs.TLS)
require.NoError(t, err)
impersonatedIdent, err := tlsca.FromSubject(impersonatedTLSCert.Subject, impersonatedTLSCert.NotAfter)
require.NoError(t, err)

// Check for proper cert extensions
require.True(t, impersonatedIdent.Renewable)
require.False(t, impersonatedIdent.DisallowReissue)
require.Equal(t, "test", impersonatedIdent.BotName)

// Initial certs have generation=1 and we start with a renewal, so add 2
require.Equal(t, uint64(2), impersonatedIdent.Generation)
}

// TestRegisterBot_RemoteAddr checks that certs returned for bot registration contain specified in the request remote addr.
func TestRegisterBot_RemoteAddr(t *testing.T) {
t.Parallel()
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (a *Server) generateCertsBot(
}

certs, err := a.generateInitialBotCerts(
ctx, machineidv1.BotResourceName(botName), req.RemoteAddr, req.PublicSSHKey, expires, renewable,
ctx, botName, machineidv1.BotResourceName(botName), req.RemoteAddr, req.PublicSSHKey, expires, renewable,
)
if err != nil {
return nil, trace.Wrap(err)
Expand Down
3 changes: 3 additions & 0 deletions lib/auth/keygen/keygen.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ func (k *Keygen) GenerateUserCertWithoutValidation(c services.UserCertParams) ([
if c.Generation > 0 {
cert.Permissions.Extensions[teleport.CertExtensionGeneration] = fmt.Sprint(c.Generation)
}
if c.BotName != "" {
cert.Permissions.Extensions[teleport.CertExtensionBotName] = c.BotName
}
if c.AllowedResourceIDs != "" {
cert.Permissions.Extensions[teleport.CertExtensionAllowedResources] = c.AllowedResourceIDs
}
Expand Down
15 changes: 10 additions & 5 deletions lib/auth/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4055,7 +4055,8 @@ func TestGRPCServer_CreateTokenV2(t *testing.T) {
Code: events.ProvisionTokenCreateCode,
},
UserMetadata: eventtypes.UserMetadata{
User: "token-creator",
User: "token-creator",
UserKind: eventtypes.UserKind_USER_KIND_HUMAN,
},
Roles: types.SystemRoles{types.RoleNode, types.RoleKube},
JoinMethod: types.JoinMethodToken,
Expand Down Expand Up @@ -4083,7 +4084,8 @@ func TestGRPCServer_CreateTokenV2(t *testing.T) {
Code: events.ProvisionTokenCreateCode,
},
UserMetadata: eventtypes.UserMetadata{
User: "token-creator",
User: "token-creator",
UserKind: eventtypes.UserKind_USER_KIND_HUMAN,
},
Roles: types.SystemRoles{types.RoleTrustedCluster},
JoinMethod: types.JoinMethodToken,
Expand Down Expand Up @@ -4204,7 +4206,8 @@ func TestGRPCServer_UpsertTokenV2(t *testing.T) {
Code: events.ProvisionTokenCreateCode,
},
UserMetadata: eventtypes.UserMetadata{
User: "token-upserter",
User: "token-upserter",
UserKind: eventtypes.UserKind_USER_KIND_HUMAN,
},
Roles: types.SystemRoles{types.RoleNode, types.RoleKube},
JoinMethod: types.JoinMethodToken,
Expand Down Expand Up @@ -4232,7 +4235,8 @@ func TestGRPCServer_UpsertTokenV2(t *testing.T) {
Code: events.ProvisionTokenCreateCode,
},
UserMetadata: eventtypes.UserMetadata{
User: "token-upserter",
User: "token-upserter",
UserKind: eventtypes.UserKind_USER_KIND_HUMAN,
},
Roles: types.SystemRoles{types.RoleTrustedCluster},
JoinMethod: types.JoinMethodToken,
Expand Down Expand Up @@ -4262,7 +4266,8 @@ func TestGRPCServer_UpsertTokenV2(t *testing.T) {
Code: events.ProvisionTokenCreateCode,
},
UserMetadata: eventtypes.UserMetadata{
User: "token-upserter",
User: "token-upserter",
UserKind: eventtypes.UserKind_USER_KIND_HUMAN,
},
Roles: types.SystemRoles{types.RoleNode},
JoinMethod: types.JoinMethodToken,
Expand Down
5 changes: 4 additions & 1 deletion lib/services/authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,13 @@ type UserCertParams struct {
DisallowReissue bool
// CertificateExtensions are user configured ssh key extensions
CertificateExtensions []*types.CertExtension
// Renewable indicates this certificate is renewable
// Renewable indicates this certificate is renewable.
Renewable bool
// Generation counts the number of times a certificate has been renewed.
Generation uint64
// BotName is set to the name of the bot, if the user is a Machine ID bot user.
// Empty for human users.
BotName string
// AllowedResourceIDs lists the resources the user should be able to access.
AllowedResourceIDs string
// ConnectionDiagnosticID references the ConnectionDiagnostic that we should use to append traces when testing a Connection.
Expand Down
3 changes: 3 additions & 0 deletions lib/srv/authhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ func (h *AuthHandlers) CreateIdentityContext(sconn *ssh.ServerConn) (IdentityCon
if _, ok := certificate.Extensions[teleport.CertExtensionRenewable]; ok {
identity.Renewable = true
}
if botName, ok := certificate.Extensions[teleport.CertExtensionBotName]; ok {
identity.BotName = botName
}
if generationStr, ok := certificate.Extensions[teleport.CertExtensionGeneration]; ok {
generation, err := strconv.ParseUint(generationStr, 10, 64)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions lib/srv/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ type IdentityContext struct {
// been renewed.
Generation uint64

// BotName is the name of the Machine ID bot this identity is associated
// with, if any.
BotName string

// AllowedResourceIDs lists the resources this identity should be allowed to
// access
AllowedResourceIDs []types.ResourceID
Expand Down Expand Up @@ -1196,12 +1200,18 @@ func eventDeviceMetadataFromCert(cert *ssh.Certificate) *apievents.DeviceMetadat
}

func (id *IdentityContext) GetUserMetadata() apievents.UserMetadata {
userKind := apievents.UserKind_USER_KIND_HUMAN
if id.BotName != "" {
userKind = apievents.UserKind_USER_KIND_BOT
}

return apievents.UserMetadata{
Login: id.Login,
User: id.TeleportUser,
Impersonator: id.Impersonator,
AccessRequests: id.ActiveRequests,
TrustedDevice: eventDeviceMetadataFromCert(id.Certificate),
UserKind: userKind,
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/srv/ctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ func TestIdentityContext_GetUserMetadata(t *testing.T) {
Login: "alpaca1",
Impersonator: "llama",
AccessRequests: []string{"access-req1", "access-req2"},
UserKind: apievents.UserKind_USER_KIND_HUMAN,
},
},
{
Expand All @@ -225,6 +226,7 @@ func TestIdentityContext_GetUserMetadata(t *testing.T) {
AssetTag: "assettag1",
CredentialId: "credentialid1",
},
UserKind: apievents.UserKind_USER_KIND_HUMAN,
},
},
}
Expand Down

0 comments on commit 27dd212

Please sign in to comment.