Support low-order, on-twist, and non-canonical x25519 keys by mamckee · Pull Request #167 · microsoft/SymCrypt-OpenSSL

mamckee · 2026-04-27T23:10:27Z

The SymCrypt provider does not currently support two groups of less common x25519 keys.

Low order/on-twist keys: SymCryptEcpointSetValue validates prime-order subgroup membership ([GOrd]*P == O) by default, which rejects these keys. OpenSSL does not check this any permits these keys.
Non-canonical keys: The 20 non-canonical values 2^255 - 19 through 2^255 - 1 for X25519 are rejected by SymCryptEcpointSetValue. If these are cononicalized by the SymCrypt provider (reduced modulo p) then they will be accepted/

Added SYMCRYPT_FLAG_KEY_NO_FIPS to SymCryptEcpointSetValue calls for x25519
- Allows import or low order/on-twist keys
- Increases performance of x25519 import
- x25519 isn't FIPS anyways, so we should skip this check regardless
Added helper function p_scossl_x25519_canonicalize_public_key to canonicalize non-canonical public key values before sending them to SymCrypt
Added tests for low order/on-twist keys and exhaustive tests for non-canonical keys to EVP test file

RFC 7748 accepts any 32-byte string as a public key, including low-order and on-twist points. These do not satisfy the FIPS subgroup check performed by SymCryptEckeySetValue. Since X25519 is not a FIPS algorithm, pass SYMCRYPT_FLAG_KEY_NO_FIPS to skip this check in both the import and key-dup paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

RFC 7748 section 5 specifies that the u-coordinate is decoded by masking the high bit and interpreting the result as a little-endian integer. There are 20 non-canonical encodings where the value is in [p, 2^255-1] (p = 2^255-19). These must be reduced modulo p before being passed to SymCrypt, which rejects out-of-range coordinates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add test vectors for: - Non-canonical public keys (value >= p) that must be reduced mod p and produce correct shared secrets - Low-order public keys (on curve and on twist) where derive must fail because the shared secret would be all zeros - A non-canonical encoding of a low-order point to test both canonicalization and low-order rejection together Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The p_scossl_ecc_set_encoded_key function (used by EVP_PKEY_new_raw_public_key and EVP_PKEY_new_raw_private_key) was missing the SYMCRYPT_FLAG_KEY_NO_FIPS flag for X25519 keys and was not canonicalizing X25519 public keys. This is the same class of fix applied to p_scossl_ecc_keymgmt_set_params but for the raw key import path. Changes: - Pass SYMCRYPT_FLAG_KEY_NO_FIPS for X25519 in SymCryptEckeySetValue - Allocate a mutable copy of the public key for X25519 and apply RFC 7748 section 5 canonicalization (mask high bit, reduce mod p = 2^255 - 19) - Update cleanup to free pbPublicKey unconditionally since both X25519 and non-X25519 paths now allocate it Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Extract the X25519 public key canonicalization logic (RFC 7748 section 5 high-bit masking and modular reduction) into a shared helper function scossl_x25519_canonicalize_public_key, replacing the duplicated inline implementations in p_scossl_x25519_keymgmt_import and p_scossl_ecc_set_encoded_key. Fix p_scossl_ecc_keymgmt_set_params (OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY handler) which was missing both canonicalization and the SYMCRYPT_FLAG_KEY_NO_FIPS flag for X25519 keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add complete coverage for all 19 non-canonical public key values (p+0 through p+18) that require reduction modulo p = 2^255-19. Previously only p+3 was tested. New non-canonical tests (NonCanonical-Pub-3 through Pub-18): - p+2 reduces to u=2, p+4 through p+18 reduce to u=4..18 - All derive with Alice's private key and verify expected shared secret - p+0 and p+1 (reducing to low-order u=0 and u=1) were already covered in the low-order section New low-order test: - Canonical u=0 (identity point) - was previously only tested via non-canonical encoding New high-bit (bit 255) variant tests: - Canonical non-low-order key with bit 255 set (Bob's key) - Canonical low-order key (u=0) with bit 255 set - Non-canonical key (p+2) with bit 255 set (success case) - Non-canonical low-order key (p+1) with bit 255 set (failure case) These verify bit 255 is masked before canonicalization and low-order checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <copilot@github.com>

Copilot

Pull request overview

This PR updates the SymCrypt OpenSSL provider’s X25519 handling to accept less common (but OpenSSL-permitted) public keys by skipping subgroup/FIPS validation for X25519 and canonicalizing non-canonical public-key encodings before importing into SymCrypt.

Changes:

Add SYMCRYPT_FLAG_KEY_NO_FIPS on X25519 SymCryptEckeySetValue calls to avoid subgroup/FIPS checks during import/dup.
Add p_scossl_x25519_canonicalize_public_key and apply it to X25519 public key inputs prior to SymCrypt import.
Extend EVP test recipes with coverage for non-canonical, high-bit-set, and low-order/on-twist X25519 public keys.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
SymCryptProvider/src/p_scossl_ecc.h	Introduces `SCOSSL_X25519_KEY_SIZE` and declares the new X25519 public key canonicalization helper.
SymCryptProvider/src/p_scossl_ecc.c	Implements canonicalization and updates X25519 import/dup code paths to use `SYMCRYPT_FLAG_KEY_NO_FIPS` and canonicalization.
SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c	Canonicalizes X25519 public keys and enforces 32-byte key lengths in set/import paths; uses `SYMCRYPT_FLAG_KEY_NO_FIPS` for X25519.
EvpTestRecipes/3.0/evppkey_ecx.txt	Adds extensive EVP test vectors for non-canonical and low-order/on-twist X25519 keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vcsjones · 2026-04-29T16:57:55Z


