ssh: don't advertise rsa-sha2 algorithms if we can't use them

The server implementation looks at the HostKeys to advertise and
negotiate host key signature algorithms. A fundamental issue of the
Signer and AlgorithmSigner interfaces is that they don't expose the
supported signature algorithms, so really the server has to guess.

Currently, it would guess exclusively based on the PublicKey.Type,
regardless of whether the host key implemented AlgorithmSigner. This
means that a legacy Signer that only supports ssh-rsa still led the
server to negotiate rsa-sha2 algorithms. The server would then fail to
find a suitable host key to make the signature and crash.

This won't happen if only Signers from this package are used, but if a
custom Signer that doesn't support SignWithAlgorithm() but returns
"ssh-rsa" from PublicKey().Type() is used as a HostKey, the server is
vulnerable to DoS.

The only workable rules to determine what to advertise seems to be:

   1. a pure Signer will always Sign with the PublicKey.Type

   2. an AlgorithmSigner supports all algorithms associated with the
      PublicKey.Type

Rule number two means that we can't add new supported algorithms in the
future, which is not great, but it's too late to fix that.

rsaSigner was breaking rule number one, and although it would have been
fine where it's used, I didn't want to break our own interface contract.

It's unclear why we had separate test key entries for rsa-sha2
algorithms, since we can use the ssh-rsa key for those. The only test
that used them, TestCertTypes, seemed broken: the init was actually
failing at making the corresponding signers rsaSigners, and indeed the
test for the SHA-256 signer expected and checked a SHA-512 signature.

Pending CVE
For golang/go#49952

