Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

crypto/x509: Verify panics on certificates with an unknown public key algorithm [CVE-2024-24783] #65390

Closed
neild opened this issue Jan 30, 2024 · 5 comments
Closed

crypto/x509: Verify panics on certificates with an unknown public key algorithm [CVE-2024-24783] #65390

neild opened this issue Jan 30, 2024 · 5 comments
Assignees
@neild
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone
Go1.23

Comments

@neild
Copy link
Contributor

neild commented Jan 30, 2024 •
edited by mknyszek
Loading

Verifying a certificate chain which contains a certificate with an unknown public
key algorithm will cause Certificate.Verify to panic.

This affects all crypto/tls clients, and servers that set Config.ClientAuth to
VerifyClientCertIfGiven or RequireAndVerifyClientCert. The default behavior is
for TLS servers to not verify client certificates.

Thanks to John Howard (Google) for reporting this issue. This is CVE-2024-24783.

/cc @golang/security and @golang/release
@neild neild added this to the Go1.22 milestone Jan 30, 2024
@neild
Copy link
Contributor Author

neild commented Jan 30, 2024

@gopherbot please open backport issues for this security fix.

@gopherbot gopherbot mentioned this issue Jan 30, 2024
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #65391 (for 1.20), #65392 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Jan 30, 2024
Closed
@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 31, 2024
@mknyszek mknyszek modified the milestones: Go1.22, Go1.23 Feb 2, 2024
@neild neild self-assigned this Feb 21, 2024
@neild neild mentioned this issue Feb 21, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569235 mentions this issue: [release-branch.go1.22] crypto/x509: make sure pub key is non-nil before interface conversion

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569238 mentions this issue: [release-branch.go1.21] crypto/x509: make sure pub key is non-nil before interface conversion

gopherbot pushed a commit that referenced this issue Mar 5, 2024
@rolandshoemaker @gopherbot
[release-branch.go1.22] crypto/x509: make sure pub key is non-nil bef…
337b8e9 
…ore 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 #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 <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/+/2174343
Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
gopherbot pushed a commit that referenced this issue Mar 5, 2024
@rolandshoemaker @gopherbot
[release-branch.go1.21] crypto/x509: make sure pub key is non-nil bef…
be5b52b 
…ore 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>
@mknyszek mknyszek changed the title security: fix CVE-2024-24783 crypto/x509: Verify panics on certificates with an unknown public key algorithm [CVE-2024-24783] Mar 5, 2024
bradfitz pushed a commit to tailscale/go that referenced this issue Mar 5, 2024
@rolandshoemaker @bradfitz
[release-branch.go1.22] crypto/x509: make sure pub key is non-nil bef…
4da1f86 
…ore 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 <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/+/2174343
Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569339 mentions this issue: crypto/x509: make sure pub key is non-nil before interface conversion

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 6, 2024
@rolandshoemaker @romaindoumenc
[release-branch.go1.22] crypto/x509: make sure pub key is non-nil bef…
b66cd86 
…ore 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 <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/+/2174343
Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@rolandshoemaker rolandshoemaker mentioned this issue Mar 8, 2024
Open
@arisazimli arisazimli mentioned this issue Aug 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neild neild

Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Projects
Release Blockers
Status: Done
Milestone
Go1.23
Development

No branches or pull requests
3 participants
@neild @mknyszek @gopherbot