Conversation
ZD#21457 (30)
There was a problem hiding this comment.
Pull request overview
Security and correctness fixes across PKCS#12 verification, OpenSSL-compat EVP HMAC verification, and ARMv8 ML-KEM NEON compare logic, with new regression tests to prevent bypasses.
Changes:
- Harden PKCS#12 MAC verification by rejecting MACs whose encoded digest length doesn’t match the algorithm’s digest size.
- Tighten EVP HMAC
DigestVerifyFinalhandling of signature lengths and add a regression test for the zero-length-tag forgery case. - Fix ML-KEM NEON compare reduction logic on AArch64 and add a decapsulation tamper/FO-rejection test.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wolfcrypt/src/port/arm/armv8-mlkem-asm.S | Adjusts NEON reduction step in mlkem_cmp_neon to fold upper 64-bits into lower before scalar check. |
| wolfcrypt/src/port/arm/armv8-mlkem-asm_c.c | Mirrors the NEON reduction fix in the inline-asm C implementation. |
| wolfcrypt/src/pkcs12.c | Rejects PKCS#12 MAC verification when the computed digest size doesn’t match the parsed digest length (prevents truncated-MAC bypass). |
| wolfcrypt/src/evp.c | Changes HMAC DigestVerifyFinal length validation and removes truncated-tag acceptance. |
| tests/api/test_pkcs12.h | Registers new PKCS#12 truncated-MAC bypass regression test. |
| tests/api/test_pkcs12.c | Adds crafted PKCS#12 fixture test ensuring truncated MAC digest is rejected. |
| tests/api/test_mlkem.h | Registers new ML-KEM FO rejection test. |
| tests/api/test_mlkem.c | Adds ciphertext-tamper test to ensure FO re-encryption check triggers implicit rejection. |
| tests/api/test_evp_pkey.h | Registers new EVP HMAC zero-length verification regression test. |
| tests/api/test_evp_pkey.c | Adds test ensuring EVP_DigestVerifyFinal rejects zero-length HMAC tags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hashLen = wolfssl_mac_len(ctx->hash.hmac.macType); | ||
|
|
||
| if (siglen > hashLen || siglen > INT_MAX) | ||
| if (hashLen == 0 || siglen != hashLen) | ||
| return WOLFSSL_FAILURE; |
There was a problem hiding this comment.
The new HMAC length check (siglen != hashLen) changes previous behavior that allowed truncated HMAC tags (the removed comment explicitly referenced truncation). If OpenSSL-compat is the goal, consider only rejecting unsafe lengths (e.g., siglen == 0, siglen > hashLen, and siglen > INT_MAX to avoid the later (int)siglen cast) rather than requiring an exact-length tag, so callers that legitimately use truncated tags keep working while still fixing the zero-length-forgery case.
There was a problem hiding this comment.
This is correct, but at the same time accepting lengths such as 1 would be attackable via brute forcing (and anything below hashLen is exponentially worse than hashLen). OpenSSL compatibility shouldn't be an issue here because OpenSSL doesn't allow HMAC at all in EVP_DigestVerify*.
Description
Fixes ZD#21457 (27, 30, 31)
Testing