tests: exercise DoKexDhReply host-key signature verify with corrupted signatures#1051
Open
yosuke-wolfssl wants to merge 1 commit into
Open
tests: exercise DoKexDhReply host-key signature verify with corrupted signatures#1051yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1051
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests: exercise DoKexDhReply host-key signature verification with corrupted signatures
Summary
DoKexDhReply(client side) authenticates the server by verifying itssignature over the exchange hash
ssh->hwith the negotiated host key — RSA(
wc_SignatureVerify), ECC (wc_SignatureVerify), and Ed25519(
wc_ed25519_verify_msg). None of these verify calls was reached by anynegative test:
RewriteKexDhReplySignatureNamerewrote only the signature name string,which is rejected earlier at the name-match check (
WS_PARSE_E).TestKexDhReplyRejectsNoPublicKeyCheck/…WhenCallbackRejectsfail at thepublic-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.cwas already correct.Addressed by f_6272.
What changed (
tests/regress.c)(
REGRESS_MUTATE_SIG_DATA) that keeps the signature name and flips one byteof 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/locatelogic.
can be loaded (it previously hardcoded the RSA key).
feature macros:
TestKexDhReplyRejectsRsaSha2_256CorruptSig/…512→WS_RSA_ETestKexDhReplyRejectsEccCorruptSig(nistp256) →WS_ECC_ETestKexDhReplyRejectsEd25519CorruptSig→WS_ED25519_ETesting
tests/regress.testpasses.(deleted the RSA/ECC
wc_SignatureVerifyassignment; bypassed the Ed25519branch) — the negative controls prove the tests have teeth.