From f6e2eab8e02c3797d03987b5f394b2cb775b5518 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Mon, 11 Oct 2010 10:39:56 -0400 Subject: [PATCH] crypto/tls: better error messages for certificate issues. Fixes #1146. R=rsc, agl1 CC=golang-dev https://golang.org/cl/2380042 --- src/pkg/crypto/tls/handshake_client.go | 20 +++++++++++++------- src/pkg/crypto/tls/handshake_server.go | 9 ++++++--- src/pkg/crypto/tls/tls.go | 3 ++- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/pkg/crypto/tls/handshake_client.go b/src/pkg/crypto/tls/handshake_client.go index a37fc78ccaf..bef6d20de89 100644 --- a/src/pkg/crypto/tls/handshake_client.go +++ b/src/pkg/crypto/tls/handshake_client.go @@ -37,7 +37,8 @@ func (c *Conn) clientHandshake() os.Error { hello.random[3] = byte(t) _, err := io.ReadFull(c.config.Rand, hello.random[4:]) if err != nil { - return c.sendAlert(alertInternalError) + c.sendAlert(alertInternalError) + return os.ErrorString("short read from Rand") } finishedHash.Write(hello.marshal()) @@ -79,14 +80,16 @@ func (c *Conn) clientHandshake() os.Error { for i, asn1Data := range certMsg.certificates { cert, err := x509.ParseCertificate(asn1Data) if err != nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("failed to parse certificate from server: " + err.String()) } certs[i] = cert } for i := 1; i < len(certs); i++ { if !certs[i].BasicConstraintsValid || !certs[i].IsCA { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("intermediate certificate does not have CA bit set") } // KeyUsage status flags are ignored. From Engineering // Security, Peter Gutmann: @@ -109,7 +112,8 @@ func (c *Conn) clientHandshake() os.Error { // could only be used for Diffie-Hellman key agreement. if err := certs[i-1].CheckSignatureFrom(certs[i]); err != nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not validate certificate signature: " + err.String()) } } @@ -117,10 +121,12 @@ func (c *Conn) clientHandshake() os.Error { if c.config.RootCAs != nil { root := c.config.RootCAs.FindParent(certs[len(certs)-1]) if root == nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not find root certificate for chain") } - if certs[len(certs)-1].CheckSignatureFrom(root) != nil { - return c.sendAlert(alertBadCertificate) + if err := certs[len(certs)-1].CheckSignatureFrom(root); err != nil { + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not validate signature from expected root: " + err.String()) } } diff --git a/src/pkg/crypto/tls/handshake_server.go b/src/pkg/crypto/tls/handshake_server.go index 118dd4352f3..71cbe6a4dd0 100644 --- a/src/pkg/crypto/tls/handshake_server.go +++ b/src/pkg/crypto/tls/handshake_server.go @@ -145,7 +145,8 @@ func (c *Conn) serverHandshake() os.Error { for i, asn1Data := range certMsg.certificates { cert, err := x509.ParseCertificate(asn1Data) if err != nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not parse client's certificate: " + err.String()) } certs[i] = cert } @@ -153,7 +154,8 @@ func (c *Conn) serverHandshake() os.Error { // TODO(agl): do better validation of certs: max path length, name restrictions etc. for i := 1; i < len(certs); i++ { if err := certs[i-1].CheckSignatureFrom(certs[i]); err != nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not validate certificate signature: " + err.String()) } } @@ -199,7 +201,8 @@ func (c *Conn) serverHandshake() os.Error { copy(digest[16:36], finishedHash.serverSHA1.Sum()) err = rsa.VerifyPKCS1v15(pub, rsa.HashMD5SHA1, digest, certVerify.signature) if err != nil { - return c.sendAlert(alertBadCertificate) + c.sendAlert(alertBadCertificate) + return os.ErrorString("could not validate signature of connection nonces: " + err.String()) } finishedHash.Write(certVerify.marshal()) diff --git a/src/pkg/crypto/tls/tls.go b/src/pkg/crypto/tls/tls.go index 2aec160a1e3..052212f0bbe 100644 --- a/src/pkg/crypto/tls/tls.go +++ b/src/pkg/crypto/tls/tls.go @@ -76,7 +76,8 @@ func Dial(network, laddr, raddr string) (net.Conn, os.Error) { return nil, err } -// LoadX509KeyPair +// LoadX509KeyPair reads and parses a public/private key pair from a pair of +// files. The files must contain PEM encoded data. func LoadX509KeyPair(certFile string, keyFile string) (cert Certificate, err os.Error) { certPEMBlock, err := ioutil.ReadFile(certFile) if err != nil {