Fix client state hostkey wait timeout handling#6
Open
precla wants to merge 2 commits into
Open
Conversation
Signed-off-by: Antonio Prcela <antonio.prcela@sartura.hr>
Signed-off-by: Antonio Prcela <antonio.prcela@sartura.hr>
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.
i've found one possible issue and i have a test for it to reproduce the issue. both with the help of Claude, i did double check the code and the flow to verify it
connect_timeoutdoesn't cover host-key verification timeout withinon_hostkey()for example if
on_hostkey()takes too long for whatever reason or ifaccept_hostkey()/reject_hostkey()are not called (even tho the last one doesn't make sense to not call one of the two..) then the timeout ofc->connect_timeris never reached since it is stopped too soon in the moment the transport handshake completes (CLIENT_STATE_CONNECTING->CLIENT_STATE_HOSTKEY), before host-key verification begins. once the client reachesCLIENT_STATE_HOSTKEY_WAIT, no timer is armed (the auth timer only starts later)the ssh client's
connect_timeoutshould handle the entire "getting connected" phase. currently is basically handles onlyCLIENT_STATE_CONNECTING, but notCLIENT_STATE_HOSTKEY_WAIT. which happens in between when the client receives the server's host key and is waiting foron_hostkey()handler to callaccept_hostkey()/reject_hostkey()(or for whatever reason do some other 'magic')the states
CLIENT_STATE_HOSTKEY_WAITandCLIENT_STATE_HOSTKEYare checked inconnect_timeout_cb()so code should time-out in that case but it never does since those states never reach theconnect_timeout_cb()ifon_hostkey()takes too longif the
on_hostkey()handler never responds then the client hangs forever.on_connect()is never callednote: the timeout only survives into
CLIENT_STATE_HOSTKEY_WAITon a synchronous connect, which never occurs over a non-blocking socketproposed fix: stop the connect timer when authentication begins in
client_start_auth_timer(), not when the transport connectstest
_test_hostkey_stalladded