Skip to content

Commit

Permalink
fix: fall back to RSA if HSM fails to generate ECDSA key in legacy su…
Browse files Browse the repository at this point in the history
…ite (#48830)
  • Loading branch information
nklaassen authored Nov 12, 2024
1 parent 231208f commit 91f1411
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
26 changes: 25 additions & 1 deletion lib/auth/keystore/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 8 additions & 4 deletions lib/auth/keystore/pkcs11.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion lib/cryptosuites/suites.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 91f1411

Please sign in to comment.