diff options
author | Roland Shoemaker <bracewell@google.com> | 2024-01-18 12:51:13 -0800 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2024-03-05 16:44:27 +0000 |
commit | be5b52bea674190ef7de272664be6c7ae93ec5a0 (patch) | |
tree | 0d8b9dca0196ff11d140cd69a451eb1496123d71 | |
parent | 3643147a29352ca2894fd5d0d2069bc4b4335a7e (diff) | |
download | go-be5b52bea674190ef7de272664be6c7ae93ec5a0.tar.gz go-be5b52bea674190ef7de272664be6c7ae93ec5a0.zip |
[release-branch.go1.21] crypto/x509: make sure pub key is non-nil before interface conversion
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>
-rw-r--r-- | src/crypto/x509/verify.go | 2 | ||||
-rw-r--r-- | src/crypto/x509/verify_test.go | 19 |
2 files changed, 20 insertions, 1 deletions
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 345d434453c..56a1a1725cc 100644 --- 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 37f13f861f1..6d09d24da23 100644 --- 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{}) + } +} |