+            if (keyCtx->isX25519)
+            {
+                flags |= SYMCRYPT_FLAG_KEY_NO_FIPS;


One thing I observed from enabling this is this will permit SymCrypt to produce "all zero" shared secrets when allowing low-order public keys. Most other X25519 implementations will refuse to give back the all zero shared secret (RFC 7748 recommends it)

e.g. Wycheproof test case 64.

The way we handled this on the .NET side is to do a secure compare of the produced shared secret with the all-zero key and not permit it.

Do you have a repro? SymCrypt is supposed to reject all zero secrets, and I saw rejections when testing: https://github.com/microsoft/SymCrypt/blob/2864ff0da96642049a66808d86a8bad2736b85e8/lib/ec_dh.c#L129.

One of the test cases I added is a low-order point from wycheproof that gets rejected.

Hm, could be a bcrypt specific behavior. Feel free to resolve this one then and will take bcrypt offline.

https://github.com/microsoft/SymCrypt/blob/2864ff0da96642049a66808d86a8bad2736b85e8/lib/ec_dh.c#L129
does not reject an all zero shared secret, it rejects a shared secret which is the identity point (aka the point at infinity).

In the X/Z single projective coordinates of X25519, that is a check for Z being 0.
The shared secret from ECDH is the X coordinate when you transform back into affine coordinates (actually do the modular division X -> X/Z).

The check for all-zero secret seems to be a check to reject the use of low order public keys at secret agreement time:
https://www.rfc-editor.org/rfc/rfc7748#section-6.1
"The check for the all-zero value results from the fact that the
X25519 function produces that value if it operates on an input
corresponding to a point with small order, where the order divides
the cofactor of the curve (see Section 7). The check may be
performed by ORing all the bytes together and checking whether the
result is zero, as this eliminates standard side-channels in software
implementations."
These public key were previously rejected at public key import time with the FIPS checks, but now we're going to allow them to be imported but fail every attempt to do secret agreement with them?
It seems super wonky for .NET to be complaining that we were failing too fast.

Given the RFC documentation recommending we should reject these secret agreement values, we should probably add that test to X25519 in SymCrypt when FIPS checks are disabled to align with RFC expectations.
There is a clear performance benefit to not doing the public key order validation on public key import for the expected case, and instead checking at secret agreement time.

Recommend zero shared secret check to go into SCOSSL in this PR, and move the check down into SymCrypt as a follow up.

These public key were previously rejected at public key import time with the FIPS checks, but now we're going to allow them to be imported but fail every attempt to do secret agreement with them?
It seems super wonky for .NET to be complaining that we were failing too fast.

It's not just about failing too fast. In the case of low order it is failing too fast, but on-twist does not fail and should be permitted. It just happens that the FIPS check gates several things.

but fail every attempt to do secret agreement with them?

I don't think every attempt will fail. For example, Wycheproof test case 4 is on-twist and does not produce an all-zero shared secret.

You're right, there are several classes of X25519 point, and I oversimplified.

The FIPS public key order check rejects all but the prime-order subgroup, which are the only public keys you should expect from a peer which has performed keygen correctly and has managed to send it to you without corruption (which is why we haven't got a lot of complaints about being overly restrictive to date)

In the absence of the FIPS check at public key import time, we permit low order and on-twist points
My understanding is that low order points should always produce an all-zero shared secret
On-twist points will map to on-twist results which are high entropy (and the malicious/faulty peer will not be able to generate the same result so the E2E handshake would fail later)

Net net, removing FIPS checks can defer failure with bad public key to later in the process, reducing cost of the expected success path for good public keys
It is reasonable to reject 0 shared secrets to avoid games with a malicious peer guaranteeing you negotiate a zero-security shared secret (although a malicious peer could just publish the shared secret if they were so inclined, so it's probably not a huge deal)

mamckee and others added 13 commits April 24, 2026 15:51

Cleanup

688008e

Add key length checks for x25519

c9509ce

Fix size check

63f7e15

Rename function

83d11e4

Re-order non-canonical tests

2a833ac

Clean up test comments

66649d7

Cleanup

ed60c61

Co-authored-by: Copilot <copilot@github.com>

mamckee requested a review from Copilot April 27, 2026 23:26

Copilot started reviewing on behalf of mamckee April 27, 2026 23:27 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread SymCryptProvider/src/p_scossl_ecc.c Outdated

Use correct variable for malloc

768f4c5

mamckee requested a review from Copilot April 27, 2026 23:57

Copilot started reviewing on behalf of mamckee April 28, 2026 00:00 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

mamckee marked this pull request as ready for review April 28, 2026 00:09

mamckee requested a review from samuel-lee-msft April 28, 2026 00:10

vcsjones reviewed Apr 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support low-order, on-twist, and non-canonical x25519 keys#167

Support low-order, on-twist, and non-canonical x25519 keys#167
mamckee wants to merge 14 commits intomainfrom
mamckee-x25519-edge-case-keys

mamckee commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

vcsjones Apr 29, 2026 •

edited

Loading

Uh oh!

mamckee Apr 30, 2026

Uh oh!

vcsjones Apr 30, 2026

Uh oh!

samuel-lee-msft Apr 30, 2026

Uh oh!

samuel-lee-msft Apr 30, 2026

Uh oh!

vcsjones Apr 30, 2026

Uh oh!

vcsjones Apr 30, 2026

Uh oh!

samuel-lee-msft May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mamckee commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

vcsjones Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mamckee Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

vcsjones Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

samuel-lee-msft Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

samuel-lee-msft Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

vcsjones Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

vcsjones Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

samuel-lee-msft May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vcsjones Apr 29, 2026 •

edited

Loading