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

2 files changed, 12 insertions, 5 deletions

diff --git a/src/pkg/crypto/x509/verify.go b/src/pkg/crypto/x509/verify.go
index 20a81574d0..cad863db82 100644
--- 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 7a631186a2..111f60eb11 100644
--- 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"},
 		},
 	},
 }