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

20250106-_DhSetKey-FFDHE-short-circuit #8335

Merged

Conversation

douzzer
Copy link
Contributor

@douzzer douzzer commented Jan 6, 2025

wolfcrypt/src/dh.c: in _DhSetKey(), add short-circuit comparisons to RFC 7919 known-good moduli, preempting overhead from mp_prime_is_prime().

wolfcrypt/test/test.c: in dh_ffdhe_test(), when defined(HAVE_PUBLIC_FFDHE), use wc_DhSetKey_ex() rather than wc_DhSetKey() to exercise the primality check in _DhSetKey().

see ZD#18507

tested with wolfssl-multi-test.sh ... check-source-text fips-140-2-optest fips-140-3-dev-all plus enable-all builds with CPPFLAGS=-DHAVE_PUBLIC_FFDHE.

…RFC 7919 known-good moduli, preempting overhead from mp_prime_is_prime().

wolfcrypt/test/test.c: in dh_ffdhe_test(), when defined(HAVE_PUBLIC_FFDHE), use wc_DhSetKey_ex() rather than wc_DhSetKey() to exercise the primality check in _DhSetKey().
@SparkiDev
Copy link
Contributor

There is a trusted parameter that can be used if the prime is known to be good.
Otherwise, yeah we can do these checks as they are quick compared to the overall time of checking a prime.

@douzzer
Copy link
Contributor Author

douzzer commented Jan 7, 2025

Right, the optimization is for use cases like in ZD#18507 where DH params are read from a file, and may be a standard prime, but can't be counted on to be.

@douzzer
Copy link
Contributor Author

douzzer commented Jan 7, 2025

retest this please (Windows worker node glitch)

@douzzer douzzer assigned wolfSSL-Bot and unassigned douzzer Jan 7, 2025
@dgarske dgarske merged commit 4a12351 into wolfSSL:master Jan 7, 2025
151 checks passed
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