fix: send SSH_MSG_DISCONNECT on KEX failure

[MarkAtwood]

Bug

DoKexInit() in src/internal.c sets ret to one of the negotiation-failure codes WS_MATCH_KEX_ALGO_E / WS_MATCH_KEY_ALGO_E / WS_MATCH_ENC_ALGO_E / WS_MATCH_MAC_ALGO_E when no common KEX / host-key / cipher / MAC algorithm can be negotiated, then returns. That code propagates unchanged through DoPacket() and DoReceive(), which set ssh->error and return WS_FATAL_ERROR. No SSH_MSG_DISCONNECT is ever sent on these paths, so the peer is dropped without notification.

RFC 4253 s7.1 covers exactly this case: when no algorithm satisfying all the conditions can be found, "the connection fails, and both sides MUST disconnect." wolfSSH already terminated the connection, so the MUST was met -- what was missing is the notification telling the peer why. The library already sends one best-effort with (void)SendDisconnect(...) for service-request/accept errors (internal.c:7703, 7735); the KEX negotiation paths were inconsistent with that pattern.

Fix

Two changes, sharing one predicate:

1. At the single DoKexInit() exit point, send a best-effort SSH_MSG_DISCONNECT with reason WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED (3) when ret is any of the four match-failure codes. Uses the same (void)SendDisconnect best-effort form as the existing call sites. ret is not overwritten for match errors -- the SendKexInit/ssh->error propagation block is gated on ret == WS_SUCCESS.

2. In DoPacket(), don't answer a KEXINIT that failed to negotiate. The MSGID_KEXINIT case ran its rekey response (SendKexDhInit() / SendKexDhGexRequest()) based only on ssh->isKeying and ssh->connectState, never consulting ret, so a post-auth rekey with a mismatch could emit a KEXDH packet immediately after the disconnect just sent.

Both sites test the same predicate through a shared helper:

static int IsKexMatchError(int ret)
{
    return ret == WS_MATCH_KEX_ALGO_E || ret == WS_MATCH_KEY_ALGO_E ||
        ret == WS_MATCH_ENC_ALGO_E || ret == WS_MATCH_MAC_ALGO_E;
}

The second gate cannot be written as ret == WS_SUCCESS. A healthy client-side rekey returns WS_REKEYING (-1035) out of DoKexInit() -- a status, not an error, carried by the trailing if (ssh->error != 0) ret = ssh->error; that propagates a want-write from SendKexInit(). Gating on WS_SUCCESS suppresses every rekey response, not just failing ones, and fails the SCP rekey test at tests/api.c:3030.

Verification

Built --enable-all against a full wolfSSL install. make check: 10 pass, 1 skip, 0 fail.

Behavior confirmed on the wire with a raw probe that completes the version exchange, sends a KEXINIT with one bogus name-list, and reads to EOF. Against examples/echoserver, for each of the four negotiation fields (kex, host key, cipher, MAC): master replies with its own KEXINIT and then closes silently; with this change it replies KEXINIT followed by SSH_MSG_DISCONNECT with reason 3. An OpenSSH client will not surface this -- it computes the algorithm intersection itself and aborts before reading the server's reply, which is why a raw probe was needed.

Regression coverage for the new disconnect side effect is deferred; tests/regress.c asserts the WS_MATCH_* return codes but does not yet capture the outbound packet.

Reported by static analysis (Fenrir finding 608). Severity Low -- the connection already terminated; this adds the reason code.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Ensures the library sends a best-effort SSH_MSG_DISCONNECT when KEX negotiation fails due to no mutually supported algorithms, aligning behavior with RFC 4253 guidance and existing disconnect-on-error patterns in the codebase.

Changes:

• Adds a best-effort (void)SendDisconnect(...) call in DoKexInit() when KEX/hostkey/cipher/MAC matching fails.
• Uses WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED as the disconnect reason for these negotiation failures.

---

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

[aidangarske]

🐺 Skoll Code Review

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

Posted findings

• [High] Disconnect can be followed by another KEX packet during rekey failure — src/internal.c:5335-5338
• [Medium] Missing regression coverage for the new disconnect side effect — src/internal.c:5335-5338

Review generated by Skoll.

Unable to rereview.