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

kem/hybrid: ensure X25519 hybrids fails with low order points #541

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

Lekensteyn
Copy link
Contributor

@Lekensteyn Lekensteyn commented Mar 20, 2025

X25519 public keys with low order points result in an all-zeroes shared secret. Ensure that hybrids with X25519 and X448 fail during Encapsulate and Decapsulate when the peer provides such a garbage public key.


I discovered this while rebasing our Go fork on Go 1.24 which upgraded the BoGo test suite as part of implementing X25519MLKEM768: golang/go@4b7f7cd#diff-db7320adf5b82005863ef324a30e91235d0268afe79856722226022f3afc2611

That upgrade pulled in this change that adds extra tests for key shares: google/boringssl@ed95627

Without this fix, these crypto/tls tests would fail as the TLS handshake continues instead of aborting:

TestBogoSuite/CurveTest-Invalid-LowOrderX25519Point-Server-MLKEM-TLS13
TestBogoSuite/CurveTest-Invalid-LowOrderX25519Point-Client-MLKEM-TLS13

I don't think this is a security issue, garbage in, garbage out. Nevertheless, it doesn't hurt to add the extra checks.

@Lekensteyn
Copy link
Contributor Author

This PR currently rejects bad cases after the ECDH calculation.

This could also be fixed by rejecting public keys with low-order points in UnmarshalBinaryPublicKey, consistent with how bad cases for NIST curves are prevented. This could require a new NewPublicKey() (*Key, error) method in the dh/x25519 and dh/x448 CIRCL packages.

Any particular preference?

@Lekensteyn Lekensteyn force-pushed the pwu/kem-check-low-order-x25519-point branch from 40a65c5 to e315ae0 Compare March 21, 2025 09:07
X25519 public keys with low order points result in an all-zeroes shared
secret. Ensure that hybrids with X25519 and X448 fail during Encapsulate
and Decapsulate when the peer provides such a garbage public key.
@Lekensteyn Lekensteyn force-pushed the pwu/kem-check-low-order-x25519-point branch from e315ae0 to 3f06680 Compare March 21, 2025 09:52
@bwesterb
Copy link
Member

Any particular preference?

You can't hit the case unless you try, so hitting the error late is not a problem.

Copy link

@lukevalenta lukevalenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I agree this isn't a security concern, but doesn't hurt to reject.

@Lekensteyn Lekensteyn merged commit 5f64bbd into main Mar 21, 2025
9 of 10 checks passed
@Lekensteyn Lekensteyn deleted the pwu/kem-check-low-order-x25519-point branch March 21, 2025 15:21
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