[release-branch.go1.21] crypto/x509: make sure pub key is non-nil before interface...

alreadyInChain assumes all keys fit a interface which contains the
Equal method (which they do), but this ignores that certificates may
have a nil key when PublicKeyAlgorithm is UnknownPublicKeyAlgorithm. In
this case alreadyInChain panics.

Check that the key is non-nil as part of considerCandidate (we are never
going to build a chain containing UnknownPublicKeyAlgorithm anyway).

For #65390
Fixes #65392
Fixes CVE-2024-24783

Change-Id: Ibdccc0a487e3368b6812be35daad2512220243f3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2137282
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173774
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569238
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 345d434453c44b98b00be4899862c57d1c8973a1..56a1a1725cc3314b107f97885b2f8c421c9d4b88 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -899,7 +899,7 @@ func (c *Certificate) buildChains(currentChain []*Certificate, sigChecks *int, o
        )
 
        considerCandidate := func(certType int, candidate *Certificate) {
-               if alreadyInChain(candidate, currentChain) {
+               if candidate.PublicKey == nil || alreadyInChain(candidate, currentChain) {
                        return
                }
 

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 37f13f861f1b541fc10c98fe7a584618d33b2dcc..6d09d24da2389b9b47a064139f97da5e0d0f3826 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -2718,3 +2718,22 @@ func TestVerifyEKURootAsLeaf(t *testing.T) {
        }
 
 }
+
+func TestVerifyNilPubKey(t *testing.T) {
+       c := &Certificate{
+               RawIssuer:      []byte{1, 2, 3},
+               AuthorityKeyId: []byte{1, 2, 3},
+       }
+       opts := &VerifyOptions{}
+       opts.Roots = NewCertPool()
+       r := &Certificate{
+               RawSubject:   []byte{1, 2, 3},
+               SubjectKeyId: []byte{1, 2, 3},
+       }
+       opts.Roots.AddCert(r)
+
+       _, err := c.buildChains([]*Certificate{r}, nil, opts)
+       if _, ok := err.(UnknownAuthorityError); !ok {
+               t.Fatalf("buildChains returned unexpected error, got: %v, want %v", err, UnknownAuthorityError{})
+       }
+}