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.

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

Validate ECDH keys #226

Merged
merged 12 commits into from
Dec 10, 2024
Merged

Validate ECDH keys #226

merged 12 commits into from
Dec 10, 2024

Conversation

qmuntal
Copy link
Collaborator

@qmuntal qmuntal commented Nov 25, 2024

Go 1.24 removed some ECDH key checks from the crypto/ecdh package and moved them to crypto/internal/fips/ecdh: golang/go@566cf1c#diff-e9f4458b20e24935551d73dc6b34e4007d91ca35e1e2c5b4d1e9f2a738d04e9cL79.

This means that the checks are no longer run in the the crypto backend layer, so we must instruct OpenSSL to perform that checks.

golang/go@566cf1c also refactored the ECDH implementation so that ecdh.PrivateKey.Public is no longer lazy loaded, but generated when ecdh.NewPrivateKey is called. Following suite here, as it simplifies our implementation and makes key validation easier.

@qmuntal qmuntal marked this pull request as ready for review November 26, 2024 10:12
ecdh.go Show resolved Hide resolved
evp.go Outdated Show resolved Hide resolved
@dagood dagood self-requested a review November 26, 2024 18:22
Co-authored-by: Davis Goodin <dagood@users.noreply.github.com>
@qmuntal
Copy link
Collaborator Author

qmuntal commented Dec 4, 2024

@derekparker the fix in this PR is blocking our upstream sync process. Could you take a look? Thanks.

@qmuntal
Copy link
Collaborator Author

qmuntal commented Dec 9, 2024

For microsoft/go#1416.

@qmuntal qmuntal merged commit bdc3592 into v2 Dec 10, 2024
54 checks passed
@qmuntal qmuntal deleted the ecdhcheck branch December 10, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants