Skip to content

Commit 4da1f86

Browse files
rolandshoemakerbradfitz
authored andcommitted
[release-branch.go1.22] 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 golang#65390 Fixes golang#65831 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 <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2174343 Reviewed-by: Carlos Amedee <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/569235 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Carlos Amedee <[email protected]>
1 parent 729e510 commit 4da1f86

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed

src/crypto/x509/verify.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ func (c *Certificate) buildChains(currentChain []*Certificate, sigChecks *int, o
899899
)
900900

901901
considerCandidate := func(certType int, candidate potentialParent) {
902-
if alreadyInChain(candidate.cert, currentChain) {
902+
if candidate.cert.PublicKey == nil || alreadyInChain(candidate.cert, currentChain) {
903903
return
904904
}
905905

src/crypto/x509/verify_test.go

+19
Original file line numberDiff line numberDiff line change
@@ -2792,3 +2792,22 @@ func TestVerifyEKURootAsLeaf(t *testing.T) {
27922792
}
27932793

27942794
}
2795+
2796+
func TestVerifyNilPubKey(t *testing.T) {
2797+
c := &Certificate{
2798+
RawIssuer: []byte{1, 2, 3},
2799+
AuthorityKeyId: []byte{1, 2, 3},
2800+
}
2801+
opts := &VerifyOptions{}
2802+
opts.Roots = NewCertPool()
2803+
r := &Certificate{
2804+
RawSubject: []byte{1, 2, 3},
2805+
SubjectKeyId: []byte{1, 2, 3},
2806+
}
2807+
opts.Roots.AddCert(r)
2808+
2809+
_, err := c.buildChains([]*Certificate{r}, nil, opts)
2810+
if _, ok := err.(UnknownAuthorityError); !ok {
2811+
t.Fatalf("buildChains returned unexpected error, got: %v, want %v", err, UnknownAuthorityError{})
2812+
}
2813+
}

0 commit comments

Comments
 (0)