diff --git a/lib/auth/auth.go b/lib/auth/auth.go index e705128b88fa2..bf8fb2c024c2d 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -6545,6 +6545,9 @@ func (a *Server) createAccessListReminderNotification(ctx context.Context, owner } // GenerateCertAuthorityCRL generates an empty CRL for the local CA of a given type. +// +// WARNING: This is not safe for use in clusters using HSMs or KMS for private key material. +// Instead, you should prefer using the CRLs that are already present in the certificate_authority resource. func (a *Server) GenerateCertAuthorityCRL(ctx context.Context, caType types.CertAuthType) ([]byte, error) { // Generate a CRL for the current cluster CA. clusterName, err := a.GetClusterName() @@ -6559,10 +6562,8 @@ func (a *Server) GenerateCertAuthorityCRL(ctx context.Context, caType types.Cert return nil, trace.Wrap(err) } - // TODO(awly): this will only create a CRL for an active signer. - // If there are multiple signers (multiple HSMs), we won't have the full CRL coverage. - // Generate a CRL per signer and return all of them separately. - + // Note: this will only create a CRL for a single active signer. + // If there are multiple signers (HSMs), we won't have the full CRL coverage. cert, signer, err := a.keyStore.GetTLSCertAndSigner(ctx, ca) if trace.IsNotFound(err) { // If there is no local TLS signer found in the host CA ActiveKeys, this diff --git a/lib/auth/desktop.go b/lib/auth/desktop.go index dacf756a7d649..1bc139ca96583 100644 --- a/lib/auth/desktop.go +++ b/lib/auth/desktop.go @@ -101,8 +101,8 @@ func (a *Server) GenerateWindowsDesktopCert(ctx context.Context, req *proto.Wind cdp := windows.CRLDistributionPoint(req.CRLDomain, types.UserCA, tlsCA, true) certReq.CRLDistributionPoints = []string{cdp} } else if req.CRLEndpoint != "" { - // legacy clients will specify CRL endpoint instead of CRL domain - // DELETE IN v21 (zmb3) + // Legacy clients will specify CRL endpoint instead of CRL domain. + // DELETE IN v20 (zmb3): v19 clients will no longer be setting CRLEndpoint certReq.CRLDistributionPoints = []string{req.CRLEndpoint} a.logger.DebugContext(ctx, "Generating Windows desktop cert with legacy CDP") } diff --git a/lib/auth/init.go b/lib/auth/init.go index 43a7abff2e77a..9df3ef441d87e 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -734,19 +734,37 @@ func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthI // Add [empty] CRLs to any issuers that are missing them. // These are valid for 10 years and regenerated on CA rotation. + // DELETE IN v20(probakowski, zmb3): by v20 all auths will have + // at least been on v19, and all versions of 19 backfill missing CRLs. updated := false for _, kp := range ca.GetActiveKeys().TLS { - if kp.CRL != nil { + cert, err := tlsca.ParseCertificatePEM(kp.Cert) + if err != nil { + asrv.logger.WarnContext(ctx, "Couldn't parse CA certificate", "ca_type", caID.Type, "error", err) continue } - certBytes, signer, err := asrv.keyStore.GetTLSCertAndSigner(ctx, ca) - if err != nil { - asrv.logger.WarnContext(ctx, "Couldn't get CA certificate", "ca_type", caID.Type, "error", err) + + needsNewCRL := len(kp.CRL) == 0 + if !needsNewCRL { + // An earlier version of this code generated CRLs that may have been signed with the wrong keypair. + // To fix this, we must also validate the signature of existing CRLs. + if crl, err := x509.ParseRevocationList(kp.CRL); err != nil { + needsNewCRL = true + } else if err := crl.CheckSignatureFrom(cert); err != nil { + asrv.logger.WarnContext(ctx, "Detected CRL with invalid signature, regenerating", "ca_type", caID.Type, "ca", ca.GetName(), "error", err) + needsNewCRL = true + } + } + + if !needsNewCRL { continue } - cert, err := tlsca.ParseCertificatePEM(certBytes) + + signer, err := asrv.keyStore.TLSSigner(ctx, kp) if err != nil { - asrv.logger.WarnContext(ctx, "Couldn't parse CA certificate", "ca_type", caID.Type, "error", err) + if !errors.Is(err, keystore.ErrUnusableKey) { + asrv.logger.WarnContext(ctx, "Couldn't get TLS signer for CA", "ca", ca.GetName(), "ca_type", caID.Type, "error", err) + } continue } diff --git a/lib/auth/keystore/manager.go b/lib/auth/keystore/manager.go index ce026c35ceb0c..73e5b535d1211 100644 --- a/lib/auth/keystore/manager.go +++ b/lib/auth/keystore/manager.go @@ -30,6 +30,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "errors" "io" "log/slog" "maps" @@ -381,6 +382,35 @@ func sshSignerFromCryptoSigner(cryptoSigner crypto.Signer) (ssh.Signer, error) { } } +var ErrUnusableKey = errors.New("unable to sign with requested key") + +// TLSSigner returns a crypto.Signer for the given TLSKeyPair. +// It returns ErrUnusableKey if unable to create a signer from the given keypair, +// e.g. if it is stored in an HSM or KMS this auth service is not configured to use. +func (m *Manager) TLSSigner(ctx context.Context, keypair *types.TLSKeyPair) (crypto.Signer, error) { + for _, backend := range m.usableSigningBackends { + canUse, err := backend.canSignWithKey(ctx, keypair.Key, keypair.KeyType) + if err != nil { + return nil, trace.Wrap(err) + } + if !canUse { + continue + } + pub, err := publicKeyFromTLSCertPem(keypair.Cert) + if err != nil { + return nil, trace.Wrap(err) + } + signer, err := backend.getSigner(ctx, keypair.Key, pub) + if err != nil { + return nil, trace.Wrap(err) + } + + return &cryptoCountSigner{Signer: signer, keyType: keyTypeTLS, store: backend.name()}, nil + } + + return nil, ErrUnusableKey +} + // GetTLSCertAndSigner selects a usable TLS keypair from the given CA // and returns the PEM-encoded TLS certificate and a [crypto.Signer]. func (m *Manager) GetTLSCertAndSigner(ctx context.Context, ca types.CertAuthority) ([]byte, crypto.Signer, error) { diff --git a/lib/auth/windows/certificate_authority.go b/lib/auth/windows/certificate_authority.go index 8d8dea099343b..a8b99ac30c2d5 100644 --- a/lib/auth/windows/certificate_authority.go +++ b/lib/auth/windows/certificate_authority.go @@ -101,9 +101,13 @@ func (c *CertificateStoreClient) Update(ctx context.Context) error { } } - // All authorities are missing CRL, let's fall back to legacy behavior - // TODO(probakowski): DELETE IN v21.0.0 + // All authorities are missing CRL, fall back to legacy behavior since some early v18 auth servers + // won't have set up CRLs yet. + // DELETE IN v19 (zmb3/probakowski): this is agent code, by the time agents are on v19 we won't + // need to worry about v18 auth servers. if !hasCRL { + c.cfg.Log.Warn("No existing certificate authorities had an associated CRL. " + + "If you are using HSM or KMS for private key material, please update your auth server.") crlDER, err := c.cfg.AccessPoint.GenerateCertAuthorityCRL(ctx, caType) if err != nil { return trace.Wrap(err, "generating CRL") diff --git a/tool/tctl/common/auth_command.go b/tool/tctl/common/auth_command.go index 500e1d9f575ff..0b50e6ed050d0 100644 --- a/tool/tctl/common/auth_command.go +++ b/tool/tctl/common/auth_command.go @@ -170,7 +170,7 @@ func (a *AuthCommand) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIF a.authLS = auth.Command("ls", "List connected auth servers.") a.authLS.Flag("format", "Output format: 'yaml', 'json' or 'text'").Default(teleport.YAML).StringVar(&a.format) - a.authCRL = auth.Command("crl", "Export empty certificate revocation list (CRL) for certificate authorities.") + a.authCRL = auth.Command("crl", "Export empty certificate revocation list (CRL) for Teleport certificate authorities.") a.authCRL.Flag("type", fmt.Sprintf("Certificate authority type, one of: %s", strings.Join(allowedCRLCertificateTypes, ", "))).Required().EnumVar(&a.caType, allowedCRLCertificateTypes...) a.authCRL.Flag("out", "If set, writes exported revocation lists to files with the given path prefix").StringVar(&a.output) } @@ -193,7 +193,7 @@ func (a *AuthCommand) TryRun(ctx context.Context, cmd string, clientFunc commonc case a.authLS.FullCommand(): commandFunc = a.ListAuthServers case a.authCRL.FullCommand(): - commandFunc = a.GenerateCRLForCA + commandFunc = a.ExportCRL default: return false, nil } @@ -482,9 +482,9 @@ func (a *AuthCommand) ListAuthServers(ctx context.Context, clusterAPI authComman return nil } -// GenerateCRLForCA generates a certificate revocation list for a certificate -// authority. -func (a *AuthCommand) GenerateCRLForCA(ctx context.Context, clusterAPI authCommandClient) error { +// ExportCRL exports an empty certificate revocation list for a +// Teleport certificate authority. +func (a *AuthCommand) ExportCRL(ctx context.Context, clusterAPI authCommandClient) error { certType := types.CertAuthType(a.caType) if err := certType.Check(); err != nil { return trace.Wrap(err) @@ -528,8 +528,16 @@ func (a *AuthCommand) GenerateCRLForCA(ctx context.Context, clusterAPI authComma var results []output for i, keypair := range tlsKeys { crl := keypair.CRL + // DELETE IN v19 (probakowski, zmb3): tctl v19 means the server is either v19 or v20, + // both of which are guaranteed to have CRLs already in place. if len(crl) == 0 { - fmt.Fprintf(os.Stderr, "keypair %v is missing CRL for %v authority %v, generating legacy fallback", i, authority.GetType(), authority.GetName()) + // WARNING: GenerateCertAuthorityCRL will find any suitable keypair for signing the CRL, + // it is not guaranteed to use _this_ particular keypair. + fmt.Fprintf(os.Stderr, "Keypair %v is missing CRL for %v authority %v, generating legacy fallback.", + i, authority.GetType(), authority.GetName()) + if len(tlsKeys) > 1 { + fmt.Fprintf(os.Stderr, "If you are using HSM or KMS for private key material, please update your auth server and re-export CRLs.") + } crl, err = clusterAPI.GenerateCertAuthorityCRL(ctx, certType) if err != nil { return trace.Wrap(err) diff --git a/tool/tctl/common/auth_command_test.go b/tool/tctl/common/auth_command_test.go index b9f534efe98d1..ac5b95cf05772 100644 --- a/tool/tctl/common/auth_command_test.go +++ b/tool/tctl/common/auth_command_test.go @@ -965,7 +965,7 @@ func TestGenerateAndSignKeys(t *testing.T) { } } -func TestGenerateCRLForCA(t *testing.T) { +func TestExportCRL(t *testing.T) { ctx := context.Background() name, err := types.NewClusterName(types.ClusterNameSpecV2{ClusterName: "name", ClusterID: "clusterID"}) require.NoError(t, err) @@ -987,12 +987,12 @@ func TestGenerateCRLForCA(t *testing.T) { for _, caType := range allowedCRLCertificateTypes { t.Run(caType, func(t *testing.T) { ac := AuthCommand{caType: caType} - require.NoError(t, ac.GenerateCRLForCA(ctx, authClient)) + require.NoError(t, ac.ExportCRL(ctx, authClient)) }) } t.Run("InvalidCAType", func(t *testing.T) { ac := AuthCommand{caType: "wrong-ca"} - require.Error(t, ac.GenerateCRLForCA(ctx, authClient)) + require.Error(t, ac.ExportCRL(ctx, authClient)) }) }