Support low-order, on-twist, and non-canonical x25519 keys#167
Open
mamckee wants to merge 15 commits into
Open
Support low-order, on-twist, and non-canonical x25519 keys#167mamckee wants to merge 15 commits into
mamckee wants to merge 15 commits into
Conversation
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>
There was a problem hiding this comment.
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_FIPSon X25519SymCryptEckeySetValuecalls to avoid subgroup/FIPS checks during import/dup. - Add
p_scossl_x25519_canonicalize_public_keyand 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.
There was a problem hiding this comment.
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
reviewed
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SymCrypt provider does not currently support two groups of less common x25519 keys.
SymCryptEcpointSetValuevalidates prime-order subgroup membership ([GOrd]*P == O) by default, which rejects these keys. OpenSSL does not check this any permits these keys.SYMCRYPT_FLAG_KEY_NO_FIPStoSymCryptEcpointSetValuecalls for x25519p_scossl_x25519_canonicalize_public_keyto canonicalize non-canonical public key values before sending them to SymCrypt