crypto/x509: prevent chain cycles in Verify

It's possible to include a self-signed root certificate as an
intermediate and push Verify into a loop.

I already had a test for this so I thought that it was ok, but it
turns out that the test was void because the Verisign root certificate
doesn't contain the "IsCA" flag and so it wasn't an acceptable
intermediate certificate for that reason.

R=bradfitz
CC=golang-dev
https://golang.org/cl/4657080

diff --git a/src/pkg/crypto/x509/verify.go b/src/pkg/crypto/x509/verify.go
index 20a81574d0a82694f1730ebc597318bca72387bb..cad863db82cb356b1c17e8d822edb34b89f31f13 100644 (file)
--- a/src/pkg/crypto/x509/verify.go
+++ b/src/pkg/crypto/x509/verify.go
@@ -171,8 +171,14 @@ func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain [
                chains = append(chains, appendToFreshChain(currentChain, root))
        }
 
+nextIntermediate:
        for _, intermediateNum := range opts.Intermediates.findVerifiedParents(c) {
                intermediate := opts.Intermediates.certs[intermediateNum]
+               for _, cert := range currentChain {
+                       if cert == intermediate {
+                               continue nextIntermediate
+                       }
+               }
                err = intermediate.isValid(intermediateCertificate, opts)
                if err != nil {
                        continue

diff --git a/src/pkg/crypto/x509/verify_test.go b/src/pkg/crypto/x509/verify_test.go
index 7a631186a22cfb1609a769fd794f2e244ddced2d..111f60eb1141d9745c4a5370e7a715f129c890c2 100644 (file)
--- a/src/pkg/crypto/x509/verify_test.go
+++ b/src/pkg/crypto/x509/verify_test.go
@@ -72,23 +72,24 @@ var verifyTests = []verifyTest{
                },
        },
        {
-               leaf:          googleLeaf,
-               intermediates: []string{verisignRoot, thawteIntermediate},
-               roots:         []string{verisignRoot},
+               leaf:          dnssecExpLeaf,
+               intermediates: []string{startComIntermediate},
+               roots:         []string{startComRoot},
                currentTime:   1302726541,
 
                expectedChains: [][]string{
-                       []string{"Google", "Thawte", "VeriSign"},
+                       []string{"dnssec-exp", "StartCom Class 1", "StartCom Certification Authority"},
                },
        },
        {
                leaf:          dnssecExpLeaf,
-               intermediates: []string{startComIntermediate},
+               intermediates: []string{startComIntermediate, startComRoot},
                roots:         []string{startComRoot},
                currentTime:   1302726541,
 
                expectedChains: [][]string{
                        []string{"dnssec-exp", "StartCom Class 1", "StartCom Certification Authority"},
+                       []string{"dnssec-exp", "StartCom Class 1", "StartCom Certification Authority", "StartCom Certification Authority"},
                },
        },
 }