Commit b419e2b5 authored by Adam Langley's avatar Adam Langley

crypto/x509: provide better error messages for X.509 verify failures.

Failures caused by errors like invalid signatures or missing hash
functions cause rather generic, unhelpful error messages because no
trust chain can be constructed: "x509: certificate signed by unknown
authority."

With this change, authority errors may contain the reason why an
arbitary candidate step in the chain was rejected. For example, in the
event of a missing hash function the error looks like:

x509: certificate signed by unknown authority (possibly because of
"crypto/x509: cannot verify signature: algorithm unimplemented" while
trying to verify candidate authority certificate 'Thawte SGC CA')

Fixes 5058.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/9104051
parent 910bd157
...@@ -25,9 +25,10 @@ func NewCertPool() *CertPool { ...@@ -25,9 +25,10 @@ func NewCertPool() *CertPool {
} }
// findVerifiedParents attempts to find certificates in s which have signed the // findVerifiedParents attempts to find certificates in s which have signed the
// given certificate. If no such certificate can be found or the signature // given certificate. If any candidates were rejected then errCert will be set
// doesn't match, it returns nil. // to one of them, arbitrarily, and err will contain the reason that it was
func (s *CertPool) findVerifiedParents(cert *Certificate) (parents []int) { // rejected.
func (s *CertPool) findVerifiedParents(cert *Certificate) (parents []int, errCert *Certificate, err error) {
if s == nil { if s == nil {
return return
} }
...@@ -41,8 +42,10 @@ func (s *CertPool) findVerifiedParents(cert *Certificate) (parents []int) { ...@@ -41,8 +42,10 @@ func (s *CertPool) findVerifiedParents(cert *Certificate) (parents []int) {
} }
for _, c := range candidates { for _, c := range candidates {
if cert.CheckSignatureFrom(s.certs[c]) == nil { if err = cert.CheckSignatureFrom(s.certs[c]); err == nil {
parents = append(parents, c) parents = append(parents, c)
} else {
errCert = s.certs[c]
} }
} }
......
...@@ -89,7 +89,7 @@ func checkChainTrustStatus(c *Certificate, chainCtx *syscall.CertChainContext) e ...@@ -89,7 +89,7 @@ func checkChainTrustStatus(c *Certificate, chainCtx *syscall.CertChainContext) e
case syscall.CERT_TRUST_IS_NOT_TIME_VALID: case syscall.CERT_TRUST_IS_NOT_TIME_VALID:
return CertificateInvalidError{c, Expired} return CertificateInvalidError{c, Expired}
default: default:
return UnknownAuthorityError{c} return UnknownAuthorityError{c, nil, nil}
} }
} }
return nil return nil
...@@ -129,9 +129,9 @@ func checkChainSSLServerPolicy(c *Certificate, chainCtx *syscall.CertChainContex ...@@ -129,9 +129,9 @@ func checkChainSSLServerPolicy(c *Certificate, chainCtx *syscall.CertChainContex
case syscall.CERT_E_CN_NO_MATCH: case syscall.CERT_E_CN_NO_MATCH:
return HostnameError{c, opts.DNSName} return HostnameError{c, opts.DNSName}
case syscall.CERT_E_UNTRUSTEDROOT: case syscall.CERT_E_UNTRUSTEDROOT:
return UnknownAuthorityError{c} return UnknownAuthorityError{c, nil, nil}
default: default:
return UnknownAuthorityError{c} return UnknownAuthorityError{c, nil, nil}
} }
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package x509 package x509
import ( import (
"fmt"
"net" "net"
"runtime" "runtime"
"strings" "strings"
...@@ -91,10 +92,27 @@ func (h HostnameError) Error() string { ...@@ -91,10 +92,27 @@ func (h HostnameError) Error() string {
// UnknownAuthorityError results when the certificate issuer is unknown // UnknownAuthorityError results when the certificate issuer is unknown
type UnknownAuthorityError struct { type UnknownAuthorityError struct {
cert *Certificate cert *Certificate
// hintErr contains an error that may be helpful in determining why an
// authority wasn't found.
hintErr error
// hintCert contains a possible authority certificate that was rejected
// because of the error in hintErr.
hintCert *Certificate
} }
func (e UnknownAuthorityError) Error() string { func (e UnknownAuthorityError) Error() string {
return "x509: certificate signed by unknown authority" s := "x509: certificate signed by unknown authority"
if e.hintErr != nil {
certName := e.hintCert.Subject.CommonName
if len(certName) == 0 {
if len(e.hintCert.Subject.Organization) > 0 {
certName = e.hintCert.Subject.Organization[0]
}
certName = "serial:" + e.hintCert.SerialNumber.String()
}
s += fmt.Sprintf(" (possibly because of %q while trying to verify candidate authority certificate %q)", e.hintErr, certName)
}
return s
} }
// SystemRootsError results when we fail to load the system root certificates. // SystemRootsError results when we fail to load the system root certificates.
...@@ -249,7 +267,8 @@ func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate ...@@ -249,7 +267,8 @@ func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate
} }
func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain []*Certificate, opts *VerifyOptions) (chains [][]*Certificate, err error) { func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain []*Certificate, opts *VerifyOptions) (chains [][]*Certificate, err error) {
for _, rootNum := range opts.Roots.findVerifiedParents(c) { possibleRoots, failedRoot, rootErr := opts.Roots.findVerifiedParents(c)
for _, rootNum := range possibleRoots {
root := opts.Roots.certs[rootNum] root := opts.Roots.certs[rootNum]
err = root.isValid(rootCertificate, currentChain, opts) err = root.isValid(rootCertificate, currentChain, opts)
if err != nil { if err != nil {
...@@ -258,8 +277,9 @@ func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain [ ...@@ -258,8 +277,9 @@ func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain [
chains = append(chains, appendToFreshChain(currentChain, root)) chains = append(chains, appendToFreshChain(currentChain, root))
} }
possibleIntermediates, failedIntermediate, intermediateErr := opts.Intermediates.findVerifiedParents(c)
nextIntermediate: nextIntermediate:
for _, intermediateNum := range opts.Intermediates.findVerifiedParents(c) { for _, intermediateNum := range possibleIntermediates {
intermediate := opts.Intermediates.certs[intermediateNum] intermediate := opts.Intermediates.certs[intermediateNum]
for _, cert := range currentChain { for _, cert := range currentChain {
if cert == intermediate { if cert == intermediate {
...@@ -284,7 +304,13 @@ nextIntermediate: ...@@ -284,7 +304,13 @@ nextIntermediate:
} }
if len(chains) == 0 && err == nil { if len(chains) == 0 && err == nil {
err = UnknownAuthorityError{c} hintErr := rootErr
hintCert := failedRoot
if hintErr == nil {
hintErr = intermediateErr
hintCert = failedIntermediate
}
err = UnknownAuthorityError{c, hintErr, hintCert}
} }
return return
......
...@@ -126,6 +126,18 @@ var verifyTests = []verifyTest{ ...@@ -126,6 +126,18 @@ var verifyTests = []verifyTest{
{"dnssec-exp", "StartCom Class 1", "StartCom Certification Authority", "StartCom Certification Authority"}, {"dnssec-exp", "StartCom Class 1", "StartCom Certification Authority", "StartCom Certification Authority"},
}, },
}, },
{
leaf: googleLeafWithInvalidHash,
intermediates: []string{thawteIntermediate},
roots: []string{verisignRoot},
currentTime: 1302726541,
dnsName: "www.google.com",
// The specific error message may not occur when using system
// verification.
systemSkip: true,
errorCallback: expectHashError,
},
{ {
// The default configuration should reject an S/MIME chain. // The default configuration should reject an S/MIME chain.
leaf: smimeLeaf, leaf: smimeLeaf,
...@@ -213,6 +225,18 @@ func expectSystemRootsError(t *testing.T, i int, err error) bool { ...@@ -213,6 +225,18 @@ func expectSystemRootsError(t *testing.T, i int, err error) bool {
return true return true
} }
func expectHashError(t *testing.T, i int, err error) bool {
if err == nil {
t.Errorf("#%d: no error resulted from invalid hash", i)
return false
}
if expected := "algorithm unimplemented"; !strings.Contains(err.Error(), expected) {
t.Errorf("#%d: error resulting from invalid hash didn't contain '%s', rather it was: %s", i, expected, err)
return false
}
return true
}
func certificateFromPEM(pemBytes string) (*Certificate, error) { func certificateFromPEM(pemBytes string) (*Certificate, error) {
block, _ := pem.Decode([]byte(pemBytes)) block, _ := pem.Decode([]byte(pemBytes))
if block == nil { if block == nil {
...@@ -400,6 +424,28 @@ u2ONgJd8IyAPkU0Wueru9G2Jysa9zCRo1kNbzipYvzwY4OA8Ys+WAi0oR1A04Se6 ...@@ -400,6 +424,28 @@ u2ONgJd8IyAPkU0Wueru9G2Jysa9zCRo1kNbzipYvzwY4OA8Ys+WAi0oR1A04Se6
z5nRUP8pJcA2NhUzUnC+MY+f6H/nEQyNv4SgQhqAibAxWEEHXw== z5nRUP8pJcA2NhUzUnC+MY+f6H/nEQyNv4SgQhqAibAxWEEHXw==
-----END CERTIFICATE-----` -----END CERTIFICATE-----`
// googleLeafWithInvalidHash is the same as googleLeaf, but the signature
// algorithm in the certificate contains a nonsense OID.
const googleLeafWithInvalidHash = `-----BEGIN CERTIFICATE-----
MIIDITCCAoqgAwIBAgIQL9+89q6RUm0PmqPfQDQ+mjANBgkqhkiG9w0BATIFADBM
MQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkg
THRkLjEWMBQGA1UEAxMNVGhhd3RlIFNHQyBDQTAeFw0wOTEyMTgwMDAwMDBaFw0x
MTEyMTgyMzU5NTlaMGgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlh
MRYwFAYDVQQHFA1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKFApHb29nbGUgSW5jMRcw
FQYDVQQDFA53d3cuZ29vZ2xlLmNvbTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkC
gYEA6PmGD5D6htffvXImttdEAoN4c9kCKO+IRTn7EOh8rqk41XXGOOsKFQebg+jN
gtXj9xVoRaELGYW84u+E593y17iYwqG7tcFR39SDAqc9BkJb4SLD3muFXxzW2k6L
05vuuWciKh0R73mkszeK9P4Y/bz5RiNQl/Os/CRGK1w7t0UCAwEAAaOB5zCB5DAM
BgNVHRMBAf8EAjAAMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6Ly9jcmwudGhhd3Rl
LmNvbS9UaGF3dGVTR0NDQS5jcmwwKAYDVR0lBCEwHwYIKwYBBQUHAwEGCCsGAQUF
BwMCBglghkgBhvhCBAEwcgYIKwYBBQUHAQEEZjBkMCIGCCsGAQUFBzABhhZodHRw
Oi8vb2NzcC50aGF3dGUuY29tMD4GCCsGAQUFBzAChjJodHRwOi8vd3d3LnRoYXd0
ZS5jb20vcmVwb3NpdG9yeS9UaGF3dGVfU0dDX0NBLmNydDANBgkqhkiG9w0BAVAF
AAOBgQCfQ89bxFApsb/isJr/aiEdLRLDLE5a+RLizrmCUi3nHX4adpaQedEkUjh5
u2ONgJd8IyAPkU0Wueru9G2Jysa9zCRo1kNbzipYvzwY4OA8Ys+WAi0oR1A04Se6
z5nRUP8pJcA2NhUzUnC+MY+f6H/nEQyNv4SgQhqAibAxWEEHXw==
-----END CERTIFICATE-----`
const dnssecExpLeaf = `-----BEGIN CERTIFICATE----- const dnssecExpLeaf = `-----BEGIN CERTIFICATE-----
MIIGzTCCBbWgAwIBAgIDAdD6MA0GCSqGSIb3DQEBBQUAMIGMMQswCQYDVQQGEwJJ MIIGzTCCBbWgAwIBAgIDAdD6MA0GCSqGSIb3DQEBBQUAMIGMMQswCQYDVQQGEwJJ
TDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0 TDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0
...@@ -522,6 +568,50 @@ um0ABj6y6koQOdjQK/W/7HW/lwLFCRsI3FU34oH7N4RDYiDK51ZLZer+bMEkkySh ...@@ -522,6 +568,50 @@ um0ABj6y6koQOdjQK/W/7HW/lwLFCRsI3FU34oH7N4RDYiDK51ZLZer+bMEkkySh
NOsF/5oirpt9P/FlUQqmMGqz9IgcgA38corog14= NOsF/5oirpt9P/FlUQqmMGqz9IgcgA38corog14=
-----END CERTIFICATE-----` -----END CERTIFICATE-----`
const startComRootSHA256 = `-----BEGIN CERTIFICATE-----
MIIHhzCCBW+gAwIBAgIBLTANBgkqhkiG9w0BAQsFADB9MQswCQYDVQQGEwJJTDEW
MBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0YWwg
Q2VydGlmaWNhdGUgU2lnbmluZzEpMCcGA1UEAxMgU3RhcnRDb20gQ2VydGlmaWNh
dGlvbiBBdXRob3JpdHkwHhcNMDYwOTE3MTk0NjM3WhcNMzYwOTE3MTk0NjM2WjB9
MQswCQYDVQQGEwJJTDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMi
U2VjdXJlIERpZ2l0YWwgQ2VydGlmaWNhdGUgU2lnbmluZzEpMCcGA1UEAxMgU3Rh
cnRDb20gQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkwggIiMA0GCSqGSIb3DQEBAQUA
A4ICDwAwggIKAoICAQDBiNsJvGxGfHiflXu1M5DycmLWwTYgIiRezul38kMKogZk
pMyONvg45iPwbm2xPN1yo4UcodM9tDMr0y+v/uqwQVlntsQGfQqedIXWeUyAN3rf
OQVSWff0G0ZDpNKFhdLDcfN1YjS6LIp/Ho/u7TTQEceWzVI9ujPW3U3eCztKS5/C
Ji/6tRYccjV3yjxd5srhJosaNnZcAdt0FCX+7bWgiA/deMotHweXMAEtcnn6RtYT
Kqi5pquDSR3l8u/d5AGOGAqPY1MWhWKpDhk6zLVmpsJrdAfkK+F2PrRt2PZE4XNi
HzvEvqBTViVsUQn3qqvKv3b9bZvzndu/PWa8DFaqr5hIlTpL36dYUNk4dalb6kMM
Av+Z6+hsTXBbKWWc3apdzK8BMewM69KN6Oqce+Zu9ydmDBpI125C4z/eIT574Q1w
+2OqqGwaVLRcJXrJosmLFqa7LH4XXgVNWG4SHQHuEhANxjJ/GP/89PrNbpHoNkm+
Gkhpi8KWTRoSsmkXwQqQ1vp5Iki/untp+HDH+no32NgN0nZPV/+Qt+OR0t3vwmC3
Zzrd/qqc8NSLf3Iizsafl7b4r4qgEKjZ+xjGtrVcUjyJthkqcwEKDwOzEmDyei+B
26Nu/yYwl/WL3YlXtq09s68rxbd2AvCl1iuahhQqcvbjM4xdCUsT37uMdBNSSwID
AQABo4ICEDCCAgwwDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMCAQYwHQYD
VR0OBBYEFE4L7xqkQFulF2mHMMo0aEPQQa7yMB8GA1UdIwQYMBaAFE4L7xqkQFul
F2mHMMo0aEPQQa7yMIIBWgYDVR0gBIIBUTCCAU0wggFJBgsrBgEEAYG1NwEBATCC
ATgwLgYIKwYBBQUHAgEWImh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeS5w
ZGYwNAYIKwYBBQUHAgEWKGh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL2ludGVybWVk
aWF0ZS5wZGYwgc8GCCsGAQUFBwICMIHCMCcWIFN0YXJ0IENvbW1lcmNpYWwgKFN0
YXJ0Q29tKSBMdGQuMAMCAQEagZZMaW1pdGVkIExpYWJpbGl0eSwgcmVhZCB0aGUg
c2VjdGlvbiAqTGVnYWwgTGltaXRhdGlvbnMqIG9mIHRoZSBTdGFydENvbSBDZXJ0
aWZpY2F0aW9uIEF1dGhvcml0eSBQb2xpY3kgYXZhaWxhYmxlIGF0IGh0dHA6Ly93
d3cuc3RhcnRzc2wuY29tL3BvbGljeS5wZGYwEQYJYIZIAYb4QgEBBAQDAgAHMDgG
CWCGSAGG+EIBDQQrFilTdGFydENvbSBGcmVlIFNTTCBDZXJ0aWZpY2F0aW9uIEF1
dGhvcml0eTANBgkqhkiG9w0BAQsFAAOCAgEAjo/n3JR5fPGFf59Jb2vKXfuM/gTF
wWLRfUKKvFO3lANmMD+x5wqnUCBVJX92ehQN6wQOQOY+2IirByeDqXWmN3PH/UvS
Ta0XQMhGvjt/UfzDtgUx3M2FIk5xt/JxXrAaxrqTi3iSSoX4eA+D/i+tLPfkpLst
0OcNOrg+zvZ49q5HJMqjNTbOx8aHmNrs++myziebiMMEofYLWWivydsQD032ZGNc
pRJvkrKTlMeIFw6Ttn5ii5B/q06f/ON1FE8qMt9bDeD1e5MNq6HPh+GlBEXoPBKl
CcWw0bdT82AUuoVpaiF8H3VhFyAXe2w7QSlc4axa0c2Mm+tgHRns9+Ww2vl5GKVF
P0lDV9LdJNUso/2RjSe15esUBppMeyG7Oq0wBhjA2MFrLH9ZXF2RsXAiV+uKa0hK
1Q8p7MZAwC+ITGgBF3f0JBlPvfrhsiAhS90a2Cl9qrjeVOwhVYBsHvUwyKMQ5bLm
KhQxw4UtjJixhlpPiVktucf3HMiKf8CdBUrmQk9io20ppB+Fq9vlgcitKj1MXVuE
JnHEhV5xJMqlG2zYYdMa4FTbzrqpMrUi9nNBCV24F10OD5mQ1kfabwo6YigUZ4LZ
8dCAWZvLMdibD4x3TrVoivJs9iQOLWxwxXPR3hTQcY+203sC9uO41Alua551hDnm
fyWl8kgAwKQB2j8=
-----END CERTIFICATE-----`
const smimeLeaf = `-----BEGIN CERTIFICATE----- const smimeLeaf = `-----BEGIN CERTIFICATE-----
MIIFBjCCA+6gAwIBAgISESFvrjT8XcJTEe6rBlPptILlMA0GCSqGSIb3DQEBBQUA MIIFBjCCA+6gAwIBAgISESFvrjT8XcJTEe6rBlPptILlMA0GCSqGSIb3DQEBBQUA
MFQxCzAJBgNVBAYTAkJFMRkwFwYDVQQKExBHbG9iYWxTaWduIG52LXNhMSowKAYD MFQxCzAJBgNVBAYTAkJFMRkwFwYDVQQKExBHbG9iYWxTaWduIG52LXNhMSowKAYD
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment