From 013db64d8af790373c9348a55056d0e159f2ead8 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 2 Nov 2026 09:07:53 -0700 Subject: [PATCH 1/2] Refuse RSA signatures longer than the key wolfSSH_RsaVerify() now returns WS_RSA_E when the signature is longer than the modulus, so trailing bytes after a valid signature no longer verify. A shorter signature is still left-padded with zeros. - tests: unit test with one byte appended to a valid signature --- src/internal.c | 16 ++++++---------- tests/unit.c | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index f142c0db9..e279d9118 100644 --- a/src/internal.c +++ b/src/internal.c @@ -17303,17 +17303,13 @@ int wolfSSH_RsaVerify(const byte *sig, word32 sigSz, /* Normalize the peer's signature. Some SSH implementations remove * leading zeros on the signatures they encode. We need to pad the - * front of the signature to the key size. */ + * front of the signature to the key size. A longer one is refused. */ + if (ret == WS_SUCCESS && sigSz > keySz) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "RSA signature too long"); + ret = WS_RSA_E; + } if (ret == WS_SUCCESS) { - word32 offset; - - if (keySz > sigSz) { - offset = keySz - sigSz; - } - else { - sigSz = keySz; - offset = 0; - } + word32 offset = keySz - sigSz; WMEMSET(checkSig, 0, offset); WMEMCPY(checkSig + offset, sig, sigSz); diff --git a/tests/unit.c b/tests/unit.c index ffc3517d6..7d64ea2ba 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -14926,7 +14926,8 @@ static int test_RsaVerify_BadPadding(void) /* 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. */ + * an explicit range check. Also covers a signature longer than the key and + * the two key-size guards. */ static int test_RsaVerify_SigRange(void) { int result = 0; @@ -15019,6 +15020,18 @@ static int test_RsaVerify_SigRange(void) goto done; } + /* Nor does it with a byte appended. */ + if (keySz + 1 > (word32)sizeof(sig)) { result = -555; goto done; } + sig[keySz] = 0x00; + ret = wolfSSH_TestRsaVerify(sig, keySz + 1, encDigest, + (word32)encDigestSz, &key, NULL); + if (ret != WS_RSA_E) { + printf("RsaVerify_SigRange: trailing byte ret=%d expected %d\n", + ret, WS_RSA_E); + result = -556; + goto done; + } + /* s + n must not. */ ret = wolfSSH_TestRsaVerify(sum + 1, keySz, encDigest, (word32)encDigestSz, &key, NULL); From aafcea862b204bbb9305ef7f8ea6fb5fe3c6ad7d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 2 Nov 2026 09:10:26 -0700 Subject: [PATCH 2/2] Verify RSA host key signatures by encoding DoKexDhReply() now checks an RSA host key signature over H the same way userauth does: it hashes H, encodes the DigestInfo, and compares whole blocks with wolfSSH_RsaVerify() per RFC 8332 Section 5.3, in place of wc_SignatureVerify(). This also restores ssh-rsa host keys on wolfSSL 5.9.2 and later, which refuse SHA-1 in wc_SignatureVerify(). - tests: ssh-rsa host key handshake, valid and corrupted signature - tests: host key signature with a trailing byte is rejected Issue: F-17075 --- src/internal.c | 71 +++++++++++++++++++++++++++++++++++++++++-------- tests/regress.c | 52 ++++++++++++++++++++++++++++++++---- 2 files changed, 107 insertions(+), 16 deletions(-) diff --git a/src/internal.c b/src/internal.c index e279d9118..ae3b7eca6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9213,24 +9213,73 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) if (sigKeyBlock_ptr->useRsa) { #ifndef WOLFSSH_NO_RSA + enum wc_HashType rsaHashId = + HashForId(ssh->handshake->pubKeyId); + int encDigestSz = 0; + int digestSz = wc_HashGetDigestSize(rsaHashId); +#ifdef WOLFSSH_SMALL_STACK + byte* digest = NULL; + byte* encDigest = NULL; +#else + byte digest[WC_MAX_DIGEST_SIZE]; + byte encDigest[MAX_ENCODED_SIG_SZ]; +#endif + if (sigSz < MIN_RSA_SIG_SZ) { WLOG(WS_LOG_DEBUG, "Provided signature is too small."); ret = WS_RSA_E; } + if (ret == WS_SUCCESS && (digestSz <= 0 + || digestSz > WC_MAX_DIGEST_SIZE)) { + ret = WS_INVALID_ALGO_ID; + } + +#ifdef WOLFSSH_SMALL_STACK if (ret == WS_SUCCESS) { - ret = wc_SignatureVerify( - HashForId(ssh->handshake->pubKeyId), - WC_SIGNATURE_TYPE_RSA_W_ENC, - ssh->h, ssh->hSz, sig, sigSz, - &sigKeyBlock_ptr->sk, sigKeyBlock_ptr->keySz); - if (ret != 0) { - WLOG(WS_LOG_DEBUG, - "DoKexDhReply: Signature Verify fail (%d)", - ret); - ret = WS_RSA_E; - } + digest = (byte*)WMALLOC(WC_MAX_DIGEST_SIZE, + ssh->ctx->heap, DYNTYPE_BUFFER); + encDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, + ssh->ctx->heap, DYNTYPE_BUFFER); + if (digest == NULL || encDigest == NULL) + ret = WS_MEMORY_E; } +#endif + + if (ret == WS_SUCCESS) { + ret = wc_Hash(rsaHashId, ssh->h, ssh->hSz, + digest, (word32)digestSz); + if (ret != 0) + ret = WS_CRYPTO_FAILED; + } + + if (ret == WS_SUCCESS) { + encDigestSz = wc_EncodeSignature(encDigest, digest, + (word32)digestSz, wc_HashGetOID(rsaHashId)); + if (encDigestSz <= 0) + ret = WS_CRYPTO_FAILED; + } + + if (ret == WS_SUCCESS) { + ret = wolfSSH_RsaVerify(sig, sigSz, encDigest, + (word32)encDigestSz, + &sigKeyBlock_ptr->sk.rsa.key, + ssh->ctx->heap, "DoKexDhReply"); + } + +#ifdef WOLFSSH_SMALL_STACK + if (digest != NULL) { + WS_FORCEZERO(digest, WC_MAX_DIGEST_SIZE); + WFREE(digest, ssh->ctx->heap, DYNTYPE_BUFFER); + } + if (encDigest != NULL) { + WS_FORCEZERO(encDigest, MAX_ENCODED_SIG_SZ); + WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); + } +#else + WS_FORCEZERO(digest, sizeof(digest)); + WS_FORCEZERO(encDigest, sizeof(encDigest)); +#endif #endif } else if (sigKeyBlock_ptr->useEcc) { diff --git a/tests/regress.c b/tests/regress.c index ad9b79a94..a301a5605 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -796,6 +796,8 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) /* The splice ahead of the client's KEXDH_INIT, while the server waits for * the message that opens the negotiated KEX. */ #define REGRESS_MUTATE_TERRAPIN_PRE_KEXDH 11 +/* A zero byte appended to the host key signature data. */ +#define REGRESS_MUTATE_SIG_TRAIL 12 /* 4 (len) + 1 (padLen) + 1 (msgId) + 4 (empty string payload) + 6 (pad) */ #define REGRESS_INJECT_PACKET_SZ 16U @@ -1079,6 +1081,15 @@ static int RewriteSingleKexDhReplyPacket(const byte* packet, word32 packetSz, } innerSig[flipIdx] ^= 0xFF; } + else if (mode == REGRESS_MUTATE_SIG_TRAIL) { + innerSigSz = AppendBlob(innerSig, sizeof(innerSig), innerSigSz, + sigName, sigNameSz); + innerSigSz = AppendUint32(innerSig, sizeof(innerSig), innerSigSz, + sigDataSz + 1); + innerSigSz = AppendData(innerSig, sizeof(innerSig), innerSigSz, + sigData, sigDataSz); + innerSigSz = AppendByte(innerSig, sizeof(innerSig), innerSigSz, 0x00); + } else if (mode == REGRESS_MUTATE_SIG_NAME_OVERRUN) { /* Nothing but a name length prefix, so the name it claims runs off * the end of the blob. */ @@ -2210,15 +2221,14 @@ static void TestKexDhReplyRejectsWhenCallbackRejects(void) #endif } -/* Valid name, flipped data byte: the client reaches the verify and rejects. */ -static void AssertHandshakeRejectsCorruptedSig(const char* keyAlgo, - const char* keyPath, int expectedErr) +/* Valid name, altered data: the client reaches the verify and rejects. */ +static void AssertHandshakeRejectsSigMutation(const char* keyAlgo, + const char* keyPath, byte mode, int expectedErr) { KexReplyHarness harness; KexReplyRunResult result; - InitKexReplyHarnessEx(&harness, keyAlgo, keyPath, 1, - REGRESS_MUTATE_SIG_DATA, NULL, 0); + InitKexReplyHarnessEx(&harness, keyAlgo, keyPath, 1, mode, NULL, 0); RunKexReplyHandshake(&harness, &result); AssertIntEQ(harness.mutator.parseError, 0); @@ -2236,6 +2246,13 @@ static void AssertHandshakeRejectsCorruptedSig(const char* keyAlgo, FreeKexReplyHarness(&harness); } +static void AssertHandshakeRejectsCorruptedSig(const char* keyAlgo, + const char* keyPath, int expectedErr) +{ + AssertHandshakeRejectsSigMutation(keyAlgo, keyPath, + REGRESS_MUTATE_SIG_DATA, expectedErr); +} + /* Every key agreement, the ML-KEM hybrids included, wipes K on both sides. */ static void TestKexSecretWipedPerKex(void) { @@ -2310,6 +2327,25 @@ static void TestKexDhReplyRejectsRsaSha2_512CorruptSig(void) } #endif +#ifndef WOLFSSH_NO_RSA_SHA2_256 +static void TestKexDhReplyRejectsRsaSigTrailingByte(void) +{ + AssertHandshakeRejectsSigMutation("rsa-sha2-256", + REGRESS_SERVER_KEY_PATH, REGRESS_MUTATE_SIG_TRAIL, WS_RSA_E); +} +#endif + +#ifndef WOLFSSH_NO_SSH_RSA_SHA1 +/* ssh-rsa signs H with SHA-1, which wc_SignatureVerify() refuses on newer + * wolfSSL. */ +static void TestKexDhReplyAcceptsSshRsa(void) +{ + AssertHandshakeSucceeds("ssh-rsa", REGRESS_SERVER_KEY_PATH); + AssertHandshakeRejectsCorruptedSig("ssh-rsa", + REGRESS_SERVER_KEY_PATH, WS_RSA_E); +} +#endif + #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 /* The flip preserves r's mpint length, so the verify fails with WS_ECC_E. */ static void TestKexDhReplyRejectsEccCorruptSig(void) @@ -22183,6 +22219,12 @@ int main(int argc, char** argv) #ifndef WOLFSSH_NO_RSA_SHA2_512 TestKexDhReplyRejectsRsaSha2_512CorruptSig(); #endif + #ifndef WOLFSSH_NO_RSA_SHA2_256 + TestKexDhReplyRejectsRsaSigTrailingByte(); + #endif + #ifndef WOLFSSH_NO_SSH_RSA_SHA1 + TestKexDhReplyAcceptsSshRsa(); + #endif #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 TestKexDhReplyRejectsEccCorruptSig(); #endif