From 11417cdf98a177bf48e562261c794a7decb48f4f Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Wed, 5 Mar 2025 20:40:32 -0700 Subject: [PATCH 1/3] Machine ID: Warn when returned cert TTL is less than expected This adds a warning when the returned certificate TTL is lower than requested, which can happen if `max_session_ttl` is set in a bot role. Fixes #29579 --- lib/tbot/output_utils.go | 41 +++++++++++++++++++++++++++++ lib/tbot/service_identity_output.go | 2 ++ 2 files changed, 43 insertions(+) diff --git a/lib/tbot/output_utils.go b/lib/tbot/output_utils.go index bd94651b39246..eb44c08c83186 100644 --- a/lib/tbot/output_utils.go +++ b/lib/tbot/output_utils.go @@ -396,6 +396,47 @@ func generateIdentity( return newIdentity, nil } +// warnOnEarlyExpiration logs a warning if the given identity is likely to +// expire problematically early. This can happen if either the configured TTL is +// less than the renewal interval, or if the server returns certs valid for a +// shorter-than-expected period of time. +// This assumes the identity was just renewed, for the purposes of calculating +// TTLs. +func warnOnEarlyExpiration( + ctx context.Context, + log *slog.Logger, + ident *identity.Identity, + lifetime config.CredentialLifetime, +) { + // Calculate a rough TTL, assuming this was called shortly after the + // identity was returned. We'll add a minute buffer to compensate and avoid + // superfluous warning messages. + effectiveTTL := ident.TLSIdentity.Expires.Sub(time.Now()) + time.Minute + + if effectiveTTL < lifetime.TTL { + l := log.With( + "requested_ttl", lifetime.TTL, + "renewal_interval", lifetime.RenewalInterval, + "effective_ttl", effectiveTTL, + "expires", ident.TLSIdentity.Expires, + "roles", ident.TLSIdentity.Groups, + ) + + if effectiveTTL < lifetime.RenewalInterval { + l.WarnContext(ctx, "The server returned an identity shorter than "+ + "expected and below the configured renewal interval, probably "+ + "due to a `max_session_ttl` configured on a server-side role. "+ + "Unless corrected, the credentials will be invalid for some "+ + "period until renewal.") + } else { + l.WarnContext(ctx, "The server returned an identity shorter than "+ + "the requested TTL, probably due to a `max_session_ttl` "+ + "configured on a server-side role. It may not remain valid as "+ + "long as expected.") + } + } +} + // fetchDefaultRoles requests the bot's own role from the auth server and // extracts its full list of allowed roles. func fetchDefaultRoles(ctx context.Context, roleGetter services.RoleGetter, identity *identity.Identity) ([]string, error) { diff --git a/lib/tbot/service_identity_output.go b/lib/tbot/service_identity_output.go index a2aedcdbf3e80..f8aa434cb3434 100644 --- a/lib/tbot/service_identity_output.go +++ b/lib/tbot/service_identity_output.go @@ -152,6 +152,8 @@ func (s *IdentityOutputService) generate(ctx context.Context) error { } } + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime)) + hostCAs, err := s.botAuthClient.GetCertAuthorities(ctx, types.HostCA, false) if err != nil { return trace.Wrap(err) From 168722e65b9bcefc654e968c4ffb344082117336 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Tue, 11 Mar 2025 13:14:13 -0400 Subject: [PATCH 2/3] Add warning on most outputs; add TODO comment for future improvements --- lib/tbot/output_utils.go | 7 ++++++- lib/tbot/service_application_output.go | 5 ++++- lib/tbot/service_database_output.go | 5 ++++- lib/tbot/service_identity_output.go | 5 +++-- lib/tbot/service_kubernetes_output.go | 5 ++++- lib/tbot/service_kubernetes_v2_output.go | 6 +++++- lib/tbot/service_spiffe_svid_output.go | 6 +++++- lib/tbot/service_ssh_host_output.go | 6 +++++- lib/tbot/service_workload_identity_jwt.go | 6 +++++- lib/tbot/service_workload_identity_x509.go | 6 +++++- 10 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lib/tbot/output_utils.go b/lib/tbot/output_utils.go index eb44c08c83186..c788323ab8705 100644 --- a/lib/tbot/output_utils.go +++ b/lib/tbot/output_utils.go @@ -401,7 +401,8 @@ func generateIdentity( // less than the renewal interval, or if the server returns certs valid for a // shorter-than-expected period of time. // This assumes the identity was just renewed, for the purposes of calculating -// TTLs. +// TTLs, and may log false positive warnings if the time delta is large; the +// time calculations include a 1m buffer to mitigate this. func warnOnEarlyExpiration( ctx context.Context, log *slog.Logger, @@ -422,6 +423,10 @@ func warnOnEarlyExpiration( "roles", ident.TLSIdentity.Groups, ) + // TODO(timothyb89): we can technically fetch our individual roles + // without explicit permission, and could determine which role in + // particular limited the TTL. + if effectiveTTL < lifetime.RenewalInterval { l.WarnContext(ctx, "The server returned an identity shorter than "+ "expected and below the configured renewal interval, probably "+ diff --git a/lib/tbot/service_application_output.go b/lib/tbot/service_application_output.go index 4339f302da765..bd0b7585b84c4 100644 --- a/lib/tbot/service_application_output.go +++ b/lib/tbot/service_application_output.go @@ -131,12 +131,13 @@ func (s *ApplicationOutputService) generate(ctx context.Context) error { return trace.Wrap(err) } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) routedIdentity, err := generateIdentity( ctx, s.botAuthClient, id, roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, func(req *proto.UserCertsRequest) { req.RouteToApp = routeToApp }, @@ -145,6 +146,8 @@ func (s *ApplicationOutputService) generate(ctx context.Context) error { return trace.Wrap(err) } + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + s.log.InfoContext( ctx, "Generated identity for app", diff --git a/lib/tbot/service_database_output.go b/lib/tbot/service_database_output.go index ccceecdf082e9..5295500908b77 100644 --- a/lib/tbot/service_database_output.go +++ b/lib/tbot/service_database_output.go @@ -133,12 +133,13 @@ func (s *DatabaseOutputService) generate(ctx context.Context) error { return trace.Wrap(err) } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) routedIdentity, err := generateIdentity( ctx, s.botAuthClient, id, roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, func(req *proto.UserCertsRequest) { req.RouteToDatabase = route }, @@ -147,6 +148,8 @@ func (s *DatabaseOutputService) generate(ctx context.Context) error { return trace.Wrap(err) } + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + s.log.InfoContext( ctx, "Generated identity for database", diff --git a/lib/tbot/service_identity_output.go b/lib/tbot/service_identity_output.go index f8aa434cb3434..e7569ecd7b14f 100644 --- a/lib/tbot/service_identity_output.go +++ b/lib/tbot/service_identity_output.go @@ -113,12 +113,13 @@ func (s *IdentityOutputService) generate(ctx context.Context) error { } } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) id, err := generateIdentity( ctx, s.botAuthClient, s.getBotIdentity(), roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, func(req *proto.UserCertsRequest) { req.ReissuableRoleImpersonation = s.cfg.AllowReissue }, @@ -152,7 +153,7 @@ func (s *IdentityOutputService) generate(ctx context.Context) error { } } - warnOnEarlyExpiration(ctx, s.log.With("output", s), id, cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime)) + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) hostCAs, err := s.botAuthClient.GetCertAuthorities(ctx, types.HostCA, false) if err != nil { diff --git a/lib/tbot/service_kubernetes_output.go b/lib/tbot/service_kubernetes_output.go index b413e5e8cdf4d..f8464b560c9cf 100644 --- a/lib/tbot/service_kubernetes_output.go +++ b/lib/tbot/service_kubernetes_output.go @@ -149,12 +149,13 @@ func (s *KubernetesOutputService) generate(ctx context.Context) error { // and will fail to generate certs if the cluster doesn't exist or is // offline. + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) routedIdentity, err := generateIdentity( ctx, s.botAuthClient, id, roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, func(req *proto.UserCertsRequest) { req.KubernetesCluster = kubeClusterName }, @@ -163,6 +164,8 @@ func (s *KubernetesOutputService) generate(ctx context.Context) error { return trace.Wrap(err) } + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + s.log.InfoContext( ctx, "Generated identity for Kubernetes cluster", diff --git a/lib/tbot/service_kubernetes_v2_output.go b/lib/tbot/service_kubernetes_v2_output.go index b527379229243..506b695e5f93a 100644 --- a/lib/tbot/service_kubernetes_v2_output.go +++ b/lib/tbot/service_kubernetes_v2_output.go @@ -112,17 +112,21 @@ func (s *KubernetesV2OutputService) generate(ctx context.Context) error { return trace.Wrap(err, "fetching default roles") } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) id, err := generateIdentity( ctx, s.botAuthClient, s.getBotIdentity(), roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, nil, ) if err != nil { return trace.Wrap(err, "generating identity") } + + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + // create a client that uses the impersonated identity, so that when we // fetch information, we can ensure access rights are enforced. facade := identity.NewFacade(s.botCfg.FIPS, s.botCfg.Insecure, id) diff --git a/lib/tbot/service_spiffe_svid_output.go b/lib/tbot/service_spiffe_svid_output.go index 59a2b53f9452e..eb1ac9f3d00d0 100644 --- a/lib/tbot/service_spiffe_svid_output.go +++ b/lib/tbot/service_spiffe_svid_output.go @@ -179,17 +179,21 @@ func (s *SPIFFESVIDOutputService) requestSVID( return nil, nil, nil, trace.Wrap(err, "fetching roles") } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) id, err := generateIdentity( ctx, s.botAuthClient, s.getBotIdentity(), roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, nil, ) if err != nil { return nil, nil, nil, trace.Wrap(err, "generating identity") } + + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + // create a client that uses the impersonated identity, so that when we // fetch information, we can ensure access rights are enforced. facade := identity.NewFacade(s.botCfg.FIPS, s.botCfg.Insecure, id) diff --git a/lib/tbot/service_ssh_host_output.go b/lib/tbot/service_ssh_host_output.go index 9239373efd614..716ccc6b219a0 100644 --- a/lib/tbot/service_ssh_host_output.go +++ b/lib/tbot/service_ssh_host_output.go @@ -104,17 +104,21 @@ func (s *SSHHostOutputService) generate(ctx context.Context) error { } } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) id, err := generateIdentity( ctx, s.botAuthClient, s.getBotIdentity(), roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, nil, ) if err != nil { return trace.Wrap(err, "generating identity") } + + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + // create a client that uses the impersonated identity, so that when we // fetch information, we can ensure access rights are enforced. facade := identity.NewFacade(s.botCfg.FIPS, s.botCfg.Insecure, id) diff --git a/lib/tbot/service_workload_identity_jwt.go b/lib/tbot/service_workload_identity_jwt.go index 22088f8a82c53..2d31f0060ca3c 100644 --- a/lib/tbot/service_workload_identity_jwt.go +++ b/lib/tbot/service_workload_identity_jwt.go @@ -173,18 +173,22 @@ func (s *WorkloadIdentityJWTService) requestJWTSVID( } defer impersonatedClient.Close() + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) credentials, err := workloadidentity.IssueJWTWorkloadIdentity( ctx, s.log, impersonatedClient, s.cfg.Selector, s.cfg.Audiences, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, nil, ) if err != nil { return nil, trace.Wrap(err, "generating JWT SVID") } + + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + var credential *workloadidentityv1pb.Credential switch len(credentials) { case 0: diff --git a/lib/tbot/service_workload_identity_x509.go b/lib/tbot/service_workload_identity_x509.go index 3a980527c2086..4abaf1f2129d0 100644 --- a/lib/tbot/service_workload_identity_x509.go +++ b/lib/tbot/service_workload_identity_x509.go @@ -191,17 +191,21 @@ func (s *WorkloadIdentityX509Service) requestSVID( return nil, nil, trace.Wrap(err, "fetching roles") } + effectiveLifetime := cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime) id, err := generateIdentity( ctx, s.botAuthClient, s.getBotIdentity(), roles, - cmp.Or(s.cfg.CredentialLifetime, s.botCfg.CredentialLifetime).TTL, + effectiveLifetime.TTL, nil, ) if err != nil { return nil, nil, trace.Wrap(err, "generating identity") } + + warnOnEarlyExpiration(ctx, s.log.With("output", s), id, effectiveLifetime) + // create a client that uses the impersonated identity, so that when we // fetch information, we can ensure access rights are enforced. facade := identity.NewFacade(s.botCfg.FIPS, s.botCfg.Insecure, id) From b4126f876bdb8aeb77872f67305861a1afe9df94 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Tue, 11 Mar 2025 14:51:25 -0400 Subject: [PATCH 3/3] Mute slonglint false positivess --- lib/tbot/output_utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/tbot/output_utils.go b/lib/tbot/output_utils.go index c788323ab8705..3a8834d04945e 100644 --- a/lib/tbot/output_utils.go +++ b/lib/tbot/output_utils.go @@ -412,7 +412,7 @@ func warnOnEarlyExpiration( // Calculate a rough TTL, assuming this was called shortly after the // identity was returned. We'll add a minute buffer to compensate and avoid // superfluous warning messages. - effectiveTTL := ident.TLSIdentity.Expires.Sub(time.Now()) + time.Minute + effectiveTTL := time.Until(ident.TLSIdentity.Expires) + time.Minute if effectiveTTL < lifetime.TTL { l := log.With( @@ -428,12 +428,14 @@ func warnOnEarlyExpiration( // particular limited the TTL. if effectiveTTL < lifetime.RenewalInterval { + //nolint:sloglint // multiline string is actually constant l.WarnContext(ctx, "The server returned an identity shorter than "+ "expected and below the configured renewal interval, probably "+ "due to a `max_session_ttl` configured on a server-side role. "+ "Unless corrected, the credentials will be invalid for some "+ "period until renewal.") } else { + //nolint:sloglint // multiline string is actually constant l.WarnContext(ctx, "The server returned an identity shorter than "+ "the requested TTL, probably due to a `max_session_ttl` "+ "configured on a server-side role. It may not remain valid as "+