crypto/x509: VerifyOptions.KeyUsages went from any required to all required in 1.10

[grittygrease]

The behavior of x509.Verify changed in Go 1.10, which broke a test in CFSSL.

The previous behavior was that when the array of extended key usages in x509.VerifyOptions contains ExtKeyUsageClientAuth, then calling x509.Verify on a valid chain to a leaf certificate with the client certificate auth EKU set results in a valid response, which is the expected behavior. In Go 1.10, this behavior only works when x509.VerifyOptions only contains ExtKeyUsageClientAuth. If x509.VerifyOptions contains both ExtKeyUsageClientAuth and ExtKeyUsageServerAuth, then x509.Verify fails this check for leaf certificate with a client authentication usage.

Is there a new requirement for leaf certificates with Client Auth?

What version of Go are you using (go version)?

go 1.10

Steps to reproduce

The following test fails in cfssl 1.3 (https://github.com/cloudflare/cfssl/tree/1.3.0)
https://github.com/cloudflare/cfssl/blob/master/bundler/bundler_test.go#L914

Output in Go 1.10

go test -v -run TestBundlerClientAuth ./bundler/...
=== RUN   TestBundlerClientAuth
2018/02/27 14:45:30 [INFO] bundling certificate for
2018/02/27 14:45:30 [INFO] the anchoring root is O=Root CA,L=San Francisco,ST=California,C=US
2018/02/27 14:45:30 [INFO] bundling certificate for
--- FAIL: TestBundlerClientAuth (0.02s)
	bundler_test.go:925: {"code":1214,"message":"x509: certificate specifies an incompatible key usage: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 1}"}
FAIL
FAIL	github.com/cloudflare/cfssl/bundler	0.034s

[grittygrease]

Excuse the edits. I think this can be boiled down to this loop:

go/src/crypto/x509/verify.go

Line 861 in 0681c7c

for _, eku := range requestedKeyUsages {

in which every EKU in opts.KeyUsages is required to pass the ekuPermittedBy test. Compare this to the previous logic where only one of opts.KeyUsages was required to be valid.

[FiloSottile]

The failure does not reproduce on any tagged cfssl version, checkout commit b9f59aa instead. Longer version (or skip to the <hr>):

cfssl 1.3.0 does not build with Go 1.10, and the test passes in cfssl 1.3.1 because it looks like it was fixed here cloudflare/cfssl@c68df53.

Before that fix Verify was invoked with only x509.ExtKeyUsageClientAuth in VerifyOptions (WithKeyUsages(x509.ExtKeyUsageClientAuth)) on a certificate with only ServerAuth EKU:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            29:cb:26:4b:b1:a9:26:b0:11:a4:16:b9:de:4d:fe:fa:07:db:7e:cd
    Signature Algorithm: ecdsa-with-SHA256
        Issuer:
        Validity
            Not Before: Sep 20 20:06:00 2016 GMT
            Not After : Sep 21 02:06:00 2021 GMT
        Subject:
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:dc:dd:b6:ce:ed:77:77:9d:c4:c1:50:25:91:cc:
                    41:a5:23:76:fa:6a:3d:06:29:c4:29:2e:4e:9f:96:
                    f8:3d:ae:89:3e:69:ed:ff:81:46:2a:b5:7b:fe:7c:
                    2a:a0:e9:c4:0a:27:d6:7b:9b:b9:a9:63:a8:9d:db:
                    65:7a:75:98:80
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Extended Key Usage:
                TLS Web Server Authentication
            X509v3 Basic Constraints: critical
                CA:FALSE
            X509v3 Subject Key Identifier:
                2A:71:13:98:C7:E3:FB:52:D8:CF:45:88:FD:F2:66:50:83:7A:70:F5
            X509v3 Authority Key Identifier:
                keyid:2F:95:E2:1D:11:36:B7:30:22:88:49:89:7A:17:D8:B3:76:34:29:D6

    Signature Algorithm: ecdsa-with-SHA256
         30:46:02:21:00:de:e2:8d:25:ab:3e:9e:be:74:fd:59:e4:1a:
         09:1c:78:cd:64:89:e8:36:c6:46:8a:fe:8d:96:81:d6:f0:bf:
         38:02:21:00:9a:a6:d5:14:cd:b7:2f:56:74:40:1e:5b:f1:2f:
         4b:6f:a9:2b:34:b8:57:3c:52:68:98:cd:c4:4d:12:e8:15:e0
-----BEGIN CERTIFICATE-----
MIIBaDCCAQ2gAwIBAgIUKcsmS7GpJrARpBa53k3++gfbfs0wCgYIKoZIzj0EAwIw
ADAeFw0xNjA5MjAyMDA2MDBaFw0yMTA5MjEwMjA2MDBaMAAwWTATBgcqhkjOPQIB
BggqhkjOPQMBBwNCAATc3bbO7Xd3ncTBUCWRzEGlI3b6aj0GKcQpLk6flvg9rok+
ae3/gUYqtXv+fCqg6cQKJ9Z7m7mpY6id22V6dZiAo2UwYzATBgNVHSUEDDAKBggr
BgEFBQcDATAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBQqcROYx+P7UtjPRYj98mZQ
g3pw9TAfBgNVHSMEGDAWgBQvleIdETa3MCKISYl6F9izdjQp1jAKBggqhkjOPQQD
AgNJADBGAiEA3uKNJas+nr50/VnkGgkceM1kieg2xkaK/o2WgdbwvzgCIQCaptUU
zbcvVnRAHlvxL0tvqSs0uFc8UmiYzcRNEugV4A==
-----END CERTIFICATE-----

So failing is expected behavior AFAICT.

But still I couldn't figure out why it was failing before, because the point of #23884 was to make this specific case fail again. So I checked out c68df53^ (e9bb1b2) and tested with Go 1.10, and indeed it passed, while it failed with tip as expected.

But then again it was failing on the other certificate. WithKeyUsages(x509.ExtKeyUsageClientAuth) was indeed introduced at cloudflare/cfssl@459761ceb. Rollbacking to before that commit gets us to a test that fails with Go 1.10 on the ServerAuth certificate and with tip on the ClientAuth certificate (or probably both). (#23884 explains the discrepancy.)

---

Verify is being called with four values in VerifyOptions.KeyUsages.

KeyUsages: []x509.ExtKeyUsage{
    x509.ExtKeyUsageServerAuth,
    x509.ExtKeyUsageClientAuth,
    x509.ExtKeyUsageMicrosoftServerGatedCrypto,
    x509.ExtKeyUsageNetscapeServerGatedCrypto,
},

The certificates only have ExtKeyUsageServerAuth or ExtKeyUsageClientAuth.

The current behavior is that all VerifyOptions.KeyUsages need to match for verification to pass, so this fails.

go/src/crypto/x509/verify.go

Lines 860 to 870 in b5bd5bf

	NextUsage:
	for _, eku := range requestedKeyUsages {
	for _, leafEKU := range c.ExtKeyUsage {
	if ekuPermittedBy(eku, leafEKU, checkingAgainstLeafCert) {
	continue NextUsage
	}
	}
	oid, _ := oidFromExtKeyUsage(eku)
	return nil, CertificateInvalidError{c, IncompatibleUsage, fmt.Sprintf("%#v", oid)}
	}

However the previous behavior was that any VerifyOptions.KeyUsages needed to match for verification to pass.

go/src/crypto/x509/verify.go

Lines 515 to 517 in 6732fcc

	// We walk down the list and cross out any usages that aren't supported
	// by each certificate. If we cross out all the usages, then the chain
	// is unacceptable.

The old behavior matches the docs, even if vague.

// KeyUsage specifies which Extended Key Usage values are acceptable.

So this is a pretty bad regression AFAICT, breaking most multi-value uses of VerifyOptions.KeyUsages (which must be pretty rare for no one to notice). Targeting 1.10.1.

(I don't think SANs are involved, nor the root or intermediate lack of EKUs.)

/cc @agl

[gopherbot]

Change https://golang.org/cl/97720 mentions this issue: crypto/x509: matching any requested EKU should be sufficient.

[agl]

I misremembered this and thought that all specified EKUs needed to match, but the previous behaviour was clearly that any of them was sufficient.

Sending CL to specify that in the docs and restore old behaviour.

[FiloSottile]

Reopening for cherry-pick.

[andybons]

CL 97720 OK for Go 1.10.1

[gopherbot]

Change https://golang.org/cl/102790 mentions this issue: [release-branch.go1.10] crypto/x509: matching any requested EKU should be sufficient.