From bf8efec3e2b86f0824e77894b93de9ca1ac32ea6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 27 Aug 2026 12:41:12 -0700 Subject: [PATCH] fix: compare RSA blocks instead of parsing RFC 8332 section 3 advises against verifying RSASSA-PKCS1-v1_5 by applying the key and parsing the output. wolfSSH_RsaVerify() builds the expected EMSA-PKCS1-v1_5 block, applies the key with a raw public operation, and compares whole modulus-sized blocks. - verify through wc_RsaFunction() and a constant-time block compare - reject a signature not less than the modulus, which the raw operation would otherwise reduce into a second encoding of the same signature - reject a key too small for the digest, and a wc_RsaEncryptSize() error - carve the four working blocks from one allocation - add test_RsaVerify_BadPadding() and test_RsaVerify_SigRange() Offload builds leave the modulus empty, so the range check is skipped there. That check walks the bytes itself rather than calling memcmp(), which some libcs compare as signed char, ordering the modulus's high byte backwards. The padding cases pass against the old code as well; the range case does not. Issue: F-10574 --- src/internal.c | 115 ++++++++++++++------ tests/unit.c | 284 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 368 insertions(+), 31 deletions(-) diff --git a/src/internal.c b/src/internal.c index fdaf71639..8419133ff 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14676,6 +14676,21 @@ static INLINE byte SigTypeForId(byte id) #ifndef WOLFSSH_NO_RSA +/* Big-endian magnitude compare. Not memcmp(): Zephyr's minimal libc walks it + * as signed char, which orders a modulus's high byte backwards. */ +static int BigEndianCompare(const byte* a, const byte* b, word32 sz) +{ + word32 idx; + + for (idx = 0; idx < sz; idx++) { + if (a[idx] != b[idx]) { + return (a[idx] > b[idx]) ? 1 : -1; + } + } + return 0; +} + + /* * wolfSSH_RsaVerify * sig - signature to verify @@ -14686,43 +14701,54 @@ static INLINE byte SigTypeForId(byte id) * heap - allocation heap * loc - calling function for logging * - * Takes the provided digest of type digestId and converts it to an - * encoded digest. Then verifies the signature, comparing the output - * digest and compares it. + * Rebuilds the expected EMSA-PKCS1-v1_5 block from encDigest, applies the + * public key to the signature, and compares the two blocks whole. Per RFC + * 8332 section 3, the recovered value is never parsed. */ int wolfSSH_RsaVerify(const byte *sig, word32 sigSz, const byte* encDigest, word32 encDigestSz, RsaKey* key, void* heap, const char* loc) { + byte* scratch = NULL; byte* checkSig = NULL; - int checkDigestSz; + byte* checkBlock = NULL; + byte* encBlock = NULL; + byte* nBlock = NULL; word32 keySz; + int encryptSz; int ret = WS_SUCCESS; -#ifdef WOLFSSH_SMALL_STACK - byte* checkDigest = NULL; -#else - byte checkDigest[MAX_ENCODED_SIG_SZ]; -#endif if (sig == NULL) { WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Missing RSA signature"); return WS_RSA_E; } - keySz = (word32)wc_RsaEncryptSize(key); + encryptSz = wc_RsaEncryptSize(key); + if (encryptSz <= 0) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA key size"); + return WS_RSA_E; + } + keySz = (word32)encryptSz; - if (ret == WS_SUCCESS) { - checkSig = (byte*)WMALLOC(keySz, heap, DYNTYPE_TEMP); - if (checkSig == NULL) - ret = WS_MEMORY_E; + /* Room for the header, the trailing zero, and eight pad bytes. */ + if (keySz < encDigestSz + RSA_MIN_PAD_SZ) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "RSA key too small for the digest"); + return WS_RSA_E; } -#ifdef WOLFSSH_SMALL_STACK + + /* One block each: normalized signature, recovered value, expected + * encoding, modulus. */ if (ret == WS_SUCCESS) { - checkDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, heap, DYNTYPE_TEMP); - if (checkDigest == NULL) + scratch = (byte*)WMALLOC(keySz * 4, heap, DYNTYPE_TEMP); + if (scratch == NULL) ret = WS_MEMORY_E; + else { + checkSig = scratch; + checkBlock = scratch + keySz; + encBlock = scratch + (keySz * 2); + nBlock = scratch + (keySz * 3); + } } -#endif /* Normalize the peer's signature. Some SSH implementations remove * leading zeros on the signatures they encode. We need to pad the @@ -14742,28 +14768,55 @@ int wolfSSH_RsaVerify(const byte *sig, word32 sigSz, WMEMCPY(checkSig + offset, sig, sigSz); } + /* SP 800-56B wants 1 < s < n-1. wc_RsaFunction() only checks that for a + * decrypt, and the modexp would reduce s mod n, so s + n verifies as a + * second encoding of the same signature. Offload builds leave n empty and + * have nothing to compare against. */ + if (ret == WS_SUCCESS && mp_count_bits(&key->n) != 0) { + word32 eSz = keySz, nSz = keySz; + + /* checkBlock is scratch until the modexp below fills it. */ + if (wc_RsaFlattenPublicKey(key, checkBlock, &eSz, nBlock, &nSz) != 0 + || nSz != keySz) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA public key"); + ret = WS_RSA_E; + } + else if (BigEndianCompare(checkSig, nBlock, keySz) >= 0) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "RSA signature out of range"); + ret = WS_RSA_E; + } + } + + /* Expected block: 0x00 0x01 || 0xFF pad || 0x00 || DigestInfo. */ if (ret == WS_SUCCESS) { - volatile int sizeCompare; - volatile int compare; + word32 padSz = keySz - encDigestSz - 3; + word32 idx = 0; + + encBlock[idx++] = 0x00; + encBlock[idx++] = 0x01; + WMEMSET(encBlock + idx, 0xFF, padSz); + idx += padSz; + encBlock[idx++] = 0x00; + WMEMCPY(encBlock + idx, encDigest, encDigestSz); + } - checkDigestSz = wc_RsaSSL_Verify(checkSig, keySz, - checkDigest, MAX_ENCODED_SIG_SZ, key); + /* RFC 8332 section 3: compare whole blocks, don't parse. */ + if (ret == WS_SUCCESS) { + word32 checkBlockSz = keySz; + int rsaRet; - sizeCompare = checkDigestSz > 0 && encDigestSz != (word32)checkDigestSz; - compare = ConstantCompare(encDigest, checkDigest, encDigestSz); + rsaRet = wc_RsaFunction(checkSig, keySz, checkBlock, &checkBlockSz, + RSA_PUBLIC_ENCRYPT, key, NULL); - if (checkDigestSz < 0 || sizeCompare || compare) { + if (rsaRet != 0 || checkBlockSz != keySz + || ConstantCompare(checkBlock, encBlock, keySz) != 0) { WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA Verify"); ret = WS_RSA_E; } } -#ifdef WOLFSSH_SMALL_STACK - if (checkDigest) - WFREE(checkDigest, heap, DYNTYPE_TEMP); -#endif - if (checkSig) - WFREE(checkSig, heap, DYNTYPE_TEMP); + if (scratch) + WFREE(scratch, heap, DYNTYPE_TEMP); WOLFSSH_UNUSED(loc); /* Unused when WLOG is not defined */ return ret; } diff --git a/tests/unit.c b/tests/unit.c index 489bc04bf..97b434119 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -9394,6 +9394,280 @@ static int test_RsaVerify_BadDigest(void) return result; } + +/* wolfSSH_RsaVerify compares whole blocks, so a signature over malformed + * EMSA-PKCS1-v1_5 padding must be refused. The blocks are signed with a raw + * private-key operation, since normal signing would not produce them. The + * control case guards against a broken harness passing everything. */ +static int test_RsaVerify_BadPadding(void) +{ + int result = 0; + int ret; + RsaKey key; + WC_RNG rng; + word32 idx = 0; + word32 keySz; + word32 padSz; + byte data[32]; + byte digest[WC_SHA256_DIGEST_SIZE]; + byte encDigest[MAX_ENCODED_SIG_SZ]; + int encDigestSz; + byte block[512]; + byte sig[512]; + word32 sigSz; + int encryptSz; + int keyInit = 0, rngInit = 0; + + WMEMSET(data, 0x42, sizeof(data)); + + if (wc_InitRng(&rng) != 0) { + printf("RsaVerify_BadPadding: wc_InitRng failed\n"); + return -520; + } + rngInit = 1; + if (wc_InitRsaKey(&key, NULL) != 0) { + printf("RsaVerify_BadPadding: wc_InitRsaKey failed\n"); + result = -521; + goto done; + } + keyInit = 1; + + ret = wc_RsaPrivateKeyDecode(unitTestRsaPrivKey, &idx, &key, + unitTestRsaPrivKeySz); + if (ret != 0) { result = -522; goto done; } + + encryptSz = wc_RsaEncryptSize(&key); + if (encryptSz <= 0 || (word32)encryptSz > sizeof(block)) { + result = -523; + goto done; + } + keySz = (word32)encryptSz; + + ret = wc_Hash(WC_HASH_TYPE_SHA256, data, sizeof(data), + digest, WC_SHA256_DIGEST_SIZE); + if (ret != 0) { result = -524; goto done; } + + encDigestSz = wc_EncodeSignature(encDigest, digest, + WC_SHA256_DIGEST_SIZE, wc_HashGetOID(WC_HASH_TYPE_SHA256)); + if (encDigestSz <= 0) { result = -525; goto done; } + + /* The fixed test key has room; a smaller one would underflow padSz. */ + if (keySz < (word32)encDigestSz + RSA_MIN_PAD_SZ) { + result = -532; + goto done; + } + padSz = keySz - (word32)encDigestSz - 3; + + /* Control: a well-formed block must verify. */ + block[0] = 0x00; + block[1] = 0x01; + WMEMSET(block + 2, 0xFF, padSz); + block[2 + padSz] = 0x00; + WMEMCPY(block + 3 + padSz, encDigest, (word32)encDigestSz); + + sigSz = (word32)sizeof(sig); + ret = wc_RsaFunction(block, keySz, sig, &sigSz, RSA_PRIVATE_DECRYPT, + &key, &rng); + if (ret != 0) { result = -526; goto done; } + + ret = wolfSSH_TestRsaVerify(sig, sigSz, encDigest, (word32)encDigestSz, + &key, NULL); + if (ret != WS_SUCCESS) { + printf("RsaVerify_BadPadding: well-formed block ret=%d expected %d\n", + ret, WS_SUCCESS); + result = -527; + goto done; + } + + /* A pad byte that is not 0xFF. */ + block[2 + (padSz / 2)] = 0xFE; + sigSz = (word32)sizeof(sig); + ret = wc_RsaFunction(block, keySz, sig, &sigSz, RSA_PRIVATE_DECRYPT, + &key, &rng); + if (ret != 0) { result = -528; goto done; } + + ret = wolfSSH_TestRsaVerify(sig, sigSz, encDigest, (word32)encDigestSz, + &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_BadPadding: bad pad byte ret=%d expected %d\n", + ret, WS_RSA_E); + result = -529; + goto done; + } + + /* A wrong block type. */ + block[2 + (padSz / 2)] = 0xFF; + block[1] = 0x02; + sigSz = (word32)sizeof(sig); + ret = wc_RsaFunction(block, keySz, sig, &sigSz, RSA_PRIVATE_DECRYPT, + &key, &rng); + if (ret != 0) { result = -530; goto done; } + + ret = wolfSSH_TestRsaVerify(sig, sigSz, encDigest, (word32)encDigestSz, + &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_BadPadding: bad block type ret=%d expected %d\n", + ret, WS_RSA_E); + result = -531; + } + +done: + if (rngInit) + wc_FreeRng(&rng); + if (keyInit) + wc_FreeRsaKey(&key); + return result; +} + + +/* A signature is only valid in 0 <= s < n. The raw public operation reduces + * mod n on its own, so s + n recovers the same block and would verify without + * an explicit range check. Also covers the two key-size guards. */ +static int test_RsaVerify_SigRange(void) +{ + int result = 0; + int ret; + RsaKey key; + RsaKey emptyKey; + WC_RNG rng; + word32 idx = 0; + word32 keySz; + word32 eSz, nSz; + byte data[32]; + byte digest[WC_SHA256_DIGEST_SIZE]; + byte encDigest[MAX_ENCODED_SIG_SZ]; + int encDigestSz = 0; + byte bigDigest[512]; + byte nBuf[512]; + byte eBuf[512]; + byte sig[512]; + byte sum[513]; + word32 sigSz = 0; + int encryptSz; + int attempt; + int carry = 1; + int keyInit = 0, rngInit = 0, emptyInit = 0; + + if (wc_InitRng(&rng) != 0) { + printf("RsaVerify_SigRange: wc_InitRng failed\n"); + return -540; + } + rngInit = 1; + if (wc_InitRsaKey(&key, NULL) != 0) { + printf("RsaVerify_SigRange: wc_InitRsaKey failed\n"); + result = -541; + goto done; + } + keyInit = 1; + + ret = wc_RsaPrivateKeyDecode(unitTestRsaPrivKey, &idx, &key, + unitTestRsaPrivKeySz); + if (ret != 0) { result = -542; goto done; } + + encryptSz = wc_RsaEncryptSize(&key); + if (encryptSz <= 0 || (word32)encryptSz > sizeof(sig)) { + result = -543; + goto done; + } + keySz = (word32)encryptSz; + + eSz = (word32)sizeof(eBuf); + nSz = (word32)sizeof(nBuf); + ret = wc_RsaFlattenPublicKey(&key, eBuf, &eSz, nBuf, &nSz); + if (ret != 0 || nSz != keySz) { result = -544; goto done; } + + /* s + n has to fit in keySz bytes to be a signature at all. s depends on + * the message, so sign a few until one does. */ + for (attempt = 0; attempt < 16 && carry; attempt++) { + int i; + word32 acc = 0; + + WMEMSET(data, (byte)(0x42 + attempt), sizeof(data)); + ret = wc_Hash(WC_HASH_TYPE_SHA256, data, sizeof(data), + digest, WC_SHA256_DIGEST_SIZE); + if (ret != 0) { result = -545; goto done; } + + encDigestSz = wc_EncodeSignature(encDigest, digest, + WC_SHA256_DIGEST_SIZE, wc_HashGetOID(WC_HASH_TYPE_SHA256)); + if (encDigestSz <= 0) { result = -546; goto done; } + + ret = wc_RsaSSL_Sign(encDigest, (word32)encDigestSz, sig, sizeof(sig), + &key, &rng); + if (ret <= 0 || (word32)ret != keySz) { result = -547; goto done; } + sigSz = (word32)ret; + + for (i = (int)keySz - 1; i >= 0; i--) { + acc = (word32)sig[i] + (word32)nBuf[i] + (acc >> 8); + sum[i + 1] = (byte)(acc & 0xFF); + } + sum[0] = (byte)(acc >> 8); + carry = sum[0] != 0; + } + if (carry) { result = -548; goto done; } + + /* The signature itself still verifies. */ + ret = wolfSSH_TestRsaVerify(sig, sigSz, encDigest, (word32)encDigestSz, + &key, NULL); + if (ret != WS_SUCCESS) { + printf("RsaVerify_SigRange: valid sig ret=%d expected %d\n", + ret, WS_SUCCESS); + result = -549; + goto done; + } + + /* s + n must not. */ + ret = wolfSSH_TestRsaVerify(sum + 1, keySz, encDigest, (word32)encDigestSz, + &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_SigRange: sig plus modulus ret=%d expected %d\n", + ret, WS_RSA_E); + result = -550; + goto done; + } + + /* The smallest digest the key has no room to pad. */ + WMEMSET(bigDigest, 0, sizeof(bigDigest)); + ret = wolfSSH_TestRsaVerify(sig, sigSz, bigDigest, + keySz - RSA_MIN_PAD_SZ + 1, &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_SigRange: oversized digest ret=%d expected %d\n", + ret, WS_RSA_E); + result = -551; + goto done; + } + + /* Bigger than the block itself. Without the guard, padSz underflows and + * the pad WMEMSET runs off the scratch buffer, so only the guard can + * bring this back. */ + ret = wolfSSH_TestRsaVerify(sig, sigSz, bigDigest, keySz - 2, &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_SigRange: unpaddable digest ret=%d expected %d\n", + ret, WS_RSA_E); + result = -554; + goto done; + } + + /* A key with no size to it is refused as well. */ + if (wc_InitRsaKey(&emptyKey, NULL) != 0) { result = -552; goto done; } + emptyInit = 1; + ret = wolfSSH_TestRsaVerify(sig, sigSz, encDigest, (word32)encDigestSz, + &emptyKey, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_SigRange: empty key ret=%d expected %d\n", + ret, WS_RSA_E); + result = -553; + } + +done: + if (rngInit) + wc_FreeRng(&rng); + if (keyInit) + wc_FreeRsaKey(&key); + if (emptyInit) + wc_FreeRsaKey(&emptyKey); + return result; +} + #endif /* !WOLFSSH_NO_RSA */ #if !defined(WOLFSSH_NO_ED25519) && defined(HAVE_ED25519) && \ @@ -17594,6 +17868,16 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("RsaVerify_BadDigest: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_RsaVerify_BadPadding(); + printf("RsaVerify_BadPadding: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + + unitResult = test_RsaVerify_SigRange(); + printf("RsaVerify_SigRange: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif #if !defined(WOLFSSH_NO_RSA) && !defined(WOLFSSH_NO_SSH_RSA_SHA1) unitResult = test_DoUserAuthRequestRsa();