crypto/tls: refactor certificate and signature algorithm logic

This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.

In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.

Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.

On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.

On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.

There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.

The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.

Updates #32426

Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
This commit is contained in:
Filippo Valsorda 2019-11-01 19:00:33 -04:00
parent edbdd2848e
commit 0c490ab182
11 changed files with 398 additions and 348 deletions

View file

@ -6,71 +6,62 @@ package tls
import (
"crypto"
"crypto/ed25519"
"testing"
)
func TestSignatureSelection(t *testing.T) {
rsaCert := &testRSAPrivateKey.PublicKey
ecdsaCert := &testECDSAPrivateKey.PublicKey
ed25519Cert := testEd25519PrivateKey.Public().(ed25519.PublicKey)
sigsPKCS1WithSHA := []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1}
sigsPSSWithSHA := []SignatureScheme{PSSWithSHA256, PSSWithSHA384}
sigsECDSAWithSHA := []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}
rsaCert := &Certificate{
Certificate: [][]byte{testRSACertificate},
PrivateKey: testRSAPrivateKey,
}
ecdsaCert := &Certificate{
Certificate: [][]byte{testP256Certificate},
PrivateKey: testP256PrivateKey,
}
ed25519Cert := &Certificate{
Certificate: [][]byte{testEd25519Certificate},
PrivateKey: testEd25519PrivateKey,
}
tests := []struct {
pubkey crypto.PublicKey
cert *Certificate
peerSigAlgs []SignatureScheme
ourSigAlgs []SignatureScheme
tlsVersion uint16
expectedSigAlg SignatureScheme // if tlsVersion == VersionTLS12
expectedSigAlg SignatureScheme
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, signaturePKCS1v15, crypto.MD5SHA1},
{rsaCert, nil, nil, VersionTLS10, 0, signaturePKCS1v15, 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, signaturePKCS1v15, crypto.MD5SHA1},
{rsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS11, 0, signaturePKCS1v15, 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},
{rsaCert, []SignatureScheme{PKCS1WithSHA1, PKCS1WithSHA256}, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA512, PKCS1WithSHA1}, VersionTLS12, PKCS1WithSHA512, signaturePKCS1v15, crypto.SHA512},
{rsaCert, []SignatureScheme{PSSWithSHA256, PKCS1WithSHA256}, VersionTLS12, PKCS1WithSHA256, signaturePKCS1v15, crypto.SHA256},
{rsaCert, []SignatureScheme{PSSWithSHA384, PKCS1WithSHA1}, VersionTLS13, PSSWithSHA384, signatureRSAPSS, crypto.SHA384},
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, VersionTLS12, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256},
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, VersionTLS13, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256},
{ed25519Cert, []SignatureScheme{Ed25519}, VersionTLS12, Ed25519, signatureEd25519, directSigning},
{ed25519Cert, []SignatureScheme{Ed25519}, VersionTLS13, Ed25519, signatureEd25519, directSigning},
// TLS 1.2 without signature_algorithms extension
// https://tools.ietf.org/html/rfc5246#page-47
{rsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{ecdsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{rsaCert, nil, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{ecdsaCert, nil, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA1}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA256}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA256, signaturePKCS1v15, 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},
// RSASSA-PSS is defined in TLS 1.3 for TLS 1.2
// https://tools.ietf.org/html/draft-ietf-tls-tls13-21#page-45
{rsaCert, []SignatureScheme{PSSWithSHA256}, sigsPSSWithSHA, VersionTLS12, PSSWithSHA256, signatureRSAPSS, crypto.SHA256},
// All results are fixed for Ed25519. RFC 8422, Section 5.10.
{ed25519Cert, []SignatureScheme{Ed25519}, []SignatureScheme{ECDSAWithSHA1, Ed25519}, VersionTLS12, Ed25519, signatureEd25519, directSigning},
{ed25519Cert, nil, nil, VersionTLS12, Ed25519, signatureEd25519, directSigning},
// TLS 1.2 does not restrict the ECDSA curve (our ecdsaCert is P-256)
{ecdsaCert, []SignatureScheme{ECDSAWithP384AndSHA384}, VersionTLS12, ECDSAWithP384AndSHA384, signatureECDSA, crypto.SHA384},
}
for testNo, test := range tests {
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion)
sigAlg, err := selectSignatureScheme(test.tlsVersion, test.cert, test.peerSigAlgs)
if err != nil {
t.Errorf("test[%d]: unexpected error: %v", testNo, err)
t.Errorf("test[%d]: unexpected selectSignatureScheme error: %v", testNo, err)
}
if test.tlsVersion == VersionTLS12 && test.expectedSigAlg != sigAlg {
if test.expectedSigAlg != sigAlg {
t.Errorf("test[%d]: expected signature scheme %#x, got %#x", testNo, test.expectedSigAlg, sigAlg)
}
sigType, hashFunc, err := typeAndHashFromSignatureScheme(sigAlg)
if err != nil {
t.Errorf("test[%d]: unexpected typeAndHashFromSignatureScheme error: %v", testNo, err)
}
if test.expectedSigType != sigType {
t.Errorf("test[%d]: expected signature algorithm %#x, got %#x", testNo, test.expectedSigType, sigType)
}
@ -80,26 +71,81 @@ func TestSignatureSelection(t *testing.T) {
}
badTests := []struct {
pubkey crypto.PublicKey
cert *Certificate
peerSigAlgs []SignatureScheme
ourSigAlgs []SignatureScheme
tlsVersion uint16
}{
{rsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12},
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS12},
{ecdsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12},
{rsaCert, []SignatureScheme{0}, sigsPKCS1WithSHA, VersionTLS12},
{ed25519Cert, sigsECDSAWithSHA, sigsECDSAWithSHA, VersionTLS12},
{ed25519Cert, []SignatureScheme{Ed25519}, sigsECDSAWithSHA, VersionTLS12},
{ecdsaCert, []SignatureScheme{Ed25519}, []SignatureScheme{Ed25519}, VersionTLS12},
{ed25519Cert, nil, nil, VersionTLS11},
{ed25519Cert, nil, nil, VersionTLS10},
{rsaCert, []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}, VersionTLS12},
{ecdsaCert, []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1}, VersionTLS12},
{rsaCert, []SignatureScheme{0}, VersionTLS12},
{ed25519Cert, []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}, VersionTLS12},
{ecdsaCert, []SignatureScheme{Ed25519}, VersionTLS12},
// RFC 5246, Section 7.4.1.4.1, says to only consider {sha1,ecdsa} as
// default when the extension is missing, and RFC 8422 does not update
// it. Anyway, if a stack supports Ed25519 it better support sigalgs.
{ed25519Cert, nil, VersionTLS12},
// TLS 1.3 has no default signature_algorithms.
{rsaCert, nil, VersionTLS13},
{ecdsaCert, nil, VersionTLS13},
{ed25519Cert, nil, VersionTLS13},
// Wrong curve, which TLS 1.3 checks
{ecdsaCert, []SignatureScheme{ECDSAWithP384AndSHA384}, VersionTLS13},
// TLS 1.3 does not support PKCS1v1.5 or SHA-1.
{rsaCert, []SignatureScheme{PKCS1WithSHA256}, VersionTLS13},
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, VersionTLS13},
}
for testNo, test := range badTests {
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion)
sigAlg, err := selectSignatureScheme(test.tlsVersion, test.cert, test.peerSigAlgs)
if err == nil {
t.Errorf("test[%d]: unexpected success, got %#x %#x %#x", testNo, sigAlg, sigType, hashFunc)
t.Errorf("test[%d]: unexpected success, got %#x", testNo, sigAlg)
}
}
}
func TestLegacyTypeAndHash(t *testing.T) {
sigType, hashFunc, err := legacyTypeAndHashFromPublicKey(testRSAPrivateKey.Public())
if err != nil {
t.Errorf("RSA: unexpected error: %v", err)
}
if expectedSigType := signaturePKCS1v15; expectedSigType != sigType {
t.Errorf("RSA: expected signature type %#x, got %#x", expectedSigType, sigType)
}
if expectedHashFunc := crypto.MD5SHA1; expectedHashFunc != hashFunc {
t.Errorf("RSA: expected hash %#x, got %#x", expectedHashFunc, sigType)
}
sigType, hashFunc, err = legacyTypeAndHashFromPublicKey(testECDSAPrivateKey.Public())
if err != nil {
t.Errorf("ECDSA: unexpected error: %v", err)
}
if expectedSigType := signatureECDSA; expectedSigType != sigType {
t.Errorf("ECDSA: expected signature type %#x, got %#x", expectedSigType, sigType)
}
if expectedHashFunc := crypto.SHA1; expectedHashFunc != hashFunc {
t.Errorf("ECDSA: expected hash %#x, got %#x", expectedHashFunc, sigType)
}
// Ed25519 is not supported by TLS 1.0 and 1.1.
_, _, err = legacyTypeAndHashFromPublicKey(testEd25519PrivateKey.Public())
if err == nil {
t.Errorf("Ed25519: unexpected success")
}
}
// TestSupportedSignatureAlgorithms checks that all supportedSignatureAlgorithms
// have valid type and hash information.
func TestSupportedSignatureAlgorithms(t *testing.T) {
for _, sigAlg := range supportedSignatureAlgorithms {
sigType, hash, err := typeAndHashFromSignatureScheme(sigAlg)
if err != nil {
t.Errorf("%#04x: unexpected error: %v", sigAlg, err)
}
if sigType == 0 {
t.Errorf("%#04x: missing signature type", sigAlg)
}
if hash == 0 && sigAlg != Ed25519 {
t.Errorf("%#04x: missing hash", sigAlg)
}
}
}