Change-Id: Ie658eefcadd87906e63fc7faae8249376aa96c79
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392355
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Filippo Valsorda
2022-03-14 23:46:59 +00:00
parent fcc990c556
commit 1baeb1ce4c
12 changed files with 234 additions and 233 deletions
+44 -84
View File
@@ -76,7 +76,7 @@ func parsePubKey(in []byte, algo string) (pubKey PublicKey, rest []byte, err err
case KeyAlgoSKED25519:
return parseSKEd25519(in)
case CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoSKECDSA256v01, CertAlgoED25519v01, CertAlgoSKED25519v01:
cert, err := parseCert(in, certToPrivAlgo(algo))
cert, err := parseCert(in, certKeyAlgoNames[algo])
if err != nil {
return nil, nil, err
}
@@ -295,18 +295,21 @@ func MarshalAuthorizedKey(key PublicKey) []byte {
return b.Bytes()
}
// PublicKey is an abstraction of different types of public keys.
// PublicKey represents a public key using an unspecified algorithm.
//
// Some PublicKeys provided by this package also implement CryptoPublicKey.
type PublicKey interface {
// Type returns the key's type, e.g. "ssh-rsa".
// Type returns the key format name, e.g. "ssh-rsa".
Type() string
// Marshal returns the serialized key data in SSH wire format,
// with the name prefix. To unmarshal the returned data, use
// the ParsePublicKey function.
// Marshal returns the serialized key data in SSH wire format, with the name
// prefix. To unmarshal the returned data, use the ParsePublicKey function.
Marshal() []byte
// Verify that sig is a signature on the given data using this
// key. This function will hash the data appropriately first.
// Verify that sig is a signature on the given data using this key. This
// method will hash the data appropriately first. sig.Format is allowed to
// be any signature algorithm compatible with the key type, the caller
// should check if it has more stringent requirements.
Verify(data []byte, sig *Signature) error
}
@@ -317,23 +320,32 @@ type CryptoPublicKey interface {
}
// A Signer can create signatures that verify against a public key.
//
// Some Signers provided by this package also implement AlgorithmSigner.
type Signer interface {
// PublicKey returns an associated PublicKey instance.
// PublicKey returns the associated PublicKey.
PublicKey() PublicKey
// Sign returns raw signature for the given data. This method
// will apply the hash specified for the keytype to the data.
// Sign returns a signature for the given data. This method will hash the
// data appropriately first. The signature algorithm is expected to match
// the key format returned by the PublicKey.Type method (and not to be any
// alternative algorithm supported by the key format).
Sign(rand io.Reader, data []byte) (*Signature, error)
}
// A AlgorithmSigner is a Signer that also supports specifying a specific
// algorithm to use for signing.
// An AlgorithmSigner is a Signer that also supports specifying an algorithm to
// use for signing.
//
// An AlgorithmSigner can't advertise the algorithms it supports, so it should
// be prepared to be invoked with every algorithm supported by the public key
// format.
type AlgorithmSigner interface {
Signer
// SignWithAlgorithm is like Signer.Sign, but allows specifying a desired
// signing algorithm. Callers may pass an empty string for the algorithm in
// which case the AlgorithmSigner will use a default algorithm.
// which case the AlgorithmSigner will use a default algorithm. This default
// doesn't currently control any behavior in this package.
SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error)
}
@@ -385,17 +397,11 @@ func (r *rsaPublicKey) Marshal() []byte {
}
func (r *rsaPublicKey) Verify(data []byte, sig *Signature) error {
var hash crypto.Hash
switch sig.Format {
case KeyAlgoRSA:
hash = crypto.SHA1
case KeyAlgoRSASHA256:
hash = crypto.SHA256
case KeyAlgoRSASHA512:
hash = crypto.SHA512
default:
supportedAlgos := algorithmsForKeyFormat(r.Type())
if !contains(supportedAlgos, sig.Format) {
return fmt.Errorf("ssh: signature type %s for key type %s", sig.Format, r.Type())
}
hash := hashFuncs[sig.Format]
h := hash.New()
h.Write(data)
digest := h.Sum(nil)
@@ -470,7 +476,7 @@ func (k *dsaPublicKey) Verify(data []byte, sig *Signature) error {
if sig.Format != k.Type() {
return fmt.Errorf("ssh: signature type %s for key type %s", sig.Format, k.Type())
}
h := crypto.SHA1.New()
h := hashFuncs[sig.Format].New()
h.Write(data)
digest := h.Sum(nil)
@@ -503,7 +509,7 @@ func (k *dsaPrivateKey) PublicKey() PublicKey {
}
func (k *dsaPrivateKey) Sign(rand io.Reader, data []byte) (*Signature, error) {
return k.SignWithAlgorithm(rand, data, "")
return k.SignWithAlgorithm(rand, data, k.PublicKey().Type())
}
func (k *dsaPrivateKey) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) {
@@ -511,7 +517,7 @@ func (k *dsaPrivateKey) SignWithAlgorithm(rand io.Reader, data []byte, algorithm
return nil, fmt.Errorf("ssh: unsupported signature algorithm %s", algorithm)
}
h := crypto.SHA1.New()
h := hashFuncs[k.PublicKey().Type()].New()
h.Write(data)
digest := h.Sum(nil)
r, s, err := dsa.Sign(rand, k.PrivateKey, digest)
@@ -607,19 +613,6 @@ func supportedEllipticCurve(curve elliptic.Curve) bool {
return curve == elliptic.P256() || curve == elliptic.P384() || curve == elliptic.P521()
}
// ecHash returns the hash to match the given elliptic curve, see RFC
// 5656, section 6.2.1
func ecHash(curve elliptic.Curve) crypto.Hash {
bitSize := curve.Params().BitSize
switch {
case bitSize <= 256:
return crypto.SHA256
case bitSize <= 384:
return crypto.SHA384
}
return crypto.SHA512
}
// parseECDSA parses an ECDSA key according to RFC 5656, section 3.1.
func parseECDSA(in []byte) (out PublicKey, rest []byte, err error) {
var w struct {
@@ -675,7 +668,7 @@ func (k *ecdsaPublicKey) Verify(data []byte, sig *Signature) error {
return fmt.Errorf("ssh: signature type %s for key type %s", sig.Format, k.Type())
}
h := ecHash(k.Curve).New()
h := hashFuncs[sig.Format].New()
h.Write(data)
digest := h.Sum(nil)
@@ -779,7 +772,7 @@ func (k *skECDSAPublicKey) Verify(data []byte, sig *Signature) error {
return fmt.Errorf("ssh: signature type %s for key type %s", sig.Format, k.Type())
}
h := ecHash(k.Curve).New()
h := hashFuncs[sig.Format].New()
h.Write([]byte(k.application))
appDigest := h.Sum(nil)
@@ -878,7 +871,7 @@ func (k *skEd25519PublicKey) Verify(data []byte, sig *Signature) error {
return fmt.Errorf("invalid size %d for Ed25519 public key", l)
}
h := sha256.New()
h := hashFuncs[sig.Format].New()
h.Write([]byte(k.application))
appDigest := h.Sum(nil)
@@ -943,15 +936,6 @@ func newDSAPrivateKey(key *dsa.PrivateKey) (Signer, error) {
return &dsaPrivateKey{key}, nil
}
type rsaSigner struct {
AlgorithmSigner
defaultAlgorithm string
}
func (s *rsaSigner) Sign(rand io.Reader, data []byte) (*Signature, error) {
return s.AlgorithmSigner.SignWithAlgorithm(rand, data, s.defaultAlgorithm)
}
type wrappedSigner struct {
signer crypto.Signer
pubKey PublicKey
@@ -974,44 +958,20 @@ func (s *wrappedSigner) PublicKey() PublicKey {
}
func (s *wrappedSigner) Sign(rand io.Reader, data []byte) (*Signature, error) {
return s.SignWithAlgorithm(rand, data, "")
return s.SignWithAlgorithm(rand, data, s.pubKey.Type())
}
func (s *wrappedSigner) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) {
var hashFunc crypto.Hash
if _, ok := s.pubKey.(*rsaPublicKey); ok {
// RSA keys support a few hash functions determined by the requested signature algorithm
switch algorithm {
case "", KeyAlgoRSA:
algorithm = KeyAlgoRSA
hashFunc = crypto.SHA1
case KeyAlgoRSASHA256:
hashFunc = crypto.SHA256
case KeyAlgoRSASHA512:
hashFunc = crypto.SHA512
default:
return nil, fmt.Errorf("ssh: unsupported signature algorithm %s", algorithm)
}
} else {
// The only supported algorithm for all other key types is the same as the type of the key
if algorithm == "" {
algorithm = s.pubKey.Type()
} else if algorithm != s.pubKey.Type() {
return nil, fmt.Errorf("ssh: unsupported signature algorithm %s", algorithm)
}
switch key := s.pubKey.(type) {
case *dsaPublicKey:
hashFunc = crypto.SHA1
case *ecdsaPublicKey:
hashFunc = ecHash(key.Curve)
case ed25519PublicKey:
default:
return nil, fmt.Errorf("ssh: unsupported key type %T", key)
}
if algorithm == "" {
algorithm = s.pubKey.Type()
}
supportedAlgos := algorithmsForKeyFormat(s.pubKey.Type())
if !contains(supportedAlgos, algorithm) {
return nil, fmt.Errorf("ssh: unsupported signature algorithm %q for key format %q", algorithm, s.pubKey.Type())
}
hashFunc := hashFuncs[algorithm]
var digest []byte
if hashFunc != 0 {
h := hashFunc.New()