Skip to content

Commit 7a48b48

Browse files
LekensteynLekensteyn
committed
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
1 parent b72678f commit 7a48b48

File tree

5 files changed

+139
-194
lines changed

5 files changed

+139
-194
lines changed

‎src/crypto/tls/auth.go

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package tls
6+
7+
import (
8+
"crypto"
9+
"crypto/ecdsa"
10+
"crypto/rsa"
11+
"encoding/asn1"
12+
"errors"
13+
"fmt"
14+
)
15+
16+
// pickSignatureAlgorithm selects a signature algorithm that is compatible with
17+
// the given public key and the list of algorithms from the peer and this side.
18+
//
19+
// The returned SignatureScheme codepoint is only meaningful for TLS 1.2,
20+
// previous TLS versions have a fixed hash function.
21+
func pickSignatureAlgorithm(pubkey crypto.PublicKey, peerSigAlgs, ourSigAlgs []SignatureScheme, tlsVersion uint16) (SignatureScheme, uint8, crypto.Hash, error) {
22+
if tlsVersion < VersionTLS12 || len(peerSigAlgs) == 0 {
23+
// If the client didn't specify any signature_algorithms
24+
// extension then we can assume that it supports SHA1. See
25+
// http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1
26+
switch pubkey.(type) {
27+
case *rsa.PublicKey:
28+
if tlsVersion < VersionTLS12 {
29+
return 0, signatureRSA, crypto.MD5SHA1, nil
30+
} else {
31+
return PKCS1WithSHA1, signatureRSA, crypto.SHA1, nil
32+
}
33+
case *ecdsa.PublicKey:
34+
return ECDSAWithSHA1, signatureECDSA, crypto.SHA1, nil
35+
default:
36+
return 0, 0, 0, fmt.Errorf("tls: unsupported public key: %T", pubkey)
37+
}
38+
}
39+
for _, sigAlg := range peerSigAlgs {
40+
if !isSupportedSignatureAlgorithm(sigAlg, ourSigAlgs) {
41+
continue
42+
}
43+
hashAlg, err := lookupTLSHash(sigAlg)
44+
if err != nil {
45+
panic("tls: supported signature algorithm has an unknown hash function")
46+
}
47+
sigType := signatureFromSignatureScheme(sigAlg)
48+
switch pubkey.(type) {
49+
case *rsa.PublicKey:
50+
if sigType == signatureRSA {
51+
return sigAlg, sigType, hashAlg, nil
52+
}
53+
case *ecdsa.PublicKey:
54+
if sigType == signatureECDSA {
55+
return sigAlg, sigType, hashAlg, nil
56+
}
57+
}
58+
}
59+
return 0, 0, 0, errors.New("tls: peer doesn't support any common signature algorithms")
60+
}
61+
62+
// verifyHandshakeSignature verifies a signature against pre-hashed handshake
63+
// contents.
64+
func verifyHandshakeSignature(sigType uint8, pubkey crypto.PublicKey, hashFunc crypto.Hash, digest, sig []byte) error {
65+
switch sigType {
66+
case signatureECDSA:
67+
pubKey, ok := pubkey.(*ecdsa.PublicKey)
68+
if !ok {
69+
return errors.New("tls: ECDSA signing requires a ECDSA public key")
70+
}
71+
ecdsaSig := new(ecdsaSignature)
72+
if _, err := asn1.Unmarshal(sig, ecdsaSig); err != nil {
73+
return err
74+
}
75+
if ecdsaSig.R.Sign() <= 0 || ecdsaSig.S.Sign() <= 0 {
76+
return errors.New("tls: ECDSA signature contained zero or negative values")
77+
}
78+
if !ecdsa.Verify(pubKey, digest, ecdsaSig.R, ecdsaSig.S) {
79+
return errors.New("tls: ECDSA verification failure")
80+
}
81+
case signatureRSA:
82+
pubKey, ok := pubkey.(*rsa.PublicKey)
83+
if !ok {
84+
return errors.New("tls: RSA signing requires a RSA public key")
85+
}
86+
if err := rsa.VerifyPKCS1v15(pubKey, hashFunc, digest, sig); err != nil {
87+
return err
88+
}
89+
default:
90+
return errors.New("tls: unknown signature algorithm")
91+
}
92+
return nil
93+
}

‎src/crypto/tls/handshake_client.go

+5-15
Original file line numberDiff line numberDiff line change
@@ -471,26 +471,16 @@ func (hs *clientHandshakeState) doFullHandshake() error {
471471
return fmt.Errorf("tls: client certificate private key of type %T does not implement crypto.Signer", chainToSend.PrivateKey)
472472
}
473473

