use rsa-sha2-256 by default for RSA SSH signatures

lib/auth/keystore/aws_kms.go

@@ -117,7 +117,7 @@ func (a *awsKMSKeystore) keyTypeDescription() string {
 
 // 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 (a *awsKMSKeystore) generateKey(ctx context.Context, algorithm cryptosuites.Algorithm, opts ...rsaKeyOption) ([]byte, crypto.Signer, error) {
+func (a *awsKMSKeystore) generateKey(ctx context.Context, algorithm cryptosuites.Algorithm) ([]byte, crypto.Signer, error) {
 	alg, err := awsAlgorithm(algorithm)
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
@@ -269,7 +269,14 @@ func (a *awsKMSSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpt
 			return nil, trace.BadParameter("unsupported hash func %q for AWS KMS key type %T", opts.HashFunc(), a.pub)
 		}
 	case crypto.SHA512:
-		signingAlg = kms.SigningAlgorithmSpecRsassaPkcs1V15Sha512
+		switch a.pub.(type) {
+		case *rsa.PublicKey:
+			signingAlg = kms.SigningAlgorithmSpecRsassaPkcs1V15Sha512
+		case *ecdsa.PublicKey:
+			signingAlg = kms.SigningAlgorithmSpecEcdsaSha512
+		default:
+			return nil, trace.BadParameter("unsupported hash func %q for AWS KMS key type %T", opts.HashFunc(), a.pub)
+		}
 	default:
 		return nil, trace.BadParameter("unsupported hash func %q for AWS KMS key", opts.HashFunc())
 	}

lib/auth/keystore/aws_kms_test.go

@@ -264,7 +264,7 @@ func (f *fakeAWSKMSService) CreateKey(input *kms.CreateKeyInput) (*kms.CreateKey
 	var privKeyPEM []byte
 	switch aws.StringValue(input.KeySpec) {
 	case kms.KeySpecRsa2048:
-		privKeyPEM = testRSAPrivateKeyPEM
+		privKeyPEM = testRSA2048PrivateKeyPEM
 	case kms.KeySpecEccNistP256:
 		signer, err := cryptosuites.GenerateKeyWithAlgorithm(cryptosuites.ECDSAP256)
 		if err != nil {

lib/auth/keystore/gcp_kms.go

@@ -113,8 +113,8 @@ func (g *gcpKMSKeyStore) keyTypeDescription() string {
 // generateKey creates a new private key and returns its identifier and a crypto.Signer. The returned
 // identifier for gcpKMSKeyStore encodes the full GCP KMS key version name, and can be passed to getSigner
 // later to get an equivalent crypto.Signer.
-func (g *gcpKMSKeyStore) generateKey(ctx context.Context, algorithm cryptosuites.Algorithm, opts ...rsaKeyOption) ([]byte, crypto.Signer, error) {
-	alg, err := gcpAlgorithm(algorithm, opts...)
+func (g *gcpKMSKeyStore) generateKey(ctx context.Context, algorithm cryptosuites.Algorithm) ([]byte, crypto.Signer, error) {
+	alg, err := gcpAlgorithm(algorithm)
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
 	}
@@ -152,22 +152,10 @@ func (g *gcpKMSKeyStore) generateKey(ctx context.Context, algorithm cryptosuites
 	return keyID.marshal(), signer, nil
 }
 
-func gcpAlgorithm(alg cryptosuites.Algorithm, opts ...rsaKeyOption) (kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm, error) {
-	rsaOpts := &rsaKeyOptions{}
-	for _, opt := range opts {
-		opt(rsaOpts)
-	}
-
+func gcpAlgorithm(alg cryptosuites.Algorithm) (kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm, error) {
 	switch alg {
 	case cryptosuites.RSA2048:
-		switch rsaOpts.digestAlgorithm {
-		case crypto.SHA256, 0:
-			return kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256, nil
-		case crypto.SHA512:
-			return kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_4096_SHA512, nil
-		default:
-			return kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_ALGORITHM_UNSPECIFIED, trace.BadParameter("unsupported digest algorithm: %v", rsaOpts.digestAlgorithm)
-		}
+		return kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256, nil
 	case cryptosuites.ECDSAP256:
 		return kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, nil
 	}
@@ -360,23 +348,34 @@ func (s *kmsSigner) Public() crypto.PublicKey {
 }
 
 func (s *kmsSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) {
-	requestDigest := &kmspb.Digest{}
+	var (
+		requestDigest *kmspb.Digest
+		data          []byte
+	)
 	switch opts.HashFunc() {
 	case crypto.SHA256:
-		requestDigest.Digest = &kmspb.Digest_Sha256{
-			Sha256: digest,
+		requestDigest = &kmspb.Digest{
+			Digest: &kmspb.Digest_Sha256{
+				Sha256: digest,
+			},
 		}
 	case crypto.SHA512:
-		requestDigest.Digest = &kmspb.Digest_Sha512{
-			Sha512: digest,
+		requestDigest = &kmspb.Digest{
+			Digest: &kmspb.Digest_Sha512{
+				Sha512: digest,
+			},
 		}
+	case crypto.Hash(0):
+		// Ed25519 uses no hash and sends the full raw data.
+		data = digest
 	default:
 		return nil, trace.BadParameter("unsupported hash func for GCP KMS signer: %v", opts.HashFunc())
 	}
 
 	resp, err := doGCPRequest(s.ctx, s.g, s.g.kmsClient.AsymmetricSign, &kmspb.AsymmetricSignRequest{
 		Name:   s.keyID.keyVersionName,
 		Digest: requestDigest,
+		Data:   data,
 	})
 	if err != nil {
 		return nil, trace.Wrap(err, "error while attempting GCP KMS signing operation")

lib/auth/keystore/gcp_kms_test.go

@@ -111,7 +111,7 @@ func (f *fakeGCPKMSServer) CreateCryptoKey(ctx context.Context, req *kmspb.Creat
 	var pem []byte
 	switch cryptoKey.VersionTemplate.Algorithm {
 	case kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256, kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_4096_SHA512:
-		pem = testRSAPrivateKeyPEM
+		pem = testRSA2048PrivateKeyPEM
 	case kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256:
 		signer, err := cryptosuites.GenerateKeyWithAlgorithm(cryptosuites.ECDSAP256)
 		if err != nil {
@@ -499,7 +499,7 @@ func TestGCPKMSKeystore(t *testing.T) {
 			require.NoError(t, err, "unexpected error creating CA")
 
 			// Client private key that will be the basis of test certs to be signed.
-			clientPrivKey, err := keys.ParsePrivateKey(testRSAPrivateKeyPEM)
+			clientPrivKey, err := keys.ParsePrivateKey(testRSA2048PrivateKeyPEM)
 			require.NoError(t, err)
 
 			// Test signing an SSH certificate.

lib/auth/keystore/keystore_test.go

@@ -53,7 +53,7 @@ const (
 )
 
 var (
-	testRSAPrivateKeyPEM = []byte(`-----BEGIN RSA PRIVATE KEY-----
+	testRSA2048PrivateKeyPEM = []byte(`-----BEGIN RSA PRIVATE KEY-----
 MIIEowIBAAKCAQEAqiD2rRJ5kq7hP55eOCM9DtdkWPMI8PBKgxaAiQ9J9YF3aNur
 98b8kACcTQ8ixSkHsLccVqRdt/Cnb7jtBSrwxJ9BN09fZEiyCvy7lwxNGBMQEaov
 9UU722nvuWKb+EkHzcVV9ie9i8wM88xpzzYO8eda8FZjHxaaoe2lkrHiiOFQRubJ
@@ -116,17 +116,17 @@ JhuTMEqUaAOZBoQLn+txjl3nu9WwTThJzlY0L4w=
 
 	testRawSSHKeyPair = &types.SSHKeyPair{
 		PublicKey:      testRSASSHPublicKey,
-		PrivateKey:     testRSAPrivateKeyPEM,
+		PrivateKey:     testRSA2048PrivateKeyPEM,
 		PrivateKeyType: types.PrivateKeyType_RAW,
 	}
 	testRawTLSKeyPair = &types.TLSKeyPair{
 		Cert:    testRSACert,
-		Key:     testRSAPrivateKeyPEM,
+		Key:     testRSA2048PrivateKeyPEM,
 		KeyType: types.PrivateKeyType_RAW,
 	}
 	testRawJWTKeyPair = &types.JWTKeyPair{
 		PublicKey:      testRSAPublicKeyPEM,
-		PrivateKey:     testRSAPrivateKeyPEM,
+		PrivateKey:     testRSA2048PrivateKeyPEM,
 		PrivateKeyType: types.PrivateKeyType_RAW,
 	}
 
@@ -271,6 +271,11 @@ func TestManager(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
 
+	sshSubjectKey, _, err := ed25519.GenerateKey(rand.Reader)
+	require.NoError(t, err)
+	sshSubjectPubKey, err := ssh.NewPublicKey(sshSubjectKey)
+	require.NoError(t, err)
+
 	pack := newTestPack(ctx, t)
 
 	for _, backendDesc := range pack.backends {
@@ -335,6 +340,20 @@ func TestManager(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, jwtKeyPair.PublicKey, pubkeyPem)
 
+			// Try signing an SSH cert.
+			sshCert := ssh.Certificate{
+				Key:         sshSubjectPubKey,
+				ValidBefore: uint64(time.Now().Add(time.Hour).Unix()),
+			}
+			require.NoError(t, sshCert.SignCert(rand.Reader, sshSigner))
+			// Verify the signature.
+			checker := ssh.CertChecker{
+				IsUserAuthority: func(pub ssh.PublicKey) bool {
+					return pub == sshSigner.PublicKey()
+				},
+			}
+			require.NoError(t, checker.CheckCert("root", &sshCert))
+
 			// Test what happens when the CA has only raw keys, which will be the
 			// initial state when migrating from software to a HSM/KMS backend.
 			ca, err = types.NewCertAuthority(types.CertAuthoritySpecV2{
@@ -544,7 +563,7 @@ func newTestPack(ctx context.Context, t *testing.T) *testPack {
 		config:              servicecfg.KeystoreConfig{},
 		opts:                &baseOpts,
 		backend:             softwareBackend,
-		unusedRawKey:        testRSAPrivateKeyPEM,
+		unusedRawKey:        testRSA2048PrivateKeyPEM,
 		deletionDoesNothing: true,
 	})
 

lib/auth/keystore/manager.go

@@ -22,6 +22,10 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/elliptic"
+	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
@@ -105,26 +109,12 @@ type Manager struct {
 	authPrefGetter cryptosuites.AuthPreferenceGetter
 }
 
-// rsaKeyOptions configure options for RSA key generation.
-type rsaKeyOptions struct {
-	digestAlgorithm crypto.Hash
-}
-
-// rsaKeyOption is a functional option for RSA key generation.
-type rsaKeyOption func(*rsaKeyOptions)
-
-func withRSADigestAlgorithm(alg crypto.Hash) rsaKeyOption {
-	return func(opts *rsaKeyOptions) {
-		opts.digestAlgorithm = alg
-	}
-}
-
 // backend is an interface that holds private keys and provides signing
 // operations.
 type backend interface {
 	// generateRSA creates a new key pair and returns its identifier and a crypto.Signer. The returned
 	// identifier can be passed to getSigner later to get an equivalent crypto.Signer.
-	generateKey(context.Context, cryptosuites.Algorithm, ...rsaKeyOption) (keyID []byte, signer crypto.Signer, err error)
+	generateKey(context.Context, cryptosuites.Algorithm) (keyID []byte, signer crypto.Signer, err error)
 
 	// getSigner returns a crypto.Signer for the given key identifier, if it is found.
 	// The public key is passed as well so that it does not need to be fetched
@@ -290,14 +280,7 @@ func (m *Manager) getSSHSigner(ctx context.Context, keySet types.CAKeySet) (ssh.
 				return nil, trace.Wrap(err)
 			}
 			signer = &cryptoCountSigner{Signer: signer, keyType: keyTypeSSH, store: backend.name()}
-			sshSigner, err := ssh.NewSignerFromSigner(signer)
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			if sshSigner.PublicKey().Type() == ssh.KeyAlgoRSA {
-				// SHA-512 to match NewSSHKeyPair.
-				sshSigner = toRSASHA512Signer(sshSigner)
-			}
+			sshSigner, err := sshSignerFromCryptoSigner(signer)
 			return sshSigner, trace.Wrap(err)
 		}
 	}
@@ -316,22 +299,74 @@ func publicKeyFromSSHAuthorizedKey(sshAuthorizedKey []byte) (crypto.PublicKey, e
 	return cryptoPublicKey.CryptoPublicKey(), nil
 }
 
-// toRSASHA512Signer forces an ssh.MultiAlgorithmSigner into using
-// "rsa-sha2-sha512" (instead of its SHA256 default).
-func toRSASHA512Signer(signer ssh.Signer) ssh.Signer {
-	ss, ok := signer.(ssh.MultiAlgorithmSigner)
+func sshSignerFromCryptoSigner(cryptoSigner crypto.Signer) (ssh.Signer, error) {
+	sshSigner, err := ssh.NewSignerFromSigner(cryptoSigner)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	// [ssh.NewSignerFromSigner] currently always returns an [ssh.AlgorithmSigner].
+	algorithmSigner, ok := sshSigner.(ssh.AlgorithmSigner)
 	if !ok {
-		return signer
+		return nil, trace.BadParameter("SSH CA: unsupported key type: %s", sshSigner.PublicKey().Type())
+	}
+	// Note: we don't actually create keys with all the algorithms supported
+	// below, but customers have been known to import their own existing keys.
+	switch pub := cryptoSigner.Public().(type) {
+	case *rsa.PublicKey:
+		// The current default hash used in ssh.(*Certificate).SignCert for an
+		// RSA signer created via ssh.NewSignerFromSigner is always SHA256,
+		// irrespective of the key size.
+		// This was a change in golang.org/x/crypto 0.14.0, prior to that the
+		// default was always SHA512.
+		//
+		// Due to the historical SHA512 default that existed at a time when
+		// hash algorithm selection was much more difficult, there are many
+		// existing GCP KMS keys that were created as 4096-bit keys using a
+		// SHA512 hash. GCP KMS is very particular about RSA hash algorithms:
+		// - 2048-bit or 3072-bit keys *must* use SHA256
+		// - 4096-bit keys *must* use SHA256 or SHA512
+		// - the hash length must be set *when the key is created* and can't be
+		//   changed.
+		//
+		// The chosen signature algorithms below are necessary to support
+		// existing GCP KMS keys, but they are also reasonable defaults for keys
+		// outside of GCP KMS.
+		//
+		// [rsa.PublicKey.Size()] returns 256 for a 2048-bit key; more generally
+		// it always returns the bit length divided by 8.
+		keySize := pub.Size()
+		switch {
+		case keySize < 256:
+			return nil, trace.BadParameter("SSH CA: RSA key size (%d) is too small", keySize)
+		case keySize < 512:
+			// This case matches 2048 and 3072 bit GCP KMS keys which *must* use SHA256.
+			return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoRSASHA256})
+		default:
+			// This case matches existing 4096 bit GCP KMS keys which *must* use SHA512
+			return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoRSASHA512})
+		}
+	case *ecdsa.PublicKey:
+		// These are all the current defaults, but let's set them explicitly so
+		// golang.org/x/crypto/ssh can't change them in an update and break some
+		// HSM or KMS that wouldn't support the new default.
+		switch pub.Curve {
+		case elliptic.P256():
+			return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoECDSA256})
+		case elliptic.P384():
+			return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoECDSA384})
+		case elliptic.P521():
+			return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoECDSA521})
+		default:
+			return nil, trace.BadParameter("SSH CA: ECDSA curve: %s", pub.Curve.Params().Name)
+		}
+	case ed25519.PublicKey:
+		// This is the current default, but let's set it explicitly so
+		// golang.org/x/crypto/ssh can't change it in an update and break some
+		// HSM or KMS that wouldn't support the new default.
+		return ssh.NewSignerWithAlgorithms(algorithmSigner, []string{ssh.KeyAlgoED25519})
+	default:
+		return nil, trace.BadParameter("SSH CA: unsupported key type: %s", sshSigner.PublicKey().Type())
 	}
-	return rsaSHA512Signer{MultiAlgorithmSigner: ss}
-}
-
-type rsaSHA512Signer struct {
-	ssh.MultiAlgorithmSigner
-}
-
-func (s rsaSHA512Signer) Algorithms() []string {
-	return []string{ssh.KeyAlgoRSASHA512}
 }
 
 // GetTLSCertAndSigner selects a usable TLS keypair from the given CA
@@ -433,18 +468,16 @@ func (m *Manager) NewSSHKeyPair(ctx context.Context, purpose cryptosuites.KeyPur
 }
 
 func (m *Manager) newSSHKeyPair(ctx context.Context, alg cryptosuites.Algorithm) (*types.SSHKeyPair, error) {
-	// The default hash length for SSH signers is 512 bits.
-	sshKey, cryptoSigner, err := m.backendForNewKeys.generateKey(ctx, alg, withRSADigestAlgorithm(crypto.SHA512))
+	sshKey, signer, err := m.backendForNewKeys.generateKey(ctx, alg)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	sshSigner, err := ssh.NewSignerFromSigner(cryptoSigner)
+	sshPub, err := ssh.NewPublicKey(signer.Public())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	publicKey := ssh.MarshalAuthorizedKey(sshSigner.PublicKey())
 	return &types.SSHKeyPair{
-		PublicKey:      publicKey,
+		PublicKey:      ssh.MarshalAuthorizedKey(sshPub),
 		PrivateKey:     sshKey,
 		PrivateKeyType: keyType(sshKey),
 	}, nil

lib/auth/keystore/pkcs11.go

@@ -123,7 +123,7 @@ 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, _ ...rsaKeyOption) ([]byte, crypto.Signer, error) {
+func (p *pkcs11KeyStore) generateKey(ctx context.Context, alg cryptosuites.Algorithm) ([]byte, crypto.Signer, error) {
 	// the key identifiers are not created in a thread safe
 	// manner so all calls are serialized to prevent races.
 	p.semaphore <- struct{}{}

lib/auth/keystore/software.go

@@ -59,7 +59,7 @@ func (s *softwareKeyStore) keyTypeDescription() string {
 // generateRSA creates a new private key and returns its identifier and a crypto.Signer. The returned
 // identifier for softwareKeyStore is a pem-encoded private key, and can be passed to getSigner later to get
 // an equivalent crypto.Signer.
-func (s *softwareKeyStore) generateKey(ctx context.Context, alg cryptosuites.Algorithm, _ ...rsaKeyOption) ([]byte, crypto.Signer, error) {
+func (s *softwareKeyStore) generateKey(ctx context.Context, alg cryptosuites.Algorithm) ([]byte, crypto.Signer, error) {
 	if alg == cryptosuites.RSA2048 && s.rsaKeyPairSource != nil {
 		privateKeyPEM, _, err := s.rsaKeyPairSource()
 		if err != nil {