forked from golang/go
-
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
crypto/tls: consolidate signatures handling in SKE and CV
ServerKeyExchange and CertificateVerify can share the same logic for picking a signature algorithm (based on the certificate public key and advertised algorithms), selecting a hash algorithm (depending on TLS version) and signature verification. Refactor the code to achieve code reuse, have common error checking (especially for intersecting supported signature algorithms) and to prepare for addition of new signature algorithms. Code should be easier to read since version-dependent logic is concentrated at one place. Change-Id: I978dec3815d28e33c3cfbc85f0c704b1894c25a3 Reviewed-on: https://go-review.googlesource.com/79735 Reviewed-by: Filippo Valsorda <[email protected]> Run-TryBot: Filippo Valsorda <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
- Loading branch information
1 parent
db6c5da
commit 032d9ec
Showing
6 changed files
with
243 additions
and
196 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
// Copyright 2017 The Go Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package tls | ||
|
||
import ( | ||
"crypto" | ||
"crypto/ecdsa" | ||
"crypto/rsa" | ||
"encoding/asn1" | ||
"errors" | ||
"fmt" | ||
) | ||
|
||
// pickSignatureAlgorithm selects a signature algorithm that is compatible with | ||
// the given public key and the list of algorithms from the peer and this side. | ||
// The lists of signature algorithms (peerSigAlgs and ourSigAlgs) are ignored | ||
// for tlsVersion < VersionTLS12. | ||
// | ||
// The returned SignatureScheme codepoint is only meaningful for TLS 1.2, | ||
// previous TLS versions have a fixed hash function. | ||
func pickSignatureAlgorithm(pubkey crypto.PublicKey, peerSigAlgs, ourSigAlgs []SignatureScheme, tlsVersion uint16) (sigAlg SignatureScheme, sigType uint8, hashFunc crypto.Hash, err error) { | ||
if tlsVersion < VersionTLS12 || len(peerSigAlgs) == 0 { | ||
// For TLS 1.1 and before, the signature algorithm could not be | ||
// negotiated and the hash is fixed based on the signature type. | ||
// For TLS 1.2, if the client didn't send signature_algorithms | ||
// extension then we can assume that it supports SHA1. See | ||
// https://tools.ietf.org/html/rfc5246#section-7.4.1.4.1 | ||
switch pubkey.(type) { | ||
case *rsa.PublicKey: | ||
if tlsVersion < VersionTLS12 { | ||
return 0, signatureRSA, crypto.MD5SHA1, nil | ||
} else { | ||
return PKCS1WithSHA1, signatureRSA, crypto.SHA1, nil | ||
} | ||
case *ecdsa.PublicKey: | ||
return ECDSAWithSHA1, signatureECDSA, crypto.SHA1, nil | ||
default: | ||
return 0, 0, 0, fmt.Errorf("tls: unsupported public key: %T", pubkey) | ||
} | ||
} | ||
for _, sigAlg := range peerSigAlgs { | ||
if !isSupportedSignatureAlgorithm(sigAlg, ourSigAlgs) { | ||
continue | ||
} | ||
hashAlg, err := lookupTLSHash(sigAlg) | ||
if err != nil { | ||
panic("tls: supported signature algorithm has an unknown hash function") | ||
} | ||
sigType := signatureFromSignatureScheme(sigAlg) | ||
switch pubkey.(type) { | ||
case *rsa.PublicKey: | ||
if sigType == signatureRSA { | ||
return sigAlg, sigType, hashAlg, nil | ||
} | ||
case *ecdsa.PublicKey: | ||
if sigType == signatureECDSA { | ||
return sigAlg, sigType, hashAlg, nil | ||
} | ||
default: | ||
return 0, 0, 0, fmt.Errorf("tls: unsupported public key: %T", pubkey) | ||
} | ||
} | ||
return 0, 0, 0, errors.New("tls: peer doesn't support any common signature algorithms") | ||
} | ||
|
||
// verifyHandshakeSignature verifies a signature against pre-hashed handshake | ||
// contents. | ||
func verifyHandshakeSignature(sigType uint8, pubkey crypto.PublicKey, hashFunc crypto.Hash, digest, sig []byte) error { | ||
switch sigType { | ||
case signatureECDSA: | ||
pubKey, ok := pubkey.(*ecdsa.PublicKey) | ||
if !ok { | ||
return errors.New("tls: ECDSA signing requires a ECDSA public key") | ||
} | ||
ecdsaSig := new(ecdsaSignature) | ||
if _, err := asn1.Unmarshal(sig, ecdsaSig); err != nil { | ||
return err | ||
} | ||
if ecdsaSig.R.Sign() <= 0 || ecdsaSig.S.Sign() <= 0 { | ||
return errors.New("tls: ECDSA signature contained zero or negative values") | ||
} | ||
if !ecdsa.Verify(pubKey, digest, ecdsaSig.R, ecdsaSig.S) { | ||
return errors.New("tls: ECDSA verification failure") | ||
} | ||
case signatureRSA: | ||
pubKey, ok := pubkey.(*rsa.PublicKey) | ||
if !ok { | ||
return errors.New("tls: RSA signing requires a RSA public key") | ||
} | ||
if err := rsa.VerifyPKCS1v15(pubKey, hashFunc, digest, sig); err != nil { | ||
return err | ||
} | ||
default: | ||
return errors.New("tls: unknown signature algorithm") | ||
} | ||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// Copyright 2017 The Go Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package tls | ||
|
||
import ( | ||
"crypto" | ||
"testing" | ||
) | ||
|
||
func TestSignatureSelection(t *testing.T) { | ||
rsaCert := &testRSAPrivateKey.PublicKey | ||
ecdsaCert := &testECDSAPrivateKey.PublicKey | ||
sigsPKCS1WithSHA := []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1} | ||
sigsECDSAWithSHA := []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1} | ||
|
||
tests := []struct { | ||
pubkey crypto.PublicKey | ||
peerSigAlgs []SignatureScheme | ||
ourSigAlgs []SignatureScheme | ||
tlsVersion uint16 | ||
|
||
expectedSigAlg SignatureScheme // or 0 if ignored | ||
expectedSigType uint8 | ||
expectedHash crypto.Hash | ||
}{ | ||
// Hash is fixed for RSA in TLS 1.1 and before. | ||
// https://tools.ietf.org/html/rfc4346#page-44 | ||
{rsaCert, nil, nil, VersionTLS11, 0, signatureRSA, crypto.MD5SHA1}, | ||
{rsaCert, nil, nil, VersionTLS10, 0, signatureRSA, crypto.MD5SHA1}, | ||
{rsaCert, nil, nil, VersionSSL30, 0, signatureRSA, crypto.MD5SHA1}, | ||
|
||
// Before TLS 1.2, there is no signature_algorithms extension | ||
// nor field in CertificateRequest and digitally-signed and thus | ||
// it should be ignored. | ||
{rsaCert, sigsPKCS1WithSHA, nil, VersionTLS11, 0, signatureRSA, crypto.MD5SHA1}, | ||
{rsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS11, 0, signatureRSA, crypto.MD5SHA1}, | ||
// Use SHA-1 for TLS 1.0 and 1.1 with ECDSA, see https://tools.ietf.org/html/rfc4492#page-20 | ||
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS11, 0, signatureECDSA, crypto.SHA1}, | ||
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS10, 0, signatureECDSA, crypto.SHA1}, | ||
|
||
// TLS 1.2 without signature_algorithms extension | ||
// https://tools.ietf.org/html/rfc5246#page-47 | ||
{rsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signatureRSA, crypto.SHA1}, | ||
{ecdsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1}, | ||
|
||
{rsaCert, []SignatureScheme{PKCS1WithSHA1}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signatureRSA, crypto.SHA1}, | ||
{rsaCert, []SignatureScheme{PKCS1WithSHA256}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA256, signatureRSA, crypto.SHA256}, | ||
// "sha_hash" may denote hashes other than SHA-1 | ||
// https://tools.ietf.org/html/draft-ietf-tls-rfc4492bis-17#page-17 | ||
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, sigsECDSAWithSHA, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1}, | ||
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, sigsECDSAWithSHA, VersionTLS12, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256}, | ||
} | ||
|
||
for testNo, test := range tests { | ||
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion) | ||
if err != nil { | ||
t.Errorf("test[%d]: unexpected error: %v", testNo, err) | ||
} | ||
if test.expectedSigAlg != 0 && test.expectedSigAlg != sigAlg { | ||
t.Errorf("test[%d]: expected signature scheme %#x, got %#x", testNo, test.expectedSigAlg, sigAlg) | ||
} | ||
if test.expectedSigType != sigType { | ||
t.Errorf("test[%d]: expected signature algorithm %#x, got %#x", testNo, test.expectedSigType, sigType) | ||
} | ||
if test.expectedHash != hashFunc { | ||
t.Errorf("test[%d]: expected hash function %#x, got %#x", testNo, test.expectedHash, hashFunc) | ||
} | ||
} | ||
|
||
badTests := []struct { | ||
pubkey crypto.PublicKey | ||
peerSigAlgs []SignatureScheme | ||
ourSigAlgs []SignatureScheme | ||
tlsVersion uint16 | ||
}{ | ||
{rsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12}, | ||
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS12}, | ||
{ecdsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12}, | ||
{rsaCert, []SignatureScheme{0}, sigsPKCS1WithSHA, VersionTLS12}, | ||
|
||
// ECDSA is unspecified for SSL 3.0 in RFC 4492. | ||
// TODO a SSL 3.0 client cannot advertise signature_algorithms, | ||
// but if an application feeds an ECDSA certificate anyway, it | ||
// will be accepted rather than trigger a handshake failure. Ok? | ||
//{ecdsaCert, nil, nil, VersionSSL30}, | ||
} | ||
|
||
for testNo, test := range badTests { | ||
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion) | ||
if err == nil { | ||
t.Errorf("test[%d]: unexpected success, got %#x %#x %#x", testNo, sigAlg, sigType, hashFunc) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.