Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set output and input encodings separately at end of KEX #205

Merged
merged 1 commit into from
Oct 23, 2021

Conversation

tomaswolf
Copy link
Member

Once SSH_MSG_NEWKEYS is sent any subsequent packet sent must use the
new encoding settings. Once SSH_MSG_NEWKEYS is received, the new
encoding settings are to be used for any message received. So set the
cipher/mac/compression separately for outgoing and incoming messages
in sendNewKeys() and handleNewKeys().

Previously, we set both only in handleNewKeys(), i.e., when the peer's
SSH_MSG_NEWKEYS was received. This makes implementing a KEX extension
handler more complicated than necessary since it had to delay sending
the SSH_MSG_EXT_INFO packet until after the peer's SSH_MSG_NEW_KEYS was
received.

RFC 8308 recommends that "the server sends its SSH_MSG_EXT_INFO not
only as the next packet after SSH_MSG_NEWKEYS, but without delay". This
is now possible since the output settings are already set up correctly.

SSH_MSG_EXT_INFO is always sent and received after SSH_MSG_NEWKEY, and
the Apache MINA sshd implementation guarantees that either party handles
the peer's SSH_MSG_NEWKEY after having sent its own SSH_MSG_NEWKEY.

@tomaswolf tomaswolf requested a review from gnodet October 17, 2021 16:02
@tomaswolf
Copy link
Member Author

@benhumphreys since this changes sending of server-sig-algs and you apparently want to use that in Bitbucket Server you might want to double-check this change, too. (It fixes the remaining issue of sending the EXT_INFO message "without delay" as recommended by RFC 8308.)

@benhumphreys
Copy link

Thanks @tomaswolf. I've also tested this branch and have found no problems.
I expanded my testing a little to include OpenSSH 6.6.1p1, 8.1p, 8.2p1 and 8.8p1.

Thanks again!

Once SSH_MSG_NEWKEYS is sent any subsequent packet sent must use the
new encoding settings. Once SSH_MSG_NEWKEYS is received, the new
encoding settings are to be used for any message received. So set the
cipher/mac/compression separately for outgoing and incoming messages
in sendNewKeys() and handleNewKeys().

Previously, we set both only in handleNewKeys(), i.e., when the peer's
SSH_MSG_NEWKEYS was received. This makes implementing a KEX extension
handler more complicated than necessary since it had to delay sending
the SSH_MSG_EXT_INFO packet until after the peer's SSH_MSG_NEW_KEYS was
received.

RFC 8308 recommends that "the server sends its SSH_MSG_EXT_INFO not
only as the next packet after SSH_MSG_NEWKEYS, but without delay". This
is now possible since the output settings are already set up correctly.

SSH_MSG_EXT_INFO is always sent and received after SSH_MSG_NEWKEY, and
the Apache MINA sshd implementation guarantees that either party handles
the peer's SSH_MSG_NEWKEY *after* having sent its own SSH_MSG_NEWKEY.
@tomaswolf tomaswolf merged commit 956dcbb into apache:master Oct 23, 2021
@tomaswolf tomaswolf deleted the sshd-1216 branch June 7, 2022 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants