mirror of
https://github.com/gravitational/teleport.git
synced 2026-10-24 12:49:51 +08:00
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.
This commit is contained in:
committed by
Andrew Lytvynov
parent
88326d4e98
commit
96f56f3f40
@@ -798,6 +798,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)
|
||||
|
||||
@@ -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.
|
||||
@@ -307,6 +309,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)
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
@@ -478,6 +479,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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
@@ -555,6 +556,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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user