Skip to content

ML-KEM/ML-DSA: harden against fault attacks#9734

Open
SparkiDev wants to merge 1 commit intowolfSSL:masterfrom
SparkiDev:mlkem_mldsa_harden
Open

ML-KEM/ML-DSA: harden against fault attacks#9734
SparkiDev wants to merge 1 commit intowolfSSL:masterfrom
SparkiDev:mlkem_mldsa_harden

Conversation

@SparkiDev
Copy link
Contributor

Description

ML-DSA: check pointer to the y parameter has not be faulted.
ML-KEM: to harden against faultiong, use a different buffer for private seed, sigma, and add a check that the buffer was copied correctly.
SHA-3: fix size of check variables.

Fixes zd#21108

Testing

./configure --disable-shared --enable-mlkem --enable-mldsa --enable-faultharden

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

ML-DSA: check pointer to the y parameter has not be faulted.
ML-KEM: to harden against faultiong, use a different buffer for private
seed, sigma, and add a check that the buffer was copied correctly.
SHA-3: fix size of check variables.
@SparkiDev SparkiDev self-assigned this Feb 3, 2026
@SparkiDev
Copy link
Contributor Author

Jenkins retest this please

Nodes went down.

@SparkiDev SparkiDev assigned wolfSSL-Bot and unassigned SparkiDev Mar 2, 2026
@SparkiDev SparkiDev requested a review from wolfSSL-Bot March 2, 2026 11:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens post-quantum ML-KEM (Kyber) and ML-DSA (Dilithium) implementations against fault-injection attacks and updates SHA-3 fault-hardening checks, controlled via the --enable-faultharden configure option.

Changes:

  • ML-KEM: use a separate sigma buffer under fault hardening and validate the copy.
  • ML-DSA: add pointer-integrity checks to detect faulted y/yt pointers during signing.
  • SHA-3: rename/standardize the fault-hardening macro and widen check counters to avoid overflow.

Reviewed changes

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

File Description
wolfcrypt/src/wc_mlkem.c Adds WC_MLKEM_FAULT_HARDEN path with separate sigma buffer and verification checks.
wolfcrypt/src/sha3.c Renames hardening macro to WC_SHA3_FAULT_HARDEN and widens check variables used for fault detection.
wolfcrypt/src/dilithium.c Adds WC_MLDSA_FAULT_HARDEN pointer-integrity checks during signing to detect faulted pointers.
configure.ac Extends --enable-faultharden to define the new SHA-3/ML-KEM/ML-DSA fault-hardening macros.

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

Comment on lines +598 to 601
#elif defined(WC_SHA3_FAULT_HARDEN)
static WC_INLINE word64 Load64Unaligned(const unsigned char *a)
{
#ifdef WC_64BIT_CPU
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Within this WC_SHA3_FAULT_HARDEN Load64Unaligned() implementation, the WC_32BIT_CPU / WC_16BIT_CPU branches use the logical OR operator (||) when combining shifted words. That will yield 0/1 instead of a proper bitwise combination and breaks SHA-3 output on those targets. Use bitwise OR (|) when composing the 64-bit return value.

Copilot uses AI. Check for mistakes.
#ifdef WC_MLDSA_FAULT_HARDEN
if (y_check != y) {
valid = 0;
ret = BAD_COND_E;
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

When WC_MLDSA_FAULT_HARDEN detects y_check != y, it sets ret = BAD_COND_E but then continues executing the signing steps in this iteration. Because later code assigns to ret (e.g., hashing/allocations), this fault-detection error can be overwritten and the function may continue as if nothing happened. After setting BAD_COND_E here, exit the rejection loop immediately (e.g., break/return/goto cleanup) and ensure subsequent code can’t reset ret back to 0.

Suggested change
ret = BAD_COND_E;
ret = BAD_COND_E;
break;

Copilot uses AI. Check for mistakes.
Comment on lines +575 to +578
if (XMEMCMP(sigma, rho, WC_ML_KEM_SYM_SZ) == 0) {
ret = BAD_COND_E;
}
if (XMEMCMP(sigma, rho + WC_ML_KEM_SYM_SZ, WC_ML_KEM_SYM_SZ) != 0) {
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The WC_MLKEM_FAULT_HARDEN copy/verification uses rho (a pointer variable) as the base for both comparisons. If the goal is to detect pointer-faulting, relying on rho here weakens the check because faulting rho could also fault the verification inputs. Consider using buf directly (e.g., compare against buf and buf + WC_ML_KEM_SYM_SZ) so the verification doesn’t depend on a potentially faulted pointer variable.

Suggested change
if (XMEMCMP(sigma, rho, WC_ML_KEM_SYM_SZ) == 0) {
ret = BAD_COND_E;
}
if (XMEMCMP(sigma, rho + WC_ML_KEM_SYM_SZ, WC_ML_KEM_SYM_SZ) != 0) {
if (XMEMCMP(sigma, buf, WC_ML_KEM_SYM_SZ) == 0) {
ret = BAD_COND_E;
}
if (XMEMCMP(sigma, buf + WC_ML_KEM_SYM_SZ, WC_ML_KEM_SYM_SZ) != 0) {

Copilot uses AI. Check for mistakes.
AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_CHECK_SIG_FAULTS -DWOLFSSL_CHECK_VER_FAULTS -DWC_SHA3_HARDEN"
AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_CHECK_SIG_FAULTS -DWOLFSSL_CHECK_VER_FAULTS -DWC_SHA3_FAULT_HARDEN -DWC_MLKEM_FAULT_HARDEN -DWC_MLDSA_FAULT_HARDEN"
fi

Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This change adds new compile-time fault-hardening code paths for SHA-3, ML-KEM, and ML-DSA behind --enable-faultharden. Since these paths are only compiled/executed with this option, it would be good to add a CI job (or test target) that builds with --enable-faultharden and runs the existing SHA-3/ML-KEM/ML-DSA test suites to prevent regressions.

Suggested change
AM_CONDITIONAL([ENABLE_FAULTHARDEN], [test "$ENABLED_FAULTHARDEN" = "yes"])

Copilot uses AI. Check for mistakes.
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