Enforce SHA-512 for RSA SSH signatures

Motivation:

    x/crypto/ssh defaults to using SHA-1 for signatures:
    https://github.com/golang/crypto/blob/master/ssh/keys.go#L963-L982
    Because Teleport uses RSA for user, host and CA keys, we end up with
    SHA-1 by default.

    SHA-1 is now considered weak and OpenSSH plans to deprecate it:
    https://www.openssh.com/txt/release-8.3

Fix:

    Wrap all RSA `ssh.Signer`s and override `SignWithAlgorithm` to
    provide `SigAlgoRSASHA2512` if not otherwise specified. This will
    only affect new certs, existing certs will use `SigAlgoRSA` until
    rotated. For CA certs (e.g. exported with `tctl auth export`) users
    might need to manually rotate.

Limited local testing with openssh 8.2 client and
`-oHostKeyAlgorithms=-ssh-rsa` confirms that this works with a new
cluster and fails with an old one.

lib/auth/init.go

@@ -796,6 +796,7 @@ func ReadSSHIdentityFromKeyPair(keyBytes, certBytes []byte) (*Identity, error) {
 	if err != nil {
 		return nil, trace.BadParameter("failed to parse private key: %v", err)
 	}
+	signer = sshutils.CompatSigner(signer)
 	// this signer authenticates using certificate signed by the cert authority
 	// not only by the public key
 	certSigner, err := ssh.NewCertSigner(cert, signer)

lib/auth/native/native.go

@@ -30,6 +30,7 @@ import (
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/services"
+	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/wrappers"
 
@@ -193,6 +194,7 @@ func (k *Keygen) GenerateHostCert(c services.HostCertParams) ([]byte, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	signer = sshutils.CompatSigner(signer)
 
 	// Build a valid list of principals from the HostID and NodeName and then
 	// add in any additional principals passed in.
@@ -304,6 +306,7 @@ func (k *Keygen) GenerateUserCert(c services.UserCertParams) ([]byte, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	signer = sshutils.CompatSigner(signer)
 	if err := cert.SignCert(rand.Reader, signer); err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/auth/rotate.go

@@ -24,6 +24,7 @@ import (
 	"github.com/gravitational/teleport/lib/auth/native"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/services"
+	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/tlsca"
 
 	"github.com/gravitational/trace"
@@ -476,6 +477,7 @@ func startNewRotation(req rotationReq, ca services.CertAuthority) error {
 		if err != nil {
 			return trace.Wrap(err)
 		}
+		signer = sshutils.CompatSigner(signer)
 
 		sshPubPEM = ssh.MarshalAuthorizedKey(signer.PublicKey())
 		sshPrivPEM = req.privateKey

lib/client/interfaces.go

@@ -27,6 +27,7 @@ import (
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/auth/native"
+	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 
@@ -245,6 +246,7 @@ func (k *Key) AsAuthMethod() (ssh.AuthMethod, error) {
 	if signer, err = ssh.NewCertSigner(keys[0].Certificate, signer); err != nil {
 		return nil, trace.Wrap(err)
 	}
+	signer = sshutils.CompatSigner(signer)
 	return NewAuthMethodForCert(signer), nil
 }
 

lib/reversetunnel/cache.go

@@ -27,6 +27,7 @@ import (
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/sshca"
+	"github.com/gravitational/teleport/lib/sshutils"
 
 	"github.com/gravitational/trace"
 	"github.com/gravitational/ttlmap"
@@ -154,6 +155,7 @@ func (c *certificateCache) generateHostCert(principals []string) (ssh.Signer, er
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	privateKey = sshutils.CompatSigner(privateKey)
 	publicKey, _, _, _, err := ssh.ParseAuthorizedKey(certBytes)
 	if err != nil {
 		return nil, err

lib/services/authority.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/wrappers"
@@ -553,6 +554,7 @@ func (ca *CertAuthorityV2) Signers() ([]ssh.Signer, error) {
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
+		signer = sshutils.CompatSigner(signer)
 		out = append(out, signer)
 	}
 	return out, nil

lib/sshutils/fingerprint.go

@@ -27,5 +27,6 @@ func PrivateKeyFingerprint(keyBytes []byte) (string, error) {
 	if err != nil {
 		return "", trace.Wrap(err)
 	}
+	signer = CompatSigner(signer)
 	return Fingerprint(signer.PublicKey()), nil
 }

lib/sshutils/signer.go

@@ -18,6 +18,7 @@ package sshutils
 
 import (
 	"crypto"
+	"io"
 
 	"golang.org/x/crypto/ssh"
 
@@ -32,6 +33,7 @@ func NewSigner(keyBytes, certBytes []byte) (ssh.Signer, error) {
 	if err != nil {
 		return nil, trace.Wrap(err, "failed to parse SSH private key")
 	}
+	keySigner = CompatSigner(keySigner)
 
 	pubkey, _, _, _, err := ssh.ParseAuthorizedKey(certBytes)
 	if err != nil {
@@ -59,3 +61,47 @@ func CryptoPublicKey(publicKey []byte) (crypto.PublicKey, error) {
 	}
 	return cryptoPubKey.CryptoPublicKey(), nil
 }
+
+// CompatSigner wraps a provided ssh.Signer to ensure algorithm compatibility
+// with OpenSSH.
+//
+// Right now it means forcing SHA-2 signatures with RSA keys, instead of the
+// default SHA-1 used by x/crypto/ssh.
+// See https://www.openssh.com/txt/release-8.2 for context.
+// This will be obsolete once https://github.com/golang/go/issues/37278 is
+// fixed upstream.
+//
+// If provided Signer is not an RSA key or does not implement
+// ssh.AlgorithmSigner, it's returned as is.
+//
+// DELETE IN 5.0: assuming https://github.com/golang/go/issues/37278 is fixed
+// by then and we pull in the fix. Also delete all call sites.
+func CompatSigner(s ssh.Signer) ssh.Signer {
+	if s.PublicKey().Type() != ssh.KeyAlgoRSA {
+		return s
+	}
+	as, ok := s.(ssh.AlgorithmSigner)
+	if !ok {
+		return s
+	}
+	return fixedAlgorithmSigner{
+		AlgorithmSigner: as,
+		alg:             ssh.SigAlgoRSASHA2512,
+	}
+}
+
+type fixedAlgorithmSigner struct {
+	ssh.AlgorithmSigner
+	alg string
+}
+
+func (s fixedAlgorithmSigner) SignWithAlgorithm(rand io.Reader, data []byte, alg string) (*ssh.Signature, error) {
+	if alg == "" {
+		alg = s.alg
+	}
+	return s.AlgorithmSigner.SignWithAlgorithm(rand, data, alg)
+}
+
+func (s fixedAlgorithmSigner) Sign(rand io.Reader, data []byte) (*ssh.Signature, error) {
+	return s.AlgorithmSigner.SignWithAlgorithm(rand, data, s.alg)
+}

lib/sshutils/signer_test.go

@@ -0,0 +1,74 @@
+package sshutils
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/rsa"
+	"io"
+
+	"golang.org/x/crypto/ssh"
+	"gopkg.in/check.v1"
+)
+
+type CompatSignerSuite struct{}
+
+var _ = check.Suite(&CompatSignerSuite{})
+
+func (s *CompatSignerSuite) TestCompatSignerNoop(c *check.C) {
+	ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	c.Assert(err, check.IsNil)
+	ecdsaSigner, err := ssh.NewSignerFromSigner(ecdsaKey)
+	c.Assert(err, check.IsNil)
+
+	// ECDSA key should be returned as-is, not wrapped.
+	c.Assert(CompatSigner(ecdsaSigner), check.Equals, ecdsaSigner)
+}
+
+func (s *CompatSignerSuite) TestCompatSigner(c *check.C) {
+	rsaSigner, err := newMockRSASigner()
+	c.Assert(err, check.IsNil)
+
+	wrapped := CompatSigner(rsaSigner)
+	// RSA key should get wrapped.
+	c.Assert(wrapped, check.Not(check.Equals), rsaSigner)
+	wrappedAS := wrapped.(ssh.AlgorithmSigner)
+
+	// Simple Sign call should use the enforced SHA-2 algorithm.
+	wrapped.Sign(nil, nil)
+	c.Assert(rsaSigner.lastAlg, check.Equals, ssh.SigAlgoRSASHA2512)
+	rsaSigner.lastAlg = ""
+
+	// SignWithAlgorithm without specifying an algorithm should use the
+	// enforced SHA-2 algorithm.
+	wrappedAS.SignWithAlgorithm(nil, nil, "")
+	c.Assert(rsaSigner.lastAlg, check.Equals, ssh.SigAlgoRSASHA2512)
+	rsaSigner.lastAlg = ""
+
+	// SignWithAlgorithm *with* an algorithm should respect the provided
+	// algorithm.
+	wrappedAS.SignWithAlgorithm(nil, nil, "foo")
+	c.Assert(rsaSigner.lastAlg, check.Equals, "foo")
+}
+
+type mockRSASigner struct {
+	ssh.Signer
+	lastAlg string
+}
+
+func newMockRSASigner() (*mockRSASigner, error) {
+	rsaKey, err := rsa.GenerateKey(rand.Reader, 2056)
+	if err != nil {
+		return nil, err
+	}
+	rsaSigner, err := ssh.NewSignerFromSigner(rsaKey)
+	if err != nil {
+		return nil, err
+	}
+	return &mockRSASigner{Signer: rsaSigner}, nil
+}
+
+func (s *mockRSASigner) SignWithAlgorithm(rand io.Reader, data []byte, alg string) (*ssh.Signature, error) {
+	s.lastAlg = alg
+	return nil, nil
+}

tool/tsh/common/identity.go

@@ -72,6 +72,7 @@ func LoadIdentity(idFn string) (*client.Key, ssh.HostKeyCallback, error) {
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
 	}
+	signer = sshutils.CompatSigner(signer)
 	// validate TLS Cert (if present):
 	if len(ident.Certs.TLS) > 0 {
 		_, err := tls.X509KeyPair(ident.Certs.TLS, ident.PrivateKey)