Conversation
We'll assume that KEX isn't going to run during auth, and the buffer space is adequate. This can be revisited if needed.
Can be used to avoid sending data and let the output buffer flush
These states are when a KexInit needs to be sent but TrafOut isn't drained.
A subset of Packet variants can be enqueued to a separate deferred packet queue. This variant is a lot smaller than Packet. The deferred packets need to be kept non-encrypted since they will be sent out-of-order with KEX packets that might continue being sent.
Window adjust will be skipped while KEX is in progress.
Previously CliSessionOpener was emitted when an OpenConfirmation packet was received. That's a problem since it could occur while a KEX is in progress. Instead emit the event from progress() This could still encounter a full output queue - maybe the queue should be drained before emitting the event. In practise this is unlikely for single sessions. When TCP forwarding is added a similar problem will need to be solved, that will have greater chance of hitting full buffers.
Non-KEX packets can't be sent during a key exchange, so put them on the deferred queue.
Rekeying has two purposes. It avoids the packet sequence counter wrapping (32 bit), and avoids too many blocks being encrypted with the same key (applicable to AES).
No functional changes needed, comment the cases involved so that future changes can take note.
Some message types not handled by DeferredPacket aren't likely to be encountered in the wild. The fuzzer shouldn't worry about those.
No noticable changes
Not needed now that we have the CS: CliServ parameter
conn.resume_servauth() can fail if the send buffer is full. In that case the runner.resume_servauth() still needs to call resume_nocheck() so that the event is completed - otherwise on the next call to progress(), BadUsage will be emitted. This shouldn't generally happen (auth packets are assumed to be sent early with adequate buffer space), was caught by fuzzing. Further changes will be made later so that the BusySend can be returned from progress() (it's lost by the drop handler currently).
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
1 participant
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.
This implements sending rekey requests after a certain data count is reached.
To handle rekey properly, it needs to be able to defer sending packets while the KEX is in progress. (Only KEX packets are allowed to be sent after sending KEXINIT, until a NEWKEYS). In-flight packets may be received after sending the KEXINIT, and some of those could need responses. A new
DeferredPacketmechanism handles that.The same BusySend handling is also used to avoid running out of WindowAdjust space - those are now by the
Channels::progress(). Some other responses can also be deferred.There are a few caveats.
DeferredPacketis small, so some packet types such as userauth packets can't be deferred.That's OK because they should occur when there is send buffer available.
Channel open requests from a sunset client may fail, the application will need to retry after a
BusySend. In future perhaps async retries could occur.This has been tested with fuzzing.
@jubeormk1 this replaces the "Workaround for NoRoom sending window adjustment" commit 2869501
Doesn't improve #40
Closes #19