diff --git a/src/internal.c b/src/internal.c index deb11d14a..f4944428c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -627,15 +627,16 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) } #endif #ifndef WOLFSSH_NO_ECDH - /* privKey is a union; the Curve25519+ML-KEM case also sets - * useEccMlKem but generates a curve25519 key, so free it below. */ - if (hs->useEcc || (hs->useEccMlKem && !hs->useCurve25519MlKem)) { + /* privKey is a union; the Curve25519+ML-KEM hybrid sets both + * useEccMlKem and useCurve25519 but generates a curve25519 key, so + * it is freed below rather than here. */ + if (hs->useEcc || (hs->useEccMlKem && !hs->useCurve25519)) { wc_ecc_free(&hs->privKey.ecc); } #endif #if !defined(WOLFSSH_NO_CURVE25519_SHA256) || \ !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256) - if (hs->useCurve25519 || hs->useCurve25519MlKem) { + if (hs->useCurve25519) { wc_curve25519_free(&hs->privKey.curve25519); } #endif @@ -5658,17 +5659,25 @@ static int KeyAgreeDh_client(WOLFSSH* ssh, byte hashId, WLOG(WS_LOG_DEBUG, "Entering KeyAgreeDh_client()"); WOLFSSH_UNUSED(hashId); - PRIVATE_KEY_UNLOCK(); - ret = wc_DhAgree(&ssh->handshake->privKey.dh, - ssh->k, &ssh->kSz, - ssh->handshake->x, ssh->handshake->xSz, - f, fSz); - PRIVATE_KEY_LOCK(); + ret = wc_DhCheckPubKey(&ssh->handshake->privKey.dh, f, fSz); if (ret != 0) { WLOG(WS_LOG_ERROR, - "Generate DH shared secret failed, %d", ret); + "Peer DH public key out of range, %d", ret); ret = WS_CRYPTO_FAILED; } + if (ret == 0) { + PRIVATE_KEY_UNLOCK(); + ret = wc_DhAgree(&ssh->handshake->privKey.dh, + ssh->k, &ssh->kSz, + ssh->handshake->x, ssh->handshake->xSz, + f, fSz); + PRIVATE_KEY_LOCK(); + if (ret != 0) { + WLOG(WS_LOG_ERROR, + "Generate DH shared secret failed, %d", ret); + ret = WS_CRYPTO_FAILED; + } + } ForceZero(ssh->handshake->x, ssh->handshake->xSz); wc_FreeDhKey(&ssh->handshake->privKey.dh); @@ -5686,6 +5695,41 @@ static int KeyAgreeDh_client(WOLFSSH* ssh, byte hashId, #endif /* WOLFSSH_NO_DH */ +#if !defined(WOLFSSH_NO_ECDH) || \ + !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256) || \ + !defined(WOLFSSH_NO_NISTP384_MLKEM1024_SHA384) +/* Import a peer's X9.63 ECC point and validate it before it is used to derive + * a shared secret. Shared by the plain and ML-KEM-hybrid ECDH paths, on both + * client and server, so the peer point validation cannot diverge between them: + * a single unit test on the plain path exercises the same import+check logic + * the hybrid paths rely on. When primeId is ECC_CURVE_INVALID the curve is + * inferred from the point length (wc_ecc_import_x963), matching the client's + * behavior; otherwise the expected curve is enforced (wc_ecc_import_x963_ex), + * matching the server's. wc_ecc_check_key rejects off-curve and degenerate + * points that import does not catch in builds without + * WOLFSSL_VALIDATE_ECC_IMPORT. Returns a wolfCrypt status (0 on success). */ +static int EccCheckPeerKey(ecc_key* key, const byte* peer, word32 peerSz, + int primeId, WC_RNG* rng) +{ + int ret; + + if (primeId == ECC_CURVE_INVALID) + ret = wc_ecc_import_x963(peer, peerSz, key); + else + ret = wc_ecc_import_x963_ex(peer, peerSz, key, primeId); +#ifdef HAVE_WC_ECC_SET_RNG + if (ret == 0) + ret = wc_ecc_set_rng(key, rng); +#endif + if (ret == 0) + ret = wc_ecc_check_key(key); + + WOLFSSH_UNUSED(rng); + return ret; +} +#endif + + /* KeyAgreeEcdh_client * hashId - wolfCrypt hash type ID used * f - peer public key @@ -5714,12 +5758,8 @@ static int KeyAgreeEcdh_client(WOLFSSH* ssh, byte hashId, key_ptr = &key_s; #endif /* WOLFSSH_SMALL_STACK */ ret = wc_ecc_init(key_ptr); - #ifdef HAVE_WC_ECC_SET_RNG if (ret == 0) - ret = wc_ecc_set_rng(key_ptr, ssh->rng); - #endif - if (ret == 0) - ret = wc_ecc_import_x963(f, fSz, key_ptr); + ret = EccCheckPeerKey(key_ptr, f, fSz, ECC_CURVE_INVALID, ssh->rng); if (ret == 0) { PRIVATE_KEY_UNLOCK(); ret = wc_ecc_shared_secret(&ssh->handshake->privKey.ecc, @@ -5942,14 +5982,13 @@ static int KeyAgreeEcdhMlKem_client(WOLFSSH* ssh, byte hashId, if (ret == 0) { ret = wc_ecc_init(key_ptr); } - #ifdef HAVE_WC_ECC_SET_RNG - if (ret == 0) { - ret = wc_ecc_set_rng(key_ptr, ssh->rng); - } - #endif + /* Import and validate the peer ECC point via the shared helper, the + * same path the plain KeyAgreeEcdh_client uses, so the off-curve + * rejection cannot diverge between the plain and hybrid code. The ECC + * point sits after the ML-KEM ciphertext in f. */ if (ret == 0) { - ret = wc_ecc_import_x963(f + length_ciphertext, - fSz - length_ciphertext, key_ptr); + ret = EccCheckPeerKey(key_ptr, f + length_ciphertext, + fSz - length_ciphertext, ECC_CURVE_INVALID, ssh->rng); } if (ret == 0) { @@ -6058,12 +6097,14 @@ static int KeyAgree_client(WOLFSSH* ssh, byte hashId, const byte* f, word32 fSz) else if (ssh->handshake->useEcc) { ret = KeyAgreeEcdh_client(ssh, hashId, f, fSz); } - else if (ssh->handshake->useCurve25519) { - ret = KeyAgreeCurve25519_client(ssh, hashId, f, fSz); - } + /* Check useEccMlKem before useCurve25519: the Curve25519+ML-KEM hybrid + * sets both flags and must route to the hybrid agreement. */ else if (ssh->handshake->useEccMlKem) { ret = KeyAgreeEcdhMlKem_client(ssh, hashId, f, fSz); } + else if (ssh->handshake->useCurve25519) { + ret = KeyAgreeCurve25519_client(ssh, hashId, f, fSz); + } else { ret = WS_INVALID_ALGO_ID; } @@ -7403,51 +7444,59 @@ static int DoUserAuthRequestPassword(WOLFSSH* ssh, WS_UserAuthData* authData, if (ret == WS_SUCCESS) { if (pw->hasNewPassword) { - /* Skip the password change. Maybe error out since we aren't - * supporting password changes at this time. */ + /* Password changes are not supported. Parse the new password + * field so the message is fully consumed, then reject the + * request rather than authenticating with the current password + * (RFC 4252 section 8: an expired password MUST NOT be used for + * authentication). The userauth callback is not called. */ ret = GetStringRef(&pw->newPasswordSz, &pw->newPassword, buf, len, &begin); + if (ret == WS_SUCCESS) { + WLOG(WS_LOG_DEBUG, + "DUARPW: rejecting unsupported password change request"); + authFailure = 1; + } } else { pw->newPassword = NULL; pw->newPasswordSz = 0; - } - if (ssh->ctx->userAuthCb != NULL) { - WLOG(WS_LOG_DEBUG, "DUARPW: Calling the userauth callback"); - ret = ssh->ctx->userAuthCb(WOLFSSH_USERAUTH_PASSWORD, - authData, ssh->userAuthCtx); - if (ret == WOLFSSH_USERAUTH_SUCCESS) { - WLOG(WS_LOG_DEBUG, "DUARPW: password check success"); - ret = WS_SUCCESS; - } - else if (ret == WOLFSSH_USERAUTH_PARTIAL_SUCCESS) { - WLOG(WS_LOG_DEBUG, "DUARPW: password check partial success"); - partialSuccess = 1; - ret = WS_SUCCESS; - } - else if (ret == WOLFSSH_USERAUTH_REJECTED) { - WLOG(WS_LOG_DEBUG, "DUARPW: password rejected"); - #ifndef NO_FAILURE_ON_REJECTED + if (ssh->ctx->userAuthCb != NULL) { + WLOG(WS_LOG_DEBUG, "DUARPW: Calling the userauth callback"); + ret = ssh->ctx->userAuthCb(WOLFSSH_USERAUTH_PASSWORD, + authData, ssh->userAuthCtx); + if (ret == WOLFSSH_USERAUTH_SUCCESS) { + WLOG(WS_LOG_DEBUG, "DUARPW: password check success"); + ret = WS_SUCCESS; + } + else if (ret == WOLFSSH_USERAUTH_PARTIAL_SUCCESS) { + WLOG(WS_LOG_DEBUG, "DUARPW: password check partial success"); + partialSuccess = 1; + ret = WS_SUCCESS; + } + else if (ret == WOLFSSH_USERAUTH_REJECTED) { + WLOG(WS_LOG_DEBUG, "DUARPW: password rejected"); + #ifndef NO_FAILURE_ON_REJECTED + authFailure = 1; + #endif + authRejected = 1; + ret = WS_USER_AUTH_E; + } + else if (ret == WOLFSSH_USERAUTH_WOULD_BLOCK) { + WLOG(WS_LOG_DEBUG, "DUARPW: userauth callback would block"); + ret = WS_AUTH_PENDING; + } + else { + WLOG(WS_LOG_DEBUG, "DUARPW: password check failed, retry"); authFailure = 1; - #endif - authRejected = 1; - ret = WS_USER_AUTH_E; - } - else if (ret == WOLFSSH_USERAUTH_WOULD_BLOCK) { - WLOG(WS_LOG_DEBUG, "DUARPW: userauth callback would block"); - ret = WS_AUTH_PENDING; + ret = WS_SUCCESS; + } } else { - WLOG(WS_LOG_DEBUG, "DUARPW: password check failed, retry"); + WLOG(WS_LOG_DEBUG, "DUARPW: No user auth callback"); authFailure = 1; - ret = WS_SUCCESS; } } - else { - WLOG(WS_LOG_DEBUG, "DUARPW: No user auth callback"); - authFailure = 1; - } } if (ret == WS_SUCCESS) @@ -10250,6 +10299,13 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) idx += UINT32_SZ; padSz = buf[idx++]; + /* RFC 4253 section 6 requires at least four bytes of padding. */ + if (padSz < MIN_PAD_LENGTH) { + WLOG(WS_LOG_DEBUG, "Packet padding length %u below minimum %u", + (word32)padSz, (word32)MIN_PAD_LENGTH); + return WS_BUFFER_E; + } + /* check for underflow */ if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > ssh->curSz) { return WS_OVERFLOW_E; @@ -12371,6 +12427,9 @@ static int KeyAgreeDh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz) if (ret == 0) ret = wc_DhSetKey(privKey, primeGroup, primeGroupSz, generator, generatorSz); + if (ret == 0) + ret = wc_DhCheckPubKey(privKey, ssh->handshake->e, + ssh->handshake->eSz); if (ret == 0) ret = wc_DhGenerateKeyPair(privKey, ssh->rng, y_ptr, &ySz, f, fSz); @@ -12446,9 +12505,8 @@ static int KeyAgreeEcdh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz) #endif if (ret == 0) - ret = wc_ecc_import_x963_ex(ssh->handshake->e, - ssh->handshake->eSz, - pubKey, primeId); + ret = EccCheckPeerKey(pubKey, ssh->handshake->e, + ssh->handshake->eSz, primeId, ssh->rng); if (ret == 0) ret = wc_ecc_make_key_ex(ssh->rng, @@ -12767,11 +12825,15 @@ static int KeyAgreeEcdhMlKem_server(WOLFSSH* ssh, byte hashId, ret = wc_ecc_set_rng(privKey, ssh->rng); } #endif + /* Import and validate the peer ECC point via the shared helper, the + * same path the plain KeyAgreeEcdh_server uses, so the off-curve + * rejection cannot diverge between the plain and hybrid code. The ECC + * point sits after the ML-KEM public key in e. */ if (ret == 0) { - ret = wc_ecc_import_x963_ex( + ret = EccCheckPeerKey(pubKey, ssh->handshake->e + length_publickey, ssh->handshake->eSz - length_publickey, - pubKey, primeId); + primeId, ssh->rng); } if (ret == 0) { ret = wc_ecc_make_key_ex(ssh->rng, @@ -13957,8 +14019,11 @@ int SendKexDhInit(WOLFSSH* ssh) #endif #ifndef WOLFSSH_NO_CURVE25519_MLKEM768_SHA256 case ID_CURVE25519_MLKEM768_SHA256: + /* The Curve25519+ML-KEM hybrid is identified by both flags: + * useEccMlKem (ML-KEM component) and useCurve25519 (classical + * component). */ ssh->handshake->useEccMlKem = 1; - ssh->handshake->useCurve25519MlKem = 1; + ssh->handshake->useCurve25519 = 1; msgId = MSGID_KEXKEM_INIT; break; #endif @@ -13984,8 +14049,11 @@ int SendKexDhInit(WOLFSSH* ssh) e, &eSz); #endif } -#ifndef WOLFSSH_NO_CURVE25519_SHA256 +#if !defined(WOLFSSH_NO_CURVE25519_SHA256) || \ + !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256) else if (ssh->handshake->useCurve25519) { + /* Plain Curve25519 or the Curve25519+ML-KEM hybrid; both need a + * Curve25519 key. The ML-KEM component, if any, is added below. */ curve25519_key* privKey = &ssh->handshake->privKey.curve25519; if (ret == 0) ret = wc_curve25519_init_ex(privKey, ssh->ctx->heap, @@ -14000,25 +14068,7 @@ int SendKexDhInit(WOLFSSH* ssh) PRIVATE_KEY_LOCK(); } } -#endif /* ! WOLFSSH_NO_CURVE25519_SHA256 */ -#ifndef WOLFSSH_NO_CURVE25519_MLKEM768_SHA256 - else if (ssh->handshake->useCurve25519MlKem) { - /* Handle Curve25519+ML-KEM variant - generate Curve25519 key */ - curve25519_key* privKey = &ssh->handshake->privKey.curve25519; - if (ret == 0) - ret = wc_curve25519_init_ex(privKey, ssh->ctx->heap, - INVALID_DEVID); - if (ret == 0) - ret = wc_curve25519_make_key(ssh->rng, CURVE25519_KEYSIZE, - privKey); - if (ret == 0) { - PRIVATE_KEY_UNLOCK(); - ret = wc_curve25519_export_public_ex(privKey, e, &eSz, - EC25519_LITTLE_ENDIAN); - PRIVATE_KEY_LOCK(); - } - } -#endif /* WOLFSSH_NO_CURVE25519_MLKEM768_SHA256 */ +#endif /* Curve25519 or Curve25519+ML-KEM */ else if (ssh->handshake->useEcc #if !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256) || \ !defined(WOLFSSH_NO_NISTP384_MLKEM1024_SHA384) @@ -18528,8 +18578,70 @@ int wolfSSH_TestKeyAgreeDh_server(WOLFSSH* ssh, byte hashId, return KeyAgreeDh_server(ssh, hashId, f, fSz); } +/* Generate the client's ephemeral DH key for the prime group selected by + * ssh->handshake->kexId, mirroring the setup SendKexDhInit() performs before + * KeyAgreeDh_client() runs (group set, key pair generated into + * handshake->x). Lets unit tests exercise the peer public key range check + * with a real private exponent in place, so removing the check would let + * wc_DhAgree proceed rather than failing for lack of a private key. */ +int wolfSSH_TestSetDhKexKey(WOLFSSH* ssh) +{ + int ret; + DhKey* privKey = &ssh->handshake->privKey.dh; + const byte* primeGroup = NULL; + const byte* generator = NULL; + word32 primeGroupSz = 0; + word32 generatorSz = 0; + byte e[MAX_KEX_KEY_SZ]; + word32 eSz = (word32)sizeof(e); + int keyInited = 0; + + ret = GetDHPrimeGroup(ssh->handshake->kexId, &primeGroup, &primeGroupSz, + &generator, &generatorSz); + if (ret == WS_SUCCESS) { + ret = wc_InitDhKey(privKey); + if (ret == WS_SUCCESS) + keyInited = 1; + } + if (ret == WS_SUCCESS) + ret = wc_DhSetKey(privKey, primeGroup, primeGroupSz, + generator, generatorSz); + if (ret == WS_SUCCESS) { + ssh->handshake->xSz = (word32)sizeof(ssh->handshake->x); + ret = wc_DhGenerateKeyPair(privKey, ssh->rng, + ssh->handshake->x, &ssh->handshake->xSz, e, &eSz); + } + if (ret == WS_SUCCESS) { + /* Mark the DH key live, mirroring the real handshake, so + * HandshakeInfoFree() reclaims it if the test exits before + * KeyAgreeDh_client() runs. wc_FreeDhKey() is idempotent, so the + * later free in KeyAgreeDh_client() and HandshakeInfoFree() is safe. */ + ssh->handshake->useDh = 1; + } + else if (keyInited) { + /* Free the key only when wc_InitDhKey succeeded so wc_DhSetKey's + * mp_int storage is not leaked. */ + wc_FreeDhKey(privKey); + } + return ret; +} + #endif /* !WOLFSSH_NO_DH */ +#ifndef WOLFSSH_NO_ECDH +int wolfSSH_TestKeyAgreeEcdh_server(WOLFSSH* ssh, byte hashId, + byte* f, word32* fSz) +{ + return KeyAgreeEcdh_server(ssh, hashId, f, fSz); +} + +int wolfSSH_TestKeyAgreeEcdh_client(WOLFSSH* ssh, byte hashId, + const byte* f, word32 fSz) +{ + return KeyAgreeEcdh_client(ssh, hashId, f, fSz); +} +#endif /* !WOLFSSH_NO_ECDH */ + #ifndef WOLFSSH_NO_DH_GEX_SHA256 int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len, diff --git a/tests/unit.c b/tests/unit.c index e18981ef7..ac24995a3 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -950,6 +950,78 @@ static int test_DoReceive_VerifyMacFailure(void) #endif /* WOLFSSH_TEST_INTERNAL && any HMAC SHA variant enabled */ +#ifdef WOLFSSH_TEST_INTERNAL +/* Verify DoReceive rejects a binary packet whose padding_length is below the + * RFC 4253 section 6 minimum of four bytes, returning WS_BUFFER_E. The packet + * is delivered in the clear (no cipher, no MAC), matching the pre-key-exchange + * transport, so DoPacket's padding check is what rejects it. */ +static int test_DoReceive_RejectsShortPadding(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + int ret; + int result = 0; + /* A well-formed MSGID_IGNORE packet carrying an empty string, but with + * padding_length = 1 (below MIN_PAD_LENGTH). Aside from the short padding + * the packet parses cleanly, so the padding check is the only thing that + * can reject it. Layout: uint32 packet_length=7, padding_length=1, + * msgId, uint32 string_len=0, 1 pad byte => 11 bytes total. */ + byte pkt[11]; + word32 totalLen = (word32)sizeof(pkt); + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -760; + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + wolfSSH_CTX_free(ctx); + return -761; + } + + pkt[0] = 0; pkt[1] = 0; pkt[2] = 0; pkt[3] = 7; /* packet_length */ + pkt[4] = 1; /* padding_length, below MIN_PAD_LENGTH (4) */ + pkt[5] = MSGID_IGNORE; + pkt[6] = 0; pkt[7] = 0; pkt[8] = 0; pkt[9] = 0; /* string_len = 0 */ + pkt[10] = 0; /* padding */ + + ssh->peerEncryptId = ID_NONE; + ssh->peerAeadMode = 0; + ssh->peerBlockSz = MIN_BLOCK_SZ; + ssh->peerMacId = ID_NONE; + ssh->peerMacSz = 0; + ssh->peerSeq = 0; + ssh->curSz = 0; + ssh->processReplyState = PROCESS_INIT; + ssh->error = 0; + + ShrinkBuffer(&ssh->inputBuffer, 1); + ret = GrowBuffer(&ssh->inputBuffer, totalLen); + if (ret != WS_SUCCESS) { + result = -762; + goto done2; + } + WMEMCPY(ssh->inputBuffer.buffer, pkt, totalLen); + ssh->inputBuffer.length = totalLen; + ssh->inputBuffer.idx = 0; + + ret = wolfSSH_TestDoReceive(ssh); + if (ret != WS_FATAL_ERROR) { + result = -763; + goto done2; + } + if (ssh->error != WS_BUFFER_E) { + result = -764; + goto done2; + } + +done2: + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} +#endif /* WOLFSSH_TEST_INTERNAL */ + + #if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256) typedef struct { @@ -2237,6 +2309,107 @@ static int test_DoUserAuthRequest_serviceName(void) } +/* userauth callback that records whether it was invoked. Returns SUCCESS so + * that, if it were ever reached for a password-change request, the request + * would be (incorrectly) authenticated - making a missed rejection visible. */ +static int s_pwChangeCbCalled = 0; +static int UnitAuthAlwaysSucceed(byte authType, WS_UserAuthData* authData, + void* ctx) +{ + (void)authData; + (void)ctx; + if (authType == WOLFSSH_USERAUTH_PASSWORD) { + s_pwChangeCbCalled = 1; + } + return WOLFSSH_USERAUTH_SUCCESS; +} + +/* Verify DoUserAuthRequest rejects a password request that sets the + * password-change flag (RFC 4252 Section 8: an expired password MUST NOT be + * used for authentication). The request is otherwise well-formed and the + * userauth callback would return SUCCESS, so a missing rejection would let the + * old password authenticate. Asserts: + * 1. ret == WS_SUCCESS (connection stays open for retry) + * 2. the userauth callback is never invoked + * 3. exactly one packet is sent and it is SSH_MSG_USERAUTH_FAILURE + * 4. *idx == len (the new-password field is fully consumed) */ +static int test_DoUserAuthRequest_rejectsPasswordChange(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + int result = 0; + int ret; + int capMsgId; + byte buf[128]; + word32 len = 0, idx = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -660; + wolfSSH_SetIOSend(ctx, CaptureIoSendAuthSvc); + wolfSSH_SetUserAuth(ctx, UnitAuthAlwaysSucceed); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -661; + goto out; + } + + s_pwChangeCbCalled = 0; + s_authSvcCaptureSz = 0; + s_authSvcSendCount = 0; + WMEMSET(s_authSvcCapture, 0, sizeof(s_authSvcCapture)); + + /* username: "user" */ + buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 4; + WMEMCPY(buf + len, "user", 4); len += 4; + /* service name: "ssh-connection" */ + buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 14; + WMEMCPY(buf + len, "ssh-connection", 14); len += 14; + /* auth method: "password" */ + buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 8; + WMEMCPY(buf + len, "password", 8); len += 8; + /* password-change flag: TRUE */ + buf[len++] = 1; + /* current password: "oldpass" */ + buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 7; + WMEMCPY(buf + len, "oldpass", 7); len += 7; + /* new password: "newpass" */ + buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 7; + WMEMCPY(buf + len, "newpass", 7); len += 7; + + ret = wolfSSH_TestDoUserAuthRequest(ssh, buf, len, &idx); + + if (ret != WS_SUCCESS) { + result = -662; + goto out; + } + if (s_pwChangeCbCalled) { + /* The callback must not run for a password-change request. */ + result = -663; + goto out; + } + if (s_authSvcSendCount != 1) { + result = -664; + goto out; + } + capMsgId = CaptureMsgId(s_authSvcCapture, s_authSvcCaptureSz); + if (capMsgId != MSGID_USERAUTH_FAILURE) { + result = -665; + goto out; + } + if (idx != len) { + result = -666; + goto out; + } + +out: + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + + /* userAuthTypesCb that advertises no methods (returns mask 0). Mirrors a * wolfsshd configuration with both PasswordAuthentication no and * PubkeyAuthentication no. */ @@ -3696,6 +3869,111 @@ static int test_KeyAgreeDh_client_zeroesEphemeralPrivKey(void) return result; } +/* Verify KeyAgreeDh_server rejects a degenerate peer DH public key. + * The peer public 'e' is read from ssh->handshake->e; a value of 1 sits at + * the low end of RFC 4253 section 8's permitted [1, p-1] range, but it is a + * weak/degenerate value that wc_DhCheckPubKey rejects (stricter than the + * RFC's bare minimum) before the shared secret is computed. Without that + * guard wc_DhAgree would happily derive the degenerate secret 1. */ +static int test_KeyAgreeDh_server_rejectsBadPeerPublic(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte f[MAX_KEX_KEY_SZ]; + word32 fSz = (word32)sizeof(f); + int result = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -730; + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -731; + goto out; + } +#ifndef WOLFSSH_NO_DH_GROUP14_SHA256 + ssh->handshake->kexId = ID_DH_GROUP14_SHA256; +#elif !defined(WOLFSSH_NO_DH_GROUP14_SHA1) + ssh->handshake->kexId = ID_DH_GROUP14_SHA1; +#else + ssh->handshake->kexId = ID_DH_GROUP1_SHA1; +#endif + + /* Degenerate peer public key e = 1 (yields the shared secret 1). */ + ssh->handshake->e[0] = 0x01; + ssh->handshake->eSz = 1; + + if (wolfSSH_TestKeyAgreeDh_server(ssh, WC_HASH_TYPE_SHA256, f, &fSz) == 0) { + /* The degenerate peer public must not be accepted. */ + result = -732; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + +/* Verify KeyAgreeDh_client rejects a degenerate peer DH public key. The + * peer public 'f' is passed straight to the function; a value of 1 sits at the + * low end of RFC 4253 section 8's permitted [1, p-1] range, but it is a + * weak/degenerate value that the wc_DhCheckPubKey guard rejects (stricter than + * the RFC's bare minimum) before the shared secret is used. + * wolfSSH_TestSetDhKexKey installs the prime + * group and generates a real ephemeral key pair first, mirroring SendKexDhInit, + * so the rejection is driven by the bad peer key rather than a missing private + * exponent. This guard is defense-in-depth: wc_DhAgree in wolfCrypt also + * enforces the same range, so the test confirms the client rejection behavior + * but does not isolate the wolfSSH check from the underlying wolfCrypt one. */ +static int test_KeyAgreeDh_client_rejectsBadPeerPublic(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte f[1]; + int result = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -750; + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -751; + goto out; + } +#ifndef WOLFSSH_NO_DH_GROUP14_SHA256 + ssh->handshake->kexId = ID_DH_GROUP14_SHA256; +#elif !defined(WOLFSSH_NO_DH_GROUP14_SHA1) + ssh->handshake->kexId = ID_DH_GROUP14_SHA1; +#else + ssh->handshake->kexId = ID_DH_GROUP1_SHA1; +#endif + + /* Install the prime group on the client's ephemeral DH key, as the client + * KEX path does before KeyAgreeDh_client runs. */ + if (wolfSSH_TestSetDhKexKey(ssh) != 0) { + result = -752; + goto out; + } + + /* Degenerate peer public key f = 1 (yields the shared secret 1). */ + f[0] = 0x01; + + if (wolfSSH_TestKeyAgreeDh_client(ssh, WC_HASH_TYPE_SHA256, f, + (word32)sizeof(f)) == 0) { + /* The degenerate peer public must not be accepted. */ + result = -753; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + #if defined(WOLFSSH_SMALL_STACK) && defined(WOLFSSH_TEST_CAPTURING_ALLOCATOR) /* Size-tracked, poisoning capture allocator. AllocHeader stores the * user-requested size in front of each allocation so tests can filter @@ -3888,6 +4166,129 @@ static int test_KeyAgreeDh_server_zeroesEphemeralPrivKey(void) #endif /* WOLFSSH_SMALL_STACK && WOLFSSH_TEST_CAPTURING_ALLOCATOR */ #endif /* !WOLFSSH_NO_DH */ +#if !defined(WOLFSSH_NO_ECDH) && !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) +/* Verify KeyAgreeEcdh_server rejects an off-curve peer ECC point. The peer + * point is read from ssh->handshake->e as an X9.63 uncompressed point; (1, 1) + * is a well-formed encoding that is not on P-256. In builds without + * WOLFSSL_VALIDATE_ECC_IMPORT (the common embedded case this hardening targets) + * wc_ecc_import_x963 accepts the coordinates, so the wc_ecc_check_key call in + * the shared EccCheckPeerKey helper is what rejects it before + * wc_ecc_shared_secret is reached; with WOLFSSL_VALIDATE_ECC_IMPORT the import + * itself rejects it. Either way the function must fail. P-256 has cofactor 1, + * so an off-curve point (not a wrong-subgroup one) is the meaningful test. */ +static int test_KeyAgreeEcdh_server_rejectsOffCurvePoint(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte f[MAX_KEX_KEY_SZ]; + word32 fSz = (word32)sizeof(f); + int result = 0; + /* X9.63 uncompressed point for P-256: 0x04 || X(32) || Y(32), here the + * off-curve point (X=1, Y=1). */ + byte point[1 + 32 + 32]; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -740; + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -741; + goto out; + } + ssh->handshake->kexId = ID_ECDH_SHA2_NISTP256; + + WMEMSET(point, 0, sizeof(point)); + point[0] = 0x04; /* uncompressed */ + point[32] = 0x01; /* X = 1 (big-endian, last byte) */ + point[64] = 0x01; /* Y = 1 */ + WMEMCPY(ssh->handshake->e, point, sizeof(point)); + ssh->handshake->eSz = (word32)sizeof(point); + + if (wolfSSH_TestKeyAgreeEcdh_server(ssh, WC_HASH_TYPE_SHA256, f, &fSz) + == 0) { + /* The off-curve peer point must not be accepted. */ + result = -742; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + +/* Verify KeyAgreeEcdh_client rejects an off-curve peer ECC point. The peer + * point 'f' is an X9.63 uncompressed encoding; (1, 1) is well-formed but is + * not on P-256. In builds without WOLFSSL_VALIDATE_ECC_IMPORT (the common + * embedded case this hardening targets) wc_ecc_import_x963 accepts the + * coordinates, so the wc_ecc_check_key call in the shared EccCheckPeerKey + * helper is what rejects it before wc_ecc_shared_secret is reached; with + * WOLFSSL_VALIDATE_ECC_IMPORT the import itself rejects it. Either way the + * function must fail. P-256 has cofactor 1, so an off-curve point (not a + * wrong-subgroup one) is the meaningful test. */ +static int test_KeyAgreeEcdh_client_rejectsOffCurvePoint(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + int result = 0; + /* X9.63 uncompressed point for P-256: 0x04 || X(32) || Y(32), here the + * off-curve point (X=1, Y=1). */ + byte point[1 + 32 + 32]; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -770; + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -771; + goto out; + } + ssh->handshake->kexId = ID_ECDH_SHA2_NISTP256; + + /* Generate a real client ephemeral P-256 key, as SendKexDhInit() does + * before KeyAgreeEcdh_client() runs. With a live private key in place the + * only way the call can fail is the peer point validation, so the test + * actually exercises the wc_ecc_check_key guard rather than tripping over a + * missing private key. KeyAgreeEcdh_client() frees privKey.ecc on the way + * out, so the cleanup path stays well defined. */ + if (wc_ecc_init(&ssh->handshake->privKey.ecc) != 0) { + result = -772; + goto out; + } + ssh->handshake->useEcc = 1; +#ifdef HAVE_WC_ECC_SET_RNG + if (wc_ecc_set_rng(&ssh->handshake->privKey.ecc, ssh->rng) != 0) { + result = -772; + goto out; + } +#endif + if (wc_ecc_make_key_ex(ssh->rng, 32, &ssh->handshake->privKey.ecc, + ECC_SECP256R1) != 0) { + result = -772; + goto out; + } + + WMEMSET(point, 0, sizeof(point)); + point[0] = 0x04; /* uncompressed */ + point[32] = 0x01; /* X = 1 (big-endian, last byte) */ + point[64] = 0x01; /* Y = 1 */ + + if (wolfSSH_TestKeyAgreeEcdh_client(ssh, WC_HASH_TYPE_SHA256, point, + (word32)sizeof(point)) == 0) { + /* The off-curve peer point must not be accepted. */ + result = -773; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} +#endif /* !WOLFSSH_NO_ECDH && !WOLFSSH_NO_ECDH_SHA2_NISTP256 */ + #if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS) && \ !defined(NO_FILESYSTEM) && !defined(WOLFSSL_NUCLEUS) && \ !defined(_WIN32) && !defined(WOLFSSH_ZEPHYR) @@ -5114,6 +5515,13 @@ int wolfSSH_UnitTest(int argc, char** argv) testResult = testResult || unitResult; #endif +#ifdef WOLFSSH_TEST_INTERNAL + unitResult = test_DoReceive_RejectsShortPadding(); + printf("DoReceiveRejectsShortPadding: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif + #if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256) unitResult = test_DhGexGroupValidate(); printf("DhGexGroupValidate: %s\n", @@ -5211,6 +5619,11 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_DoUserAuthRequest_rejectsPasswordChange(); + printf("DoUserAuthRequest_rejectsPasswordChange: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + unitResult = test_SendUserAuthFailure_emptyMethods(); printf("SendUserAuthFailure_emptyMethods: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); @@ -5268,6 +5681,16 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_KeyAgreeDh_server_rejectsBadPeerPublic(); + printf("KeyAgreeDh_server_rejectsBadPeerPublic: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + + unitResult = test_KeyAgreeDh_client_rejectsBadPeerPublic(); + printf("KeyAgreeDh_client_rejectsBadPeerPublic: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + #if defined(WOLFSSH_SMALL_STACK) && defined(WOLFSSH_TEST_CAPTURING_ALLOCATOR) unitResult = test_KeyAgreeDh_server_zeroesEphemeralPrivKey(); printf("KeyAgreeDh_server_zeroesEphemeralPrivKey: %s\n", @@ -5276,6 +5699,18 @@ int wolfSSH_UnitTest(int argc, char** argv) #endif #endif /* !WOLFSSH_NO_DH */ +#if !defined(WOLFSSH_NO_ECDH) && !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) + unitResult = test_KeyAgreeEcdh_server_rejectsOffCurvePoint(); + printf("KeyAgreeEcdh_server_rejectsOffCurvePoint: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + + unitResult = test_KeyAgreeEcdh_client_rejectsOffCurvePoint(); + printf("KeyAgreeEcdh_client_rejectsOffCurvePoint: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif /* !WOLFSSH_NO_ECDH && !WOLFSSH_NO_ECDH_SHA2_NISTP256 */ + #endif #ifdef WOLFSSH_TEST_CERTMAN_PROMOTE diff --git a/wolfssh/internal.h b/wolfssh/internal.h index f9cb3daea..e94a10faa 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -693,7 +693,6 @@ typedef struct HandshakeInfo { byte useEcc:1; byte useEccMlKem:1; byte useCurve25519:1; - byte useCurve25519MlKem:1; #ifdef WOLFSSH_TPM byte useTpm:1; #endif @@ -1433,7 +1432,14 @@ enum WS_MessageIdLimits { const byte* f, word32 fSz); WOLFSSH_API int wolfSSH_TestKeyAgreeDh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz); + WOLFSSH_API int wolfSSH_TestSetDhKexKey(WOLFSSH* ssh); #endif /* !WOLFSSH_NO_DH */ +#ifndef WOLFSSH_NO_ECDH + WOLFSSH_API int wolfSSH_TestKeyAgreeEcdh_server(WOLFSSH* ssh, byte hashId, + byte* f, word32* fSz); + WOLFSSH_API int wolfSSH_TestKeyAgreeEcdh_client(WOLFSSH* ssh, byte hashId, + const byte* f, word32 fSz); +#endif /* !WOLFSSH_NO_ECDH */ #ifndef WOLFSSH_NO_DH_GEX_SHA256 WOLFSSH_API int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx);