First packet follows check needs pubkey guess#927
First packet follows check needs pubkey guess#927ejohnstown wants to merge 2 commits intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates KEX negotiation handling to correctly evaluate the SSH first_packet_follows optimization by tracking and validating both the peer’s guessed KEX algorithm and guessed server host key algorithm (Issue F-1686).
Changes:
- Add
pubKeyIdGuesstoHandshakeInfoto store the peer’s first “server host key algorithms” preference from KEXINIT. - In
DoKexInit(), stash the peer’s host-key-algorithm guess (list[0]) alongside the existing KEX guess. - In
DoKexDhInit(), skip the first packet that follows KEXINIT when either the KEX guess or host-key guess differs from the negotiated algorithms.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
wolfssh/internal.h |
Extends handshake state to track the peer’s host-key algorithm guess for first_packet_follows validation. |
src/internal.c |
Records host-key guess during KEXINIT parsing and uses it in the first_packet_follows skip decision during KEXDH/ECDH init handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ejohnstown
force-pushed
the
first-kex-follows
branch
from
0765095 to
98db525
Compare
ejohnstown
force-pushed
the
first-kex-follows
branch
from
232a7a8 to
7be3476
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When processing the KEX Init message, stash guesses for the peer's KEX and public key algorithms. When reading first_packet_follows, if set check the guesses and set the handshake info flag ignoreNextKexMsg. When processing the KexDhInit message, check that flag. Affected functions: DoKexInit, DoKexDhInit. Issue: F-1686
ejohnstown
force-pushed
the
first-kex-follows
branch
from
7be3476 to
bafde45
Compare
Add a regression for checking the `first_kex_packet_follows` flag versus the guesses for KEX algorithm and public key algorithm.
ejohnstown
force-pushed
the
first-kex-follows
branch
from
bafde45 to
5798eb2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (ssh->handshake->ignoreNextKexMsg) { | ||
| /* skip this message. */ | ||
| WLOG(WS_LOG_DEBUG, "Skipping the client's KEX init function."); | ||
| ssh->handshake->kexPacketFollows = 0; | ||
| WLOG(WS_LOG_DEBUG, "Skipping client's KEXDH_INIT message due to " | ||
| "first_packet_follows guess mismatch."); | ||
| ssh->handshake->ignoreNextKexMsg = 0; | ||
| *idx += len; | ||
| return WS_SUCCESS; |
There was a problem hiding this comment.
ignoreNextKexMsg is only consumed/cleared in DoKexDhInit, but the first packet after KEXINIT can also be SSH_MSG_KEXDH_GEX_REQUEST (34) when DH-GEX is in play. If a peer sets first_packet_follows=1 and guesses GEX incorrectly, the guessed 34 message should be ignored per RFC 4253, but DoKexDhGexRequest() currently won’t check this flag and will process it instead. Consider applying the same “consume + clear flag + no state advance” behavior in DoKexDhGexRequest() (and any other possible first-KEX message handlers) so first_packet_follows mismatches are handled uniformly across KEX methods.
When processing the KEX Init message, stash guesses for the peer's KEX and public key algorithms. When reading first_packet_follows, if set check the guesses and set the handshake info flag ignoreNextKexMsg. When processing the KexDhInit message, check that flag.
Affected functions: DoKexInit, DoKexDhInit.
Issue: F-1686