Validate peer ECDH public key before shared-secret in key agreement

[yosuke-wolfssl]

Summary

Adds an explicit wc_ecc_check_key() validation of the peer-supplied ECDH public value in the server-side key-agreement paths, mirroring the public-value validation already performed for Curve25519.

• KeyAgreeEcdh_server (pure ECDH: ecdh-sha2-nistp256/384/521)
• KeyAgreeEcdhMlKem_server (ECC branch of the hybrid ML-KEM kex)

In both, the client's e value is imported with wc_ecc_import_x963_ex() and then fed to wc_ecc_shared_secret(). The new check sits between them and returns the wolfCrypt error via the existing ret chain on failure.

Addressed by f_5691.

Background / scope

This was raised as a missing-public-value-validation issue (potential pre-auth degenerate/off-curve point injection). On investigation, the reported severity is overstated for current wolfSSL:

• wc_ecc_import_x963_ex() imports as untrusted (wc_ecc_import_x963_ex2(..., untrusted=1)) and already rejects the point at infinity and off-curve points via sp_ecc_is_point_* / wc_ecc_point_is_on_curve, independent of WOLFSSL_VALIDATE_ECC_IMPORT.
• Verified empirically: an off-curve P-256 point (generator with the last Y byte flipped) is rejected at import with -98 (MP_VAL), before wc_ecc_shared_secret() is reached.
• The NIST curves used here are prime-order (cofactor 1), so on-curve + not-infinity already implies the correct subgroup — wc_ecc_check_key() adds no exploitable protection over the existing import checks.

This change is therefore defense-in-depth / hardening, not a behavior-changing vulnerability fix. It pins the explicit-validation contract at the call site regardless of the underlying wolfCrypt import path, and matches the style of the sibling KeyAgreeCurve25519_server (which validates with wc_curve25519_check_public() — a check that genuinely matters there since Curve25519 has cofactor 8).

No regression test is included: for these prime-order curves any off-curve input is already rejected by the import, so such a test would pass with or without this change (equivalent mutant) and give false confidence.

Changes

/* KeyAgreeEcdh_server */
    if (ret == 0)
        ret = wc_ecc_import_x963_ex(ssh->handshake->e,
                                    ssh->handshake->eSz,
                                    pubKey, primeId);

    if (ret == 0)
        ret = wc_ecc_check_key(pubKey);

    if (ret == 0)
        ret = wc_ecc_make_key_ex(ssh->rng, ...);

The same if (ret == 0) ret = wc_ecc_check_key(pubKey); is added after the import in the ECC branch of KeyAgreeEcdhMlKem_server.

Testing

• ./configure --enable-all build: clean.
• unit.test and kex.test: pass — real ECDH and hybrid ML-KEM handshakes still succeed, confirming legitimate peers are not rejected.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1061

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: COMMENT
Findings: 1 total — 1 posted, 0 skipped

Posted findings

• [Medium] Pure ECDH server validation lacks direct handshake coverage — src/internal.c:12525-12526

Review generated by Skoll.

[aidangarske]

🟡 [Medium] Pure ECDH server validation lacks direct handshake coverage
💡 SUGGEST test

The PR adds wc_ecc_check_key(pubKey) to the pure ECDH server path after importing the client public value. The existing tests/kex.c end-to-end KEX coverage exercises DH and the hybrid ML-KEM variants, including the ECC ML-KEM branch touched by this PR, but it does not run a full ecdh-sha2-nistp* handshake through KeyAgreeEcdh_server. That leaves the newly added validation call in the pure ECDH server path without direct success-path coverage in the KEX test, so backend/config-specific behavior changes in wc_ecc_check_key would be easier to miss.

Recommendation: Consider adding an end-to-end pure ECDH KEX success test (e.g. wolfSSH_KexTest_Connect("ecdh-sha2-nistp256") guarded by WOLFSSH_NO_ECDH_SHA2_NISTP256, similar to the existing ML-KEM KEX cases) so both PR-modified server paths are exercised by the test suite.