diff --git a/api/utils/sshutils/checker.go b/api/utils/sshutils/checker.go index 68543405523..ba63ddb345a 100644 --- a/api/utils/sshutils/checker.go +++ b/api/utils/sshutils/checker.go @@ -17,6 +17,8 @@ limitations under the License. package sshutils import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rsa" "net" @@ -123,12 +125,21 @@ func validateFIPSAlgorithm(key ssh.PublicKey) error { if !ok { return trace.BadParameter("unable to determine underlying public key") } - k, ok := cryptoKey.CryptoPublicKey().(*rsa.PublicKey) - if !ok { - return trace.BadParameter("only RSA keys supported") - } - if k.N.BitLen() != constants.RSAKeySize { - return trace.BadParameter("found %v-bit key, only %v-bit supported", k.N.BitLen(), constants.RSAKeySize) + switch k := cryptoKey.CryptoPublicKey().(type) { + case *rsa.PublicKey: + if k.N.BitLen() != constants.RSAKeySize { + return trace.BadParameter("found %v-bit RSA key, only %v-bit supported", k.N.BitLen(), constants.RSAKeySize) + } + case *ecdsa.PublicKey: + if k.Curve != elliptic.P256() && k.Curve != elliptic.P384() { + params := k.Curve.Params() + if params == nil { + return trace.BadParameter("unable to determine curve of ECDSA public key") + } + return trace.BadParameter("found ECDSA key with curve %s, only P-256 and P-384 are supported", params.Name) + } + default: + return trace.BadParameter("only RSA and ECDSA keys supported") } return nil } diff --git a/api/utils/sshutils/checker_test.go b/api/utils/sshutils/checker_test.go index 53db4cfe328..f32470e8125 100644 --- a/api/utils/sshutils/checker_test.go +++ b/api/utils/sshutils/checker_test.go @@ -17,55 +17,26 @@ limitations under the License. package sshutils import ( + "crypto" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/rand" "crypto/rsa" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" "github.com/gravitational/teleport/api/constants" ) -// TestCheckerValidate checks what algorithm are supported in regular (non-FIPS) mode. -func TestCheckerValidate(t *testing.T) { - checker := CertChecker{} - - rsaKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) - require.NoError(t, err) - smallRSAKey, err := rsa.GenerateKey(rand.Reader, 1024) - require.NoError(t, err) - ellipticKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - - // 2048-bit RSA keys are valid. - cryptoKey := rsaKey.Public() - sshKey, err := ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.NoError(t, err) - - // 1024-bit RSA keys are valid. - cryptoKey = smallRSAKey.Public() - sshKey, err = ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.NoError(t, err) - - // ECDSA keys are valid. - cryptoKey = ellipticKey.Public() - sshKey, err = ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.NoError(t, err) -} - // TestCheckerValidateFIPS makes sure the public key is a valid algorithm // that Teleport supports while in FIPS mode. func TestCheckerValidateFIPS(t *testing.T) { - checker := CertChecker{ + regularChecker := CertChecker{} + fipsChecker := CertChecker{ FIPS: true, } @@ -73,27 +44,68 @@ func TestCheckerValidateFIPS(t *testing.T) { require.NoError(t, err) smallRSAKey, err := rsa.GenerateKey(rand.Reader, 1024) require.NoError(t, err) - ellipticKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + ellipticKeyP224, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + require.NoError(t, err) + ellipticKeyP256, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + ellipticKeyP384, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + require.NoError(t, err) + _, ed25519Key, err := ed25519.GenerateKey(rand.Reader) require.NoError(t, err) - // 2048-bit RSA keys are valid. - cryptoKey := rsaKey.Public() - sshKey, err := ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.NoError(t, err) + for _, tc := range []struct { + desc string + key crypto.Signer + expectSSHError bool + expectFIPSError bool + }{ + { + desc: "RSA2048", + key: rsaKey, + }, + { + desc: "RSA1024", + key: smallRSAKey, + expectFIPSError: true, + }, + { + desc: "ECDSAP224", + key: ellipticKeyP224, + expectSSHError: true, + expectFIPSError: true, + }, + { + desc: "ECDSAP256", + key: ellipticKeyP256, + }, + { + desc: "ECDSAP384", + key: ellipticKeyP384, + }, + { + desc: "Ed25519", + key: ed25519Key, + expectFIPSError: true, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + cryptoKey := tc.key.Public() + sshKey, err := ssh.NewPublicKey(cryptoKey) + if tc.expectSSHError { + require.Error(t, err) + } else { + require.NoError(t, err) + } - // 1024-bit RSA keys are not valid. - cryptoKey = smallRSAKey.Public() - sshKey, err = ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.Error(t, err) + err = regularChecker.validateFIPS(sshKey) + assert.NoError(t, err) - // ECDSA keys are not valid. - cryptoKey = ellipticKey.Public() - sshKey, err = ssh.NewPublicKey(cryptoKey) - require.NoError(t, err) - err = checker.validateFIPS(sshKey) - require.Error(t, err) + err = fipsChecker.validateFIPS(sshKey) + if tc.expectFIPSError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } } diff --git a/lib/sshutils/server_test.go b/lib/sshutils/server_test.go index 304f4bc4215..704164f3d00 100644 --- a/lib/sshutils/server_test.go +++ b/lib/sshutils/server_test.go @@ -206,6 +206,9 @@ func TestHostSignerFIPS(t *testing.T) { _, ellipticSigner, err := cert.CreateEllipticCertificate("foo", ssh.HostCert) require.NoError(t, err) + _, ed25519Signer, err := cert.CreateEd25519Certificate("foo", ssh.HostCert) + require.NoError(t, err) + fn := NewChanHandlerFunc(func(_ context.Context, _ *ConnectionContext, nch ssh.NewChannel) { err := nch.Reject(ssh.Prohibited, "nothing to see here") assert.NoError(t, err) @@ -216,11 +219,17 @@ func TestHostSignerFIPS(t *testing.T) { inFIPS bool assert require.ErrorAssertionFunc }{ - // ECDSA when in FIPS mode should fail. + // Ed25519 when in FIPS mode should fail. + { + inSigner: ed25519Signer, + inFIPS: true, + assert: require.Error, + }, + // ECDSA when in FIPS mode is okay. { inSigner: ellipticSigner, inFIPS: true, - assert: require.Error, + assert: require.NoError, }, // RSA when in FIPS mode is okay. { @@ -228,6 +237,12 @@ func TestHostSignerFIPS(t *testing.T) { inFIPS: true, assert: require.NoError, }, + // Ed25519 when in not FIPS mode should succeed. + { + inSigner: ed25519Signer, + inFIPS: false, + assert: require.NoError, + }, // ECDSA when in not FIPS mode should succeed. { inSigner: ellipticSigner, diff --git a/lib/utils/cert/certs.go b/lib/utils/cert/certs.go index ddccee98a8f..cad4d986cb6 100644 --- a/lib/utils/cert/certs.go +++ b/lib/utils/cert/certs.go @@ -21,6 +21,7 @@ package cert import ( "crypto" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/rand" "time" @@ -51,9 +52,7 @@ func CreateCertificate(principal string, certType uint32) (*ssh.Certificate, ssh return cert, certSigner, nil } -// CreateEllipticCertificate creates a valid, but not supported, ECDSA -// SSH certificate. This certificate is used to make sure Teleport rejects -// such certificates. +// CreateEllipticCertificate creates a valid ECDSA P-256 certificate. func CreateEllipticCertificate(principal string, certType uint32) (*ssh.Certificate, ssh.Signer, error) { // Create ECDSA key for CA and certificate to be signed by CA. caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -73,6 +72,27 @@ func CreateEllipticCertificate(principal string, certType uint32) (*ssh.Certific return cert, certSigner, nil } +// CreateEd25519Certificate creates an Ed25519 certificate which should be +// rejected in FIPS mode. +func CreateEd25519Certificate(principal string, certType uint32) (*ssh.Certificate, ssh.Signer, error) { + // Create Ed25519 key for CA and certificate to be signed by CA. + _, caKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return nil, nil, trace.Wrap(err) + } + _, key, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + cert, certSigner, err := createCertificate(principal, certType, caKey, key) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + return cert, certSigner, nil +} + // createCertificate creates a SSH certificate for the given key signed by the // given CA key. This function exists here to allow easy key generation for // some of the more core packages like "sshutils".