From 6ba54e646cdb81d6edd2160e9bb992212c724c77 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Mon, 27 Jul 2026 15:52:27 +0900 Subject: [PATCH] fix: keep wc_RsaSSL_Sign()'s result signed in SignHashRsa --- src/agent.c | 9 +++-- tests/api.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/agent.c b/src/agent.c index 1174f35b6..2f10e386c 100644 --- a/src/agent.c +++ b/src/agent.c @@ -712,7 +712,7 @@ static int SignHashRsa(WOLFSSH_AGENT_KEY_RSA* rawKey, enum wc_HashType hashType, { RsaKey key; byte encSig[MAX_ENCODED_SIG_SZ]; - int encSigSz, ret; + int encSigSz, ret, rc; ret = wc_InitRsaKey(&key, heap); if (ret == 0) { @@ -732,12 +732,15 @@ static int SignHashRsa(WOLFSSH_AGENT_KEY_RSA* rawKey, enum wc_HashType hashType, } if (ret == 0) { WLOG(WS_LOG_INFO, "Signing hash with RSA."); - *sigSz = wc_RsaSSL_Sign(encSig, encSigSz, sig, *sigSz, &key, rng); - if (*sigSz <= 0) { + /* Keep the result signed. Storing it straight into the unsigned + * *sigSz would turn a negative error into a huge length. */ + rc = wc_RsaSSL_Sign(encSig, encSigSz, sig, *sigSz, &key, rng); + if (rc <= 0) { WLOG(WS_LOG_DEBUG, "Bad RSA Sign"); ret = WS_RSA_E; } else { + *sigSz = (word32)rc; ret = wolfSSH_RsaVerify(sig, *sigSz, encSig, encSigSz, &key, heap, "SignHashRsa"); } diff --git a/tests/api.c b/tests/api.c index cc48f193d..36f7a4e7d 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1482,7 +1482,7 @@ static void test_wolfSSH_SCP_SendSymlinkReject(void) { ; } #ifdef WOLFSSH_AGENT typedef struct AgentTestCtx { int partialWrite; - byte response[128]; + byte response[512]; word32 responseSz; int writeCalls; int readCalls; @@ -1681,6 +1681,101 @@ static void test_wolfSSH_agent_signrequest_success(void) cleanup_agent_test(ctx, ssh); } + +#ifndef WOLFSSH_NO_RSA_SHA2_256 +/* A hostile agent can answer a sign request with its own messages. An + * identity whose modulus exceeds the agent's signature buffer makes + * wc_RsaSSL_Sign() fail; pre-fix that over-reads, so run this under ASan. */ +static void test_wolfSSH_agent_signrequest_oversize_rsa_key(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + AgentTestCtx io; + byte modulus[384]; + byte body[512]; + byte data[16]; + byte digest[16] = {0}; + byte keyBlob[8] = {0}; + byte sig[16]; + word32 sigSz = sizeof(sig); + word32 idx, i; + int ret; + + /* 3072-bit modulus. The leading byte must have the sign bit clear or + * GetMpint() rejects the value as non-canonical. */ + memset(modulus, 0xa5, sizeof(modulus)); + modulus[0] = 0x01; + memset(data, 0x5a, sizeof(data)); + memset(&io, 0, sizeof(io)); + + /* MSGID_AGENT_ADD_IDENTITY: ssh-rsa, n, e, then empty d/iqmp/p/q and + * comment. The private values are never reached. */ + idx = 0; + put_uint32(body + idx, 7); + idx += LENGTH_SZ; + memcpy(body + idx, "ssh-rsa", 7); + idx += 7; + put_uint32(body + idx, sizeof(modulus)); + idx += LENGTH_SZ; + memcpy(body + idx, modulus, sizeof(modulus)); + idx += sizeof(modulus); + put_uint32(body + idx, 3); + idx += LENGTH_SZ; + body[idx++] = 0x01; + body[idx++] = 0x00; + body[idx++] = 0x01; + for (i = 0; i < 5; i++) { + put_uint32(body + idx, 0); + idx += LENGTH_SZ; + } + AssertTrue(idx <= sizeof(body)); + build_agent_message(io.response, &io.responseSz, + MSGID_AGENT_ADD_IDENTITY, body, idx); + AssertTrue(io.responseSz <= sizeof(io.response)); + + setup_agent_test(&ctx, &ssh, &io); + + /* The agent answers with an add-identity instead of a signature. The + * identity is stored, but the caller is told there was no key. */ + ret = wolfSSH_AGENT_SignRequest(ssh, digest, sizeof(digest), + sig, &sigSz, keyBlob, sizeof(keyBlob), 0); + AssertIntEQ(ret, WS_AGENT_NO_KEY_E); + AssertNotNull(ssh->agent->idList); + + /* MSGID_AGENT_SIGN_REQUEST for that identity. The key blob is the + * n and e pair, matched by its SHA-256 digest. */ + idx = 0; + put_uint32(body + idx, (LENGTH_SZ * 2) + sizeof(modulus) + 3); + idx += LENGTH_SZ; + put_uint32(body + idx, sizeof(modulus)); + idx += LENGTH_SZ; + memcpy(body + idx, modulus, sizeof(modulus)); + idx += sizeof(modulus); + put_uint32(body + idx, 3); + idx += LENGTH_SZ; + body[idx++] = 0x01; + body[idx++] = 0x00; + body[idx++] = 0x01; + put_uint32(body + idx, sizeof(data)); + idx += LENGTH_SZ; + memcpy(body + idx, data, sizeof(data)); + idx += sizeof(data); + put_uint32(body + idx, AGENT_SIGN_RSA_SHA2_256); + idx += LENGTH_SZ; + AssertTrue(idx <= sizeof(body)); + build_agent_message(io.response, &io.responseSz, + MSGID_AGENT_SIGN_REQUEST, body, idx); + AssertTrue(io.responseSz <= sizeof(io.response)); + + sigSz = sizeof(sig); + ret = wolfSSH_AGENT_SignRequest(ssh, digest, sizeof(digest), + sig, &sigSz, keyBlob, sizeof(keyBlob), 0); + AssertIntEQ(ret, WS_RSA_E); + AssertIntEQ(sigSz, 0); + + cleanup_agent_test(ctx, ssh); +} +#endif /* WOLFSSH_NO_RSA_SHA2_256 */ #endif /* WOLFSSH_AGENT */ @@ -4396,6 +4491,9 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_agent_signrequest_wrong_message(); test_wolfSSH_agent_signrequest_signature_too_large(); test_wolfSSH_agent_signrequest_success(); +#ifndef WOLFSSH_NO_RSA_SHA2_256 + test_wolfSSH_agent_signrequest_oversize_rsa_key(); +#endif #endif #ifdef WOLFSSH_KEYBOARD_INTERACTIVE test_wolfSSH_KeyboardInteractive();