Document that raw ECC import does not validate the point - #11099
Open
MarkAtwood wants to merge 1 commit into
Open
Document that raw ECC import does not validate the point#11099MarkAtwood wants to merge 1 commit into
MarkAtwood wants to merge 1 commit into
Conversation
wc_ecc_import_raw, wc_ecc_import_raw_ex and wc_ecc_import_unsigned all reach wc_ecc_import_raw_private, which only checks that the imported public point lies on the curve when WOLFSSL_VALIDATE_ECC_IMPORT is defined. That flag is off in default builds and set in FIPS builds. The x963 and ASN.1 import paths validate unconditionally, so the raw importers are the one public entry point where a caller has to know to validate. Nothing in the name or the existing documentation said so. Also corrects the brief for wc_ecc_import_raw, which described the function as filling a key with the components of a signature.
MarkAtwood
requested review from
JacobBarthelmeh
and
a lite review from Copilot
August 6, 2026 18:34
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.
Description
wc_ecc_import_raw,wc_ecc_import_raw_exandwc_ecc_import_unsignedall reachwc_ecc_import_raw_private, which validates that the imported public point lies on the curve only whenWOLFSSL_VALIDATE_ECC_IMPORTis defined (wolfcrypt/src/ecc.c). That flag is off in default builds and set in FIPS builds.The x963 and ASN.1 import paths validate unconditionally, so the raw importers are the one public entry point where the caller has to know to validate. Neither the function names nor the existing documentation said so. This adds a
\noteto each of the three saying the check is not performed, and pointing at both ways to get it.Documentation only. No functional change, no behavior change, no ABI change.
This is a documentation change rather than a code change on purpose. Raw import is trusted-input-only by design, which is the intended contract; the gap was that the contract was not written down anywhere a caller would see it.
Pre-existing issue corrected in the same block
The
\briefforwc_ecc_import_rawdescribed the function as filling anecc_keywith "the raw components of an ECC signature". It fills it with the components of a key. Corrected here since it is the same doc block. Happy to drop that hunk if you would rather keep this to a single concern.Testing
Documentation only, so no test changes. The touched file is
doc/dox_comments/header_files/ecc.h, which is Doxygen input and is not compiled.Checklist