474-
var signatureType uint8
475-
switch key.Public().(type) {
476-
case *ecdsa.PublicKey:
477-
signatureType = signatureECDSA
478-
case *rsa.PublicKey:
479-
signatureType = signatureRSA
480-
default:
474+
signatureAlgorithm, sigType, hashFunc, err := pickSignatureAlgorithm(key.Public(), certReq.supportedSignatureAlgorithms, supportedSignatureAlgorithms, c.vers)
475+
if err != nil {
481476
c.sendAlert(alertInternalError)
482-
return fmt.Errorf("tls: failed to sign handshake with client certificate: unknown client certificate key type: %T", key)
477+
return err
483478
}
484-
485479
// SignatureAndHashAlgorithm was introduced in TLS 1.2.
486480
if certVerify.hasSignatureAndHash {
487-
certVerify.signatureAlgorithm, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.supportedSignatureAlgorithms, signatureType)
488-
if err != nil {
489-
c.sendAlert(alertInternalError)
490-
return err
491-
}
481+
certVerify.signatureAlgorithm = signatureAlgorithm
492482
}
493-
digest, hashFunc, err := hs.finishedHash.hashForClientCertificate(signatureType, certVerify.signatureAlgorithm, hs.masterSecret)
483+
digest, err := hs.finishedHash.hashForClientCertificate(sigType, hashFunc, hs.masterSecret)
494484
if err != nil {
495485
c.sendAlert(alertInternalError)
496486
return err

‎src/crypto/tls/handshake_server.go

+7-52
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"crypto/rsa"
1111
"crypto/subtle"
1212
"crypto/x509"
13-
"encoding/asn1"
1413
"errors"
1514
"fmt"
1615
"io"
@@ -519,59 +518,15 @@ func (hs *serverHandshakeState) doFullHandshake() error {
519518
}
520519

521520
// Determine the signature type.
522-
var signatureAlgorithm SignatureScheme
523-
var sigType uint8
524-
if certVerify.hasSignatureAndHash {
525-
signatureAlgorithm = certVerify.signatureAlgorithm
526-
if !isSupportedSignatureAlgorithm(signatureAlgorithm, supportedSignatureAlgorithms) {
527-
return errors.New("tls: unsupported hash function for client certificate")
528-
}
529-
sigType = signatureFromSignatureScheme(signatureAlgorithm)
530-
} else {
531-
// Before TLS 1.2 the signature algorithm was implicit
532-
// from the key type, and only one hash per signature
533-
// algorithm was possible. Leave signatureAlgorithm
534-
// unset.
535-
switch pub.(type) {
536-
case *ecdsa.PublicKey:
537-
sigType = signatureECDSA
538-
case *rsa.PublicKey:
539-
sigType = signatureRSA
540-
}
521+
_, sigType, hashFunc, err := pickSignatureAlgorithm(pub, []SignatureScheme{certVerify.signatureAlgorithm}, supportedSignatureAlgorithms, c.vers)
522+
if err != nil {
523+
c.sendAlert(alertIllegalParameter)
524+
return err
541525
}
542526

543-
switch key := pub.(type) {
544-
case *ecdsa.PublicKey:
545-
if sigType != signatureECDSA {
546-
err = errors.New("tls: bad signature type for client's ECDSA certificate")
547-
break
548-
}
549-
ecdsaSig := new(ecdsaSignature)
550-
if _, err = asn1.Unmarshal(certVerify.signature, ecdsaSig); err != nil {
551-
break
552-
}
553-
if ecdsaSig.R.Sign() <= 0 || ecdsaSig.S.Sign() <= 0 {
554-
err = errors.New("tls: ECDSA signature contained zero or negative values")
555-
break
556-
}
557-
var digest []byte
558-
if digest, _, err = hs.finishedHash.hashForClientCertificate(sigType, signatureAlgorithm, hs.masterSecret); err != nil {
559-
break
560-
}
561-
if !ecdsa.Verify(key, digest, ecdsaSig.R, ecdsaSig.S) {
562-
err = errors.New("tls: ECDSA verification failure")
563-
}
564-
case *rsa.PublicKey:
565-
if sigType != signatureRSA {
566-
err = errors.New("tls: bad signature type for client's RSA certificate")
567-
break
568-
}
569-
var digest []byte
570-
var hashFunc crypto.Hash
571-
if digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(sigType, signatureAlgorithm, hs.masterSecret); err != nil {
572-
break
573-
}
574-
err = rsa.VerifyPKCS1v15(key, hashFunc, digest, certVerify.signature)
527+
var digest []byte
528+
if digest, err = hs.finishedHash.hashForClientCertificate(sigType, hashFunc, hs.masterSecret); err == nil {
529+
err = verifyHandshakeSignature(sigType, pub, hashFunc, digest, certVerify.signature)
575530
}
576531
if err != nil {
577532
c.sendAlert(alertBadCertificate)

‎src/crypto/tls/key_agreement.go

+26-104
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,11 @@ package tls
66

77
import (
88
"crypto"
9-
"crypto/ecdsa"
109
"crypto/elliptic"
1110
"crypto/md5"
1211
"crypto/rsa"
1312
"crypto/sha1"
1413
"crypto/x509"
15-
"encoding/asn1"
1614
"errors"
1715
"io"
1816
"math/big"
@@ -110,58 +108,20 @@ func md5SHA1Hash(slices [][]byte) []byte {
110108
}
111109

112110
// hashForServerKeyExchange hashes the given slices and returns their digest
113-
// and the identifier of the hash function used. The signatureAlgorithm argument
114-
// is only used for >= TLS 1.2 and identifies the hash function to use.
115-
func hashForServerKeyExchange(sigType uint8, signatureAlgorithm SignatureScheme, version uint16, slices ...[]byte) ([]byte, crypto.Hash, error) {
111+
// using the given hash function.
112+
func hashForServerKeyExchange(sigType uint8, hashFunc crypto.Hash, version uint16, slices ...[]byte) ([]byte, error) {
116113
if version >= VersionTLS12 {
117-
if !isSupportedSignatureAlgorithm(signatureAlgorithm, supportedSignatureAlgorithms) {
118-
return nil, crypto.Hash(0), errors.New("tls: unsupported hash function used by peer")
119-
}
120-
hashFunc, err := lookupTLSHash(signatureAlgorithm)
121-
if err != nil {
122-
return nil, crypto.Hash(0), err
123-
}
124114
h := hashFunc.New()
125115
for _, slice := range slices {
126116
h.Write(slice)
127117
}
128118
digest := h.Sum(nil)
129-
return digest, hashFunc, nil
119+
return digest, nil
130120
}
131121
if sigType == signatureECDSA {
132-
return sha1Hash(slices), crypto.SHA1, nil
133-
}
134-
return md5SHA1Hash(slices), crypto.MD5SHA1, nil
135-
}
136-
137-
// pickTLS12HashForSignature returns a TLS 1.2 hash identifier for signing a
138-
// ServerKeyExchange given the signature type being used and the client's
139-
// advertised list of supported signature and hash combinations.
140-
func pickTLS12HashForSignature(sigType uint8, clientList []SignatureScheme) (SignatureScheme, error) {
141-
if len(clientList) == 0 {
142-
// If the client didn't specify any signature_algorithms
143-
// extension then we can assume that it supports SHA1. See
144-
// http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1
145-
switch sigType {
146-
case signatureRSA:
147-
return PKCS1WithSHA1, nil
148-
case signatureECDSA:
149-
return ECDSAWithSHA1, nil
150-
default:
151-
return 0, errors.New("tls: unknown signature algorithm")
152-
}
122+
return sha1Hash(slices), nil
153123
}
154-
155-
for _, sigAlg := range clientList {
156-
if signatureFromSignatureScheme(sigAlg) != sigType {
157-
continue
158-
}
159-
if isSupportedSignatureAlgorithm(sigAlg, supportedSignatureAlgorithms) {
160-
return sigAlg, nil
161-
}
162-
}
163-
164-
return 0, errors.New("tls: client doesn't support any common hash functions")
124+
return md5SHA1Hash(slices), nil
165125
}
166126

167127
func curveForCurveID(id CurveID) (elliptic.Curve, bool) {
@@ -247,40 +207,25 @@ NextCandidate:
247207
serverECDHParams[3] = byte(len(ecdhePublic))
248208
copy(serverECDHParams[4:], ecdhePublic)
249209

250-
var signatureAlgorithm SignatureScheme
251-
252-
if ka.version >= VersionTLS12 {
253-
var err error
254-
signatureAlgorithm, err = pickTLS12HashForSignature(ka.sigType, clientHello.supportedSignatureAlgorithms)
255-
if err != nil {
256-
return nil, err
257-
}
210+
priv, ok := cert.PrivateKey.(crypto.Signer)
211+
if !ok {
212+
return nil, errors.New("tls: certificate private key does not implement crypto.Signer")
258213
}
259214

260-
digest, hashFunc, err := hashForServerKeyExchange(ka.sigType, signatureAlgorithm, ka.version, clientHello.random, hello.random, serverECDHParams)
215+
signatureAlgorithm, sigType, hashFunc, err := pickSignatureAlgorithm(priv.Public(), clientHello.supportedSignatureAlgorithms, supportedSignatureAlgorithms, ka.version)
261216
if err != nil {
262217
return nil, err
263218
}
219+
if sigType != ka.sigType {
220+
return nil, errors.New("tls: certificate cannot be used with the selected cipher suite")
221+
}
264222

265-
priv, ok := cert.PrivateKey.(crypto.Signer)
266-
if !ok {
267-
return nil, errors.New("tls: certificate private key does not implement crypto.Signer")
223+
digest, err := hashForServerKeyExchange(sigType, hashFunc, ka.version, clientHello.random, hello.random, serverECDHParams)
224+
if err != nil {
225+
return nil, err
268226
}
227+
269228
var sig []byte
270-
switch ka.sigType {
271-
case signatureECDSA:
272-
_, ok := priv.Public().(*ecdsa.PublicKey)
273-
if !ok {
274-
return nil, errors.New("tls: ECDHE ECDSA requires an ECDSA server key")
275-
}
276-
case signatureRSA:
277-
_, ok := priv.Public().(*rsa.PublicKey)
278-
if !ok {
279-
return nil, errors.New("tls: ECDHE RSA requires a RSA server key")
280-
}
281-
default:
282-
return nil, errors.New("tls: unknown ECDHE signature algorithm")
283-
}
284229
sig, err = priv.Sign(config.rand(), digest, hashFunc)
285230
if err != nil {
286231
return nil, errors.New("tls: failed to sign ECDHE parameters: " + err.Error())
@@ -380,53 +325,30 @@ func (ka *ecdheKeyAgreement) processServerKeyExchange(config *Config, clientHell
380325
if ka.version >= VersionTLS12 {
381326
// handle SignatureAndHashAlgorithm
382327
signatureAlgorithm = SignatureScheme(sig[0])<<8 | SignatureScheme(sig[1])
383-
if signatureFromSignatureScheme(signatureAlgorithm) != ka.sigType {
384-
return errServerKeyExchange
385-
}
386328
sig = sig[2:]
387329
if len(sig) < 2 {
388330
return errServerKeyExchange
389331
}
390332
}
333+
_, sigType, hashFunc, err := pickSignatureAlgorithm(cert.PublicKey, []SignatureScheme{signatureAlgorithm}, supportedSignatureAlgorithms, ka.version)
334+
if err != nil {
335+
return err
336+
}
337+
if sigType != ka.sigType {
338+
return errServerKeyExchange
339+
}
340+
391341
sigLen := int(sig[0])<<8 | int(sig[1])
392342
if sigLen+2 != len(sig) {
393343
return errServerKeyExchange
394344
}
395345
sig = sig[2:]
396346

397-
digest, hashFunc, err := hashForServerKeyExchange(ka.sigType, signatureAlgorithm, ka.version, clientHello.random, serverHello.random, serverECDHParams)
347+
digest, err := hashForServerKeyExchange(sigType, hashFunc, ka.version, clientHello.random, serverHello.random, serverECDHParams)
398348
if err != nil {
399349
return err
400350
}
401-
switch ka.sigType {
402-
case signatureECDSA:
403-
pubKey, ok := cert.PublicKey.(*ecdsa.PublicKey)
404-
if !ok {
405-
return errors.New("tls: ECDHE ECDSA requires a ECDSA server public key")
406-
}
407-
ecdsaSig := new(ecdsaSignature)
408-
if _, err := asn1.Unmarshal(sig, ecdsaSig); err != nil {
409-
return err
410-
}
411-
if ecdsaSig.R.Sign() <= 0 || ecdsaSig.S.Sign() <= 0 {
412-
return errors.New("tls: ECDSA signature contained zero or negative values")
413-
}
414-
if !ecdsa.Verify(pubKey, digest, ecdsaSig.R, ecdsaSig.S) {
415-
return errors.New("tls: ECDSA verification failure")
416-
}
417-
case signatureRSA:
418-
pubKey, ok := cert.PublicKey.(*rsa.PublicKey)
419-
if !ok {
420-
return errors.New("tls: ECDHE RSA requires a RSA server public key")
421-
}
422-
if err := rsa.VerifyPKCS1v15(pubKey, hashFunc, digest, sig); err != nil {
423-
return err
424-
}
425-
default:
426-
return errors.New("tls: unknown ECDHE signature algorithm")
427-
}
428-
429-
return nil
351+
return verifyHandshakeSignature(sigType, cert.PublicKey, hashFunc, digest, sig)
430352
}
431353

432354
func (ka *ecdheKeyAgreement) generateClientKeyExchange(config *Config, clientHello *clientHelloMsg, cert *x509.Certificate) ([]byte, *clientKeyExchangeMsg, error) {

0 commit comments

Comments
 (0)