code review 116360043: crypto/tls: Add new DigestSigner and KeyAlgorithm types... (issue 116360043 by jdeprez@google.com)

[jde...@google.com]

Reviewers: agl1,

Message:
Hello a...@golang.org (cc: golang-co...@googlegroups.com,
js...@google.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
crypto/tls: Add new DigestSigner and KeyAlgorithm types and fields to
Config, implement defaults for each, and use inside doFullHandshake to
produce the certificate verify message.

Please review this at https://codereview.appspot.com/116360043/

Affected files (+66, -15 lines):
M src/pkg/crypto/tls/common.go
M src/pkg/crypto/tls/handshake_client.go


Index: src/pkg/crypto/tls/common.go
===================================================================
--- a/src/pkg/crypto/tls/common.go
+++ b/src/pkg/crypto/tls/common.go
@@ -301,9 +301,27 @@
// be used.
CurvePreferences []CurveID

+ // DigestSigner is used to produce the signature required for the
+ // CertificateVerify message sent from the client during mutual
+ // authentication. If nil, the default signing algorithms will be used.
+ DigestSigner DigestSigner
+
+ // KeyAlgorithm determines which signing algorithm is used for a given
+ // private key. If nil, the key algorithm will be determined from the
+ // Go type.
+ KeyAlgorithm KeyAlgorithm
+
serverInitOnce sync.Once // guards calling (*Config).serverInit
}

+// DigestSigner uses a given private key to produce a signature of a given
message
+// digest, according to the specified hash function. An entropy source
rand is
+// provided if required.
+type DigestSigner func(privateKey interface{}, digest []byte, hashFunc
crypto.Hash, rand io.Reader) (signed []byte, err error)
+
+// KeyAlgorithm provides the algorithm type of a private key.
+type KeyAlgorithm func(privateKey interface{}) (x509.PublicKeyAlgorithm,
error)
+
func (c *Config) serverInit() {
if c.SessionTicketsDisabled {
return
Index: src/pkg/crypto/tls/handshake_client.go
===================================================================
--- a/src/pkg/crypto/tls/handshake_client.go
+++ b/src/pkg/crypto/tls/handshake_client.go
@@ -6,6 +6,7 @@

import (
"bytes"
+ "crypto"
"crypto/ecdsa"
"crypto/rsa"
"crypto/subtle"
@@ -415,23 +416,30 @@
hasSignatureAndHash: c.vers >= VersionTLS12,
}

- switch key := c.config.Certificates[0].PrivateKey.(type) {
- case *ecdsa.PrivateKey:
- digest, _, hashId :=
hs.finishedHash.hashForClientCertificate(signatureECDSA)
- r, s, err := ecdsa.Sign(c.config.rand(), key, digest)
- if err == nil {
- signed, err = asn1.Marshal(ecdsaSignature{r, s})
- }
- certVerify.signatureAndHash.signature = signatureECDSA
- certVerify.signatureAndHash.hash = hashId
- case *rsa.PrivateKey:
- digest, hashFunc, hashId :=
hs.finishedHash.hashForClientCertificate(signatureRSA)
- signed, err = rsa.SignPKCS1v15(c.config.rand(), key, hashFunc, digest)
- certVerify.signatureAndHash.signature = signatureRSA
- certVerify.signatureAndHash.hash = hashId
+ h := &certVerify.signatureAndHash
+ getKeyAlg := c.config.KeyAlgorithm
+ if getKeyAlg == nil {
+ getKeyAlg = defaultKeyAlgorithm
+ }
+ keyAlg, err := getKeyAlg(chainToSend.PrivateKey)
+ if err != nil {
+ return err
+ }
+ switch keyAlg {
+ case x509.ECDSA:
+ h.signature = signatureECDSA
+ case x509.RSA:
+ h.signature = signatureRSA
default:
- err = errors.New("unknown private key type")
+ return errors.New("tls: unknown key algorithm")
}
+ digest, hashFunc, hashID :=
hs.finishedHash.hashForClientCertificate(h.signature)
+ h.hash = hashID
+ signer := c.config.DigestSigner
+ if signer == nil {
+ signer = defaultDigestSigner
+ }
+ signed, err = signer(chainToSend.PrivateKey, digest, hashFunc,
c.config.rand())
if err != nil {
c.sendAlert(alertInternalError)
return errors.New("tls: failed to sign handshake with client
certificate: " + err.Error())
@@ -446,6 +454,31 @@
return nil
}

+func defaultKeyAlgorithm(privateKey interface{}) (x509.PublicKeyAlgorithm,
error) {
+ switch privateKey.(type) {
+ case *ecdsa.PrivateKey:
+ return x509.ECDSA, nil
+ case *rsa.PrivateKey:
+ return x509.RSA, nil
+ }
+ return x509.UnknownPublicKeyAlgorithm, errors.New("tls: unknown private
key type")
+}
+
+func defaultDigestSigner(privateKey interface{}, digest []byte, hashFunc
crypto.Hash, rand io.Reader) (signed []byte, err error) {
+ switch key := privateKey.(type) {
+ case *ecdsa.PrivateKey:
+ r, s, err := ecdsa.Sign(rand, key, digest)
+ if err == nil {
+ signed, err = asn1.Marshal(ecdsaSignature{r, s})
+ }
+ case *rsa.PrivateKey:
+ signed, err = rsa.SignPKCS1v15(rand, key, hashFunc, digest)
+ default:
+ err = errors.New("tls: unknown private key type")
+ }
+ return
+}
+
func (hs *clientHandshakeState) establishKeys() error {
c := hs.c

[a...@golang.org]

NACK.

I do hope to support this in 1.4, but it's not specific to crypto/tls.

https://codereview.appspot.com/116360043/

[jde...@google.com]

On 2014/07/28 17:44:23, agl1 wrote:
> NACK.

> I do hope to support this in 1.4, but it's not specific to crypto/tls.

Anything I can do to assist?

https://codereview.appspot.com/116360043/

[Adam Langley]

Please see whether https://codereview.appspot.com/114680043 meets your
needs. If so, I'll send it out for review.

[a...@golang.org]

R=close

https://codereview.appspot.com/116360043/

[jde...@google.com]

*** Abandoned ***

https://codereview.appspot.com/116360043/