tests: exercise DoKexDhReply host-key signature verify with corrupted signatures

[yosuke-wolfssl]

tests: exercise DoKexDhReply host-key signature verification with corrupted signatures

Summary

DoKexDhReply (client side) authenticates the server by verifying its
signature over the exchange hash ssh->h with the negotiated host key — RSA
(wc_SignatureVerify), ECC (wc_SignatureVerify), and Ed25519
(wc_ed25519_verify_msg). None of these verify calls was reached by any
negative test:

• RewriteKexDhReplySignatureName rewrote only the signature name string,
  which is rejected earlier at the name-match check (WS_PARSE_E).
• TestKexDhReplyRejectsNoPublicKeyCheck / …WhenCallbackRejects fail at the
  public-key-check callback, also before the crypto verify.

As a result, mutation testing showed the verify could be deleted without any
test failing — i.e. an attacker who knows only the server's public key could
impersonate the server / MITM the connection, and CI would stay green.

This PR adds negative regression tests that corrupt the signature data
(not the name) so the client reaches the actual cryptographic verify and must
reject the handshake. Test-only change — no production code is modified;
the verify logic in src/internal.c was already correct.

Addressed by f_6272.

What changed (tests/regress.c)

• Added a second mode to the existing duplex KEXDH_REPLY mutator
  (REGRESS_MUTATE_SIG_DATA) that keeps the signature name and flips one byte
  of the signature data. The name-rewrite and data-corrupt paths now share a
  single parametrized helper pair (RewriteSingleKexDhReplyPacket /
  RewriteKexDhReplyPacket) instead of duplicating the packet parse/locate
  logic.
• Parametrized the test harness with a host-key path so ECC and Ed25519 keys
  can be loaded (it previously hardcoded the RSA key).
• New tests, each paired with a positive control and guarded by the relevant
  feature macros:
  • TestKexDhReplyRejectsRsaSha2_256CorruptSig / …512 → WS_RSA_E
  • TestKexDhReplyRejectsEccCorruptSig (nistp256) → WS_ECC_E
  • TestKexDhReplyRejectsEd25519CorruptSig → WS_ED25519_E

Testing

• tests/regress.test passes.
• Confirmed each new test fails when its verify is temporarily neutered
  (deleted the RSA/ECC wc_SignatureVerify assignment; bypassed the Ed25519
  branch) — the negative controls prove the tests have teeth.

[Copilot]

Pull request overview

This PR strengthens wolfSSH’s regression test suite by ensuring the client-side DoKexDhReply path actually reaches and exercises the cryptographic host-key signature verification (RSA/ECC/Ed25519) under tampering, preventing a class of “verify removed but tests still pass” regressions.

Changes:

• Extended the existing duplex KEXDH_REPLY mutator to support a new mode that corrupts signature data (while keeping the signature name valid) so the handshake reaches the real verify call.
• Parameterized the KEXDH_REPLY harness to load different server host keys (RSA/ECC/Ed25519) via a passed-in key path.
• Added new negative tests (with positive controls) validating rejection/error codes for corrupted signatures across RSA-SHA2-256/512, ECDSA nistp256, and Ed25519.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1051

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 2 total — 2 posted, 0 skipped

Posted findings

• [High] Ed25519 key fixture is not linked for out-of-tree test runs — tests/regress.c:388,1101-1106
• [Medium] Non-RSA host-key tests remain hidden behind the RSA build gate — tests/regress.c:332-333,1090-1106

Review generated by Skoll.

Superseded by corrected review (findings now attached as inline comments).

[yosuke-wolfssl]

Hello @aidangarske , @ejohnstown ,
I fixed the issues you mentioned. Could you please review this again ?

[aidangarske]

over to @ejohnstown

[aidangarske]

Skoll Code Review

Scan type: review-securityOverall recommendation: APPROVE
Findings: 4 total — 4 posted, 0 skipped
4 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Low] Signature-name WMEMCMP branch still unexercised: replacement name "ssh-rsa" only ever trips the length check — tests/regress.c:792-793
• [Low] New ECC/Ed25519 name-downgrade tests do not pin the rejection error code — tests/regress.c:1012-1028
• [Info] Multi-line block comments violate repository comment policy — tests/regress.c:575-577,666-670,1088-1090,1133-1134
• [Info] Positive-control handshake runs twice per algorithm, doubling test runtime — tests/regress.c:1117,1126,1137,1146

Review generated by Skoll

[yosuke-wolfssl]

Hello @aidangarske ,
Could you please review this again ?