diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 0d101110763d3..f7dd6f8ffb192 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -18,12 +18,9 @@ package auth import ( "context" - "fmt" "net/url" - "strconv" "time" - "github.com/google/uuid" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" @@ -1649,76 +1646,6 @@ func (a *ServerWithRoles) NewKeepAliver(ctx context.Context) (types.KeepAliver, return nil, trace.NotImplemented(notImplementedMessage) } -// TODO(nic): move this to lib/auth/bot.go, leaving it here to keep a minimal diff -// generateInitialRenewableUserCerts is used to generate renewable 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 renewable -// certificates. -func (a *Server) generateInitialRenewableUserCerts(ctx context.Context, username string, pubKey []byte, expires time.Time) (*proto.Certs, error) { - var err error - - // Extract the user and role set for whom the certificate will be generated. - // This should be safe since this is typically done against a local user. - // - // This call bypasses RBAC check for users read on purpose. - // Users who are allowed to impersonate other users might not have - // permissions to read user data. - user, err := a.GetUser(username, false) - 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") - } - - // Do not allow SSO users to be impersonated. - if user.GetCreatedBy().Connector != nil { - log.Warningf("Tried to issue a renewable cert for externally managed user %v, this is not supported.", username) - return nil, trace.AccessDenied("access denied") - } - - // Cap the cert TTL to the MaxRenewableCertTTL. - if max := a.GetClock().Now().Add(defaults.MaxRenewableCertTTL); expires.After(max) { - expires = max - } - - // Inherit the user's roles and traits verbatim. - roles := user.GetRoles() - traits := user.GetTraits() - - parsedRoles, err := services.FetchRoleList(roles, a, traits) - if err != nil { - return nil, trace.Wrap(err) - } - // add implicit roles to the set and build a checker - checker := services.NewRoleSet(parsedRoles...) - - // Generate certificate - certReq := certRequest{ - user: user, - ttl: expires.Sub(a.GetClock().Now()), - publicKey: pubKey, - checker: checker, - traits: user.GetTraits(), - renewable: true, - includeHostCA: true, - generation: 1, - } - - if err := a.validateGenerationLabel(ctx, user, &certReq, 0); err != nil { - return nil, trace.Wrap(err) - } - - certs, err := a.generateUserCert(certReq) - if err != nil { - return nil, trace.Wrap(err) - } - - return certs, nil -} - // determineDesiredRolesAndTraits inspects the current request to determine // which roles and traits the requesting user wants to be present on the // resulting certificate. This does not attempt to determine if the @@ -1769,138 +1696,6 @@ func (a *ServerWithRoles) GenerateUserCerts(ctx context.Context, req proto.UserC return a.generateUserCerts(ctx, req) } -// TODO(nic): move this to lib/auth/bot.go, leaving it here to keep a minimal diff -// validateGenerationLabel validates and updates a generation label. -func (a *Server) validateGenerationLabel(ctx context.Context, user types.User, certReq *certRequest, currentIdentityGeneration uint64) error { - // Fetch the user, bypassing the cache. We might otherwise fetch a stale - // value in case of a rapid certificate renewal. - user, err := a.Identity.GetUser(user.GetName(), false) - if err != nil { - return trace.Wrap(err) - } - - var currentUserGeneration uint64 - label, labelOk := user.GetMetadata().Labels[types.BotGenerationLabel] - if labelOk { - currentUserGeneration, err = strconv.ParseUint(label, 10, 64) - if err != nil { - return trace.BadParameter("user has invalid value for label %q", types.BotGenerationLabel) - } - } - - // If there is no existing generation on any of the user, identity, or - // cert request, we have nothing to do here. - if currentUserGeneration == 0 && currentIdentityGeneration == 0 && certReq.generation == 0 { - return nil - } - - // By now, we know a generation counter is in play _somewhere_ and this is a - // bot certs. Bot certs should include the host CA so that they can make - // Teleport API calls. - certReq.includeHostCA = true - - // If the certReq already has generation set, it was explicitly requested - // (presumably this is the initial set of renewable certs). We'll want to - // commit that value to the User object. - if certReq.generation > 0 { - // ...however, if the user already has a stored generation, bail. - // (bots should be deleted and recreated if their certs expire) - if currentUserGeneration > 0 { - return trace.BadParameter( - "user %q has already been issued a renewable certificate and cannot be issued another; consider deleting and recreating the bot", - user.GetName(), - ) - } - - // Fetch a fresh copy of the user we can mutate safely. We can't - // implement a protobuf clone on User due to protobuf's proto.Clone() - // panicing when the user object has traits set, and a JSON - // marshal/unmarshal creates an import cycle so... here we are. - // There's a tiny chance the underlying user is mutated between calls - // to GetUser() but we're comparing with an older value so it'll fail - // safely. - newUser, err := a.Identity.GetUser(user.GetName(), false) - if err != nil { - return trace.Wrap(err) - } - metadata := newUser.GetMetadata() - metadata.Labels[types.BotGenerationLabel] = fmt.Sprint(certReq.generation) - newUser.SetMetadata(metadata) - - // Note: we bypass the RBAC check on purpose as bot users should not - // have user update permissions. - if err := a.CompareAndSwapUser(ctx, newUser, user); err != nil { - // If this fails it's likely to be some miscellaneous competing - // write. The request should be tried again - if it's malicious, - // someone will get a generation mismatch and trigger a lock. - return trace.WrapWithMessage(err, "Database comparison failed, try the request again") - } - - return nil - } - - // The current generations must match to continue: - if currentIdentityGeneration != currentUserGeneration { - // Lock the bot user indefinitely. - lock, err := types.NewLock(uuid.New().String(), types.LockSpecV2{ - Target: types.LockTarget{ - User: user.GetName(), - }, - Message: fmt.Sprintf( - "The bot user %q has been locked due to a certificate generation mismatch, possibly indicating a stolen certificate.", - user.GetName(), - ), - }) - if err != nil { - return trace.Wrap(err) - } - if err := a.UpsertLock(context.Background(), lock); err != nil { - return trace.Wrap(err) - } - - // Emit an audit event. - userMetadata := ClientUserMetadata(ctx) - if a.emitter.EmitAuditEvent(a.closeCtx, &apievents.RenewableCertificateGenerationMismatch{ - Metadata: apievents.Metadata{ - Type: events.RenewableCertificateGenerationMismatchEvent, - Code: events.RenewableCertificateGenerationMismatchCode, - }, - UserMetadata: userMetadata, - }); err != nil { - log.WithError(err).Warn("Failed to emit renewable cert generation mismatch event") - } - - return trace.AccessDenied( - "renewable cert generation mismatch: stored=%v, presented=%v", - currentUserGeneration, currentIdentityGeneration, - ) - } - - // Update the user with the new generation count. - newGeneration := currentIdentityGeneration + 1 - - // As above, commit some crimes to clone the User. - newUser, err := a.Identity.GetUser(user.GetName(), false) - if err != nil { - return trace.Wrap(err) - } - metadata := newUser.GetMetadata() - metadata.Labels[types.BotGenerationLabel] = fmt.Sprint(newGeneration) - newUser.SetMetadata(metadata) - - if err := a.CompareAndSwapUser(ctx, newUser, user); err != nil { - // If this fails it's likely to be some miscellaneous competing - // write. The request should be tried again - if it's malicious, - // someone will get a generation mismatch and trigger a lock. - return trace.WrapWithMessage(err, "Database comparison failed, try the request again") - } - - // And lastly, set the generation on the cert request. - certReq.generation = newGeneration - - return nil -} - func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserCertsRequest, opts ...certRequestOption) (*proto.Certs, error) { var err error diff --git a/lib/auth/bot.go b/lib/auth/bot.go index 0e4e2ef9bf994..071c4825dc6a2 100644 --- a/lib/auth/bot.go +++ b/lib/auth/bot.go @@ -19,15 +19,21 @@ package auth import ( "context" "fmt" + "strconv" "strings" "time" + "github.com/google/uuid" + "github.com/gravitational/trace" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" + apievents "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/events" + "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/utils" - "github.com/gravitational/trace" ) // BotResourceName returns the default name for resources associated with the @@ -261,3 +267,203 @@ func (s *Server) CreateBotProvisionToken(ctx context.Context, botName string, tt return token, nil } + +// validateGenerationLabel validates and updates a generation label. +func (a *Server) validateGenerationLabel(ctx context.Context, user types.User, certReq *certRequest, currentIdentityGeneration uint64) error { + // Fetch the user, bypassing the cache. We might otherwise fetch a stale + // value in case of a rapid certificate renewal. + user, err := a.Identity.GetUser(user.GetName(), false) + if err != nil { + return trace.Wrap(err) + } + + var currentUserGeneration uint64 + label, labelOk := user.GetMetadata().Labels[types.BotGenerationLabel] + if labelOk { + currentUserGeneration, err = strconv.ParseUint(label, 10, 64) + if err != nil { + return trace.BadParameter("user has invalid value for label %q", types.BotGenerationLabel) + } + } + + // If there is no existing generation on any of the user, identity, or + // cert request, we have nothing to do here. + if currentUserGeneration == 0 && currentIdentityGeneration == 0 && certReq.generation == 0 { + return nil + } + + // By now, we know a generation counter is in play _somewhere_ and this is a + // bot certs. Bot certs should include the host CA so that they can make + // Teleport API calls. + certReq.includeHostCA = true + + // If the certReq already has generation set, it was explicitly requested + // (presumably this is the initial set of renewable certs). We'll want to + // commit that value to the User object. + if certReq.generation > 0 { + // ...however, if the user already has a stored generation, bail. + // (bots should be deleted and recreated if their certs expire) + if currentUserGeneration > 0 { + return trace.BadParameter( + "user %q has already been issued a renewable certificate and cannot be issued another; consider deleting and recreating the bot", + user.GetName(), + ) + } + + // Fetch a fresh copy of the user we can mutate safely. We can't + // implement a protobuf clone on User due to protobuf's proto.Clone() + // panicing when the user object has traits set, and a JSON + // marshal/unmarshal creates an import cycle so... here we are. + // There's a tiny chance the underlying user is mutated between calls + // to GetUser() but we're comparing with an older value so it'll fail + // safely. + newUser, err := a.Identity.GetUser(user.GetName(), false) + if err != nil { + return trace.Wrap(err) + } + metadata := newUser.GetMetadata() + metadata.Labels[types.BotGenerationLabel] = fmt.Sprint(certReq.generation) + newUser.SetMetadata(metadata) + + // Note: we bypass the RBAC check on purpose as bot users should not + // have user update permissions. + if err := a.CompareAndSwapUser(ctx, newUser, user); err != nil { + // If this fails it's likely to be some miscellaneous competing + // write. The request should be tried again - if it's malicious, + // someone will get a generation mismatch and trigger a lock. + return trace.WrapWithMessage(err, "Database comparison failed, try the request again") + } + + return nil + } + + // The current generations must match to continue: + if currentIdentityGeneration != currentUserGeneration { + // Lock the bot user indefinitely. + lock, err := types.NewLock(uuid.New().String(), types.LockSpecV2{ + Target: types.LockTarget{ + User: user.GetName(), + }, + Message: fmt.Sprintf( + "The bot user %q has been locked due to a certificate generation mismatch, possibly indicating a stolen certificate.", + user.GetName(), + ), + }) + if err != nil { + return trace.Wrap(err) + } + if err := a.UpsertLock(context.Background(), lock); err != nil { + return trace.Wrap(err) + } + + // Emit an audit event. + userMetadata := ClientUserMetadata(ctx) + if a.emitter.EmitAuditEvent(a.closeCtx, &apievents.RenewableCertificateGenerationMismatch{ + Metadata: apievents.Metadata{ + Type: events.RenewableCertificateGenerationMismatchEvent, + Code: events.RenewableCertificateGenerationMismatchCode, + }, + UserMetadata: userMetadata, + }); err != nil { + log.WithError(err).Warn("Failed to emit renewable cert generation mismatch event") + } + + return trace.AccessDenied( + "renewable cert generation mismatch: stored=%v, presented=%v", + currentUserGeneration, currentIdentityGeneration, + ) + } + + // Update the user with the new generation count. + newGeneration := currentIdentityGeneration + 1 + + // As above, commit some crimes to clone the User. + newUser, err := a.Identity.GetUser(user.GetName(), false) + if err != nil { + return trace.Wrap(err) + } + metadata := newUser.GetMetadata() + metadata.Labels[types.BotGenerationLabel] = fmt.Sprint(newGeneration) + newUser.SetMetadata(metadata) + + if err := a.CompareAndSwapUser(ctx, newUser, user); err != nil { + // If this fails it's likely to be some miscellaneous competing + // write. The request should be tried again - if it's malicious, + // someone will get a generation mismatch and trigger a lock. + return trace.WrapWithMessage(err, "Database comparison failed, try the request again") + } + + // And lastly, set the generation on the cert request. + certReq.generation = newGeneration + + return nil +} + +// generateInitialRenewableUserCerts is used to generate renewable 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 renewable +// certificates. +func (a *Server) generateInitialRenewableUserCerts(ctx context.Context, username string, pubKey []byte, expires time.Time) (*proto.Certs, error) { + var err error + + // Extract the user and role set for whom the certificate will be generated. + // This should be safe since this is typically done against a local user. + // + // This call bypasses RBAC check for users read on purpose. + // Users who are allowed to impersonate other users might not have + // permissions to read user data. + user, err := a.GetUser(username, false) + 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") + } + + // Do not allow SSO users to be impersonated. + if user.GetCreatedBy().Connector != nil { + log.Warningf("Tried to issue a renewable cert for externally managed user %v, this is not supported.", username) + return nil, trace.AccessDenied("access denied") + } + + // Cap the cert TTL to the MaxRenewableCertTTL. + if max := a.GetClock().Now().Add(defaults.MaxRenewableCertTTL); expires.After(max) { + expires = max + } + + // Inherit the user's roles and traits verbatim. + roles := user.GetRoles() + traits := user.GetTraits() + + parsedRoles, err := services.FetchRoleList(roles, a, traits) + if err != nil { + return nil, trace.Wrap(err) + } + // add implicit roles to the set and build a checker + checker := services.NewRoleSet(parsedRoles...) + + // Generate certificate + certReq := certRequest{ + user: user, + ttl: expires.Sub(a.GetClock().Now()), + publicKey: pubKey, + checker: checker, + traits: user.GetTraits(), + renewable: true, + includeHostCA: true, + generation: 1, + } + + if err := a.validateGenerationLabel(ctx, user, &certReq, 0); err != nil { + return nil, trace.Wrap(err) + } + + certs, err := a.generateUserCert(certReq) + if err != nil { + return nil, trace.Wrap(err) + } + + return certs, nil +}