From 91f1411bdfd0ce77b830542a673a55a7faf2adc0 Mon Sep 17 00:00:00 2001
From: Nic Klaassen <nic@goteleport.com>
Date: Tue, 12 Nov 2024 13:55:42 -0800
Subject: [PATCH] fix: fall back to RSA if HSM fails to generate ECDSA key in
 legacy suite (#48830)

---
 lib/auth/keystore/manager.go | 26 +++++++++++++++++++++++++-
 lib/auth/keystore/pkcs11.go  | 12 ++++++++----
 lib/cryptosuites/suites.go   |  4 +++-
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/lib/auth/keystore/manager.go b/lib/auth/keystore/manager.go
index 364685511ca44..3c8fcfc16498b 100644
--- a/lib/auth/keystore/manager.go
+++ b/lib/auth/keystore/manager.go
@@ -107,6 +107,7 @@ type Manager struct {
 	usableSigningBackends []backend
 
 	currentSuiteGetter cryptosuites.GetSuiteFunc
+	logger             *slog.Logger
 }
 
 // backend is an interface that holds private keys and provides signing
@@ -227,6 +228,7 @@ func NewManager(ctx context.Context, cfg *servicecfg.KeystoreConfig, opts *Optio
 		backendForNewKeys:     backendForNewKeys,
 		usableSigningBackends: usableSigningBackends,
 		currentSuiteGetter:    cryptosuites.GetCurrentSuiteFromAuthPreference(opts.AuthPreferenceGetter),
+		logger:                opts.Logger,
 	}, nil
 }
 
@@ -529,7 +531,29 @@ func (m *Manager) NewJWTKeyPair(ctx context.Context, purpose cryptosuites.KeyPur
 	key, err := m.newJWTKeyPair(ctx, alg)
 	if err != nil {
 		createErrorCounter.WithLabelValues(keyTypeJWT, m.backendForNewKeys.name(), alg.String()).Inc()
-		return nil, trace.Wrap(err)
+		if alg == cryptosuites.RSA2048 {
+			return nil, trace.Wrap(err)
+		}
+		// Try to fall back to RSA if using the legacy suite. The HSM/KMS
+		// credentials may not have permission to create ECDSA keys, especially
+		// if set up before ECDSA support was added.
+		origErr := trace.Wrap(err, "generating %s key in %s", alg.String(), m.backendForNewKeys.name())
+		m.logger.WarnContext(ctx, "Failed to generate key with default algorithm, falling back to RSA.", "error", origErr)
+		currentSuite, suiteErr := m.currentSuiteGetter(ctx)
+		if suiteErr != nil {
+			return nil, trace.NewAggregate(origErr, trace.Wrap(suiteErr, "finding current algorithm suite"))
+		}
+		switch currentSuite {
+		case types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_UNSPECIFIED, types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_LEGACY:
+		default:
+			// Not using the legacy suite, ECDSA key gen really should have
+			// worked, return the original error.
+			return nil, origErr
+		}
+		var rsaErr error
+		if key, rsaErr = m.newJWTKeyPair(ctx, cryptosuites.RSA2048); rsaErr != nil {
+			return nil, trace.NewAggregate(origErr, trace.Wrap(rsaErr, "attempting fallback to RSA key"))
+		}
 	}
 	return key, nil
 }
diff --git a/lib/auth/keystore/pkcs11.go b/lib/auth/keystore/pkcs11.go
index 7cc8597138864..73c224148f707 100644
--- a/lib/auth/keystore/pkcs11.go
+++ b/lib/auth/keystore/pkcs11.go
@@ -87,7 +87,7 @@ func (p *pkcs11KeyStore) keyTypeDescription() string {
 	return fmt.Sprintf("PKCS#11 HSM keys created by %s", p.hostUUID)
 }
 
-func (p *pkcs11KeyStore) findUnusedID() (keyID, error) {
+func (p *pkcs11KeyStore) findUnusedID(ctx context.Context) (keyID, error) {
 	if !p.isYubiHSM {
 		id, err := uuid.NewRandom()
 		if err != nil {
@@ -124,14 +124,18 @@ func (p *pkcs11KeyStore) findUnusedID() (keyID, error) {
 // generateKey creates a new private key and returns its identifier and a crypto.Signer. The returned
 // identifier can be passed to getSigner later to get an equivalent crypto.Signer.
 func (p *pkcs11KeyStore) generateKey(ctx context.Context, alg cryptosuites.Algorithm) ([]byte, crypto.Signer, error) {
-	// the key identifiers are not created in a thread safe
+	// The key identifiers are not created in a thread safe
 	// manner so all calls are serialized to prevent races.
-	p.semaphore <- struct{}{}
+	select {
+	case p.semaphore <- struct{}{}:
+	case <-ctx.Done():
+		return nil, nil, trace.Wrap(ctx.Err())
+	}
 	defer func() {
 		<-p.semaphore
 	}()
 
-	id, err := p.findUnusedID()
+	id, err := p.findUnusedID(ctx)
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
 	}
diff --git a/lib/cryptosuites/suites.go b/lib/cryptosuites/suites.go
index d9f1ec8bf7d71..2449a589422ec 100644
--- a/lib/cryptosuites/suites.go
+++ b/lib/cryptosuites/suites.go
@@ -152,7 +152,9 @@ func (a Algorithm) String() string {
 type suite map[KeyPurpose]Algorithm
 
 var (
-	// legacy is the original algorithm suite, which exclusively uses RSA2048.
+	// legacy is the original algorithm suite, which exclusively uses RSA2048
+	// for features developed before ECDSA and Ed25519 support were added. New
+	// features should always use the new algorithms.
 	legacy = suite{
 		UserCATLS:               RSA2048,
 		UserCASSH:               RSA2048,