Bug 2929 - OpenSSH server should not send the SSH_MSG_EXT_INFO message after rekeying
Summary: OpenSSH server should not send the SSH_MSG_EXT_INFO message after rekeying
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 7.7p1
Hardware: Other Linux
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_8_0
  Show dependency treegraph
 
Reported: 2018-11-14 01:36 AEDT by Jakub Jelen
Modified: 2021-10-14 01:40 AEDT (History)
4 users (show)

See Also:


Attachments
only consider ext-info-c during initial KEX (1.12 KB, patch)
2018-11-16 13:23 AEDT, Damien Miller
dtucker: ok+
Details | Diff
check KEX_INITIAL before sending ext-info (794 bytes, patch)
2019-09-04 09:27 AEST, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelen 2018-11-14 01:36:48 AEDT
The RFC 8308 specifies, that the SSH_MSG_EXT_INFO message should be sent after the *first* SSH_MSG_NEWKEYS message, while the OpenSSH server sends it also after the rekey:

>    o  As the next packet following the server's first SSH_MSG_NEWKEYS.

Side note:
The draft-ssh-ext-info-04  from [1] is already RFC [2], so the page could make use of an update. Also the draft-rsa-dsa-sha2-256-03 is already RFC [3].

[1] http://www.openssh.com/specs.html
[2] https://tools.ietf.org/html/rfc8308
[3] https://tools.ietf.org/html/rfc8332
Comment 1 Darren Tucker 2018-11-14 21:24:45 AEDT
(In reply to Jakub Jelen from comment #0)
> The RFC 8308 specifies, that the SSH_MSG_EXT_INFO message should be
> sent after the *first* SSH_MSG_NEWKEYS message, while the OpenSSH
> server sends it also after the rekey:
> 
> >    o  As the next packet following the server's first SSH_MSG_NEWKEYS.

Under what conditions does the server send SSH_MSG_EXT_INFO a second time?  The client removes it from the proposal once sent:

sshconnect2.c:().
        /* remove ext-info from the KEX proposals for rekeying */
        myproposal[PROPOSAL_KEX_ALGS] =
            compat_kex_proposal(options.kex_algorithms);

and kex.c sets the handler to return an error after the first instance:

kex.c:kex_input_ext_info():
        debug("SSH2_MSG_EXT_INFO received");
        ssh_dispatch_set(ssh, SSH2_MSG_EXT_INFO, &kex_protocol_error);

even removing that dispatch_set on the client side I can only see a single SSH2_MSG_EXT_INFO received on the client side.

> Side note:
> The draft-ssh-ext-info-04  from [1] is already RFC [2], so the page
> could make use of an update. Also the draft-rsa-dsa-sha2-256-03 is
> already RFC [3].
> 
> [1] http://www.openssh.com/specs.html
> [2] https://tools.ietf.org/html/rfc8308
> [3] https://tools.ietf.org/html/rfc8332

Fixed those, thanks.  I periodically check them but don't always catch status changes.
Comment 2 Jakub Jelen 2018-11-15 20:13:58 AEDT
I noticed this with different client than OpenSSH. This indeed happens when the client sends the ext-info-c also with the rekeying SSH_MGS_KEXINIT, which looks like wrong thing to do (and which I probably missed in the first reads of the rfc):

>   Applications implementing this mechanism MUST add one of the
>   following indicator names to the field kex_algorithms in the
>   SSH_MSG_KEXINIT message sent by the application in the first key
>   exchange:

In that case, I will make sure this is fixed in libssh does not append the ext-info-c to the rekeying requests.

But even though the client did not do the correct thing, I think server should not be manipulated to do the bad thing either.

The message sending is triggered directly by setting the ext_info from the current key exchange, but I think, there should be also a condition to skip the call to kex_send_ext_info() if we are in state of rekeying.

https://github.com/openssh/openssh-portable/blob/120a1ec7/kex.c#L421
Comment 3 Damien Miller 2018-11-16 13:23:32 AEDT
Created attachment 3205 [details]
only consider ext-info-c during initial KEX
Comment 4 Damien Miller 2018-12-07 14:39:57 AEDT
applied - this will be in OpenSSH 8.0
Comment 5 Pawel Jakub Dawidek 2019-09-04 06:04:35 AEST
Hi Damian,

I think the fix is incomplete. It probably only works with the OpenSSH server when sandboxing is enabled, but it doesn't work with ssh_api.c.

When using API, the kex structure is allocated only once and during the first KEX the ext_info_c field is set to 1. It is then never set to 0, so during next rekeying, even though KEX_INITIAL is no longer set, the SSH_MSG_EXT_INFO will be send again as ext_info_c remains 1.

To fix that it would be enough to add:

kex->ext_info_c = 0;

right after:

kex->flags &= ~KEX_INITIAL;

in the kex_input_newkeys() function.

Thank you.
Comment 6 Damien Miller 2019-09-04 09:27:42 AEST
Created attachment 3316 [details]
check KEX_INITIAL before sending ext-info

IMO it's better to check KEX_INITIAL. Add some debug() to make it clear whether/when the ext-info is sent.

Note that disabling privsep is not supported (there is no option) and the API is still very much a work in progress.
Comment 7 Darren Tucker 2019-09-04 09:41:50 AEST
Comment on attachment 3316 [details]
check KEX_INITIAL before sending ext-info

> (ssh->kex->flags & KEX_INITIAL) != 0

given that it's being used as a boolean you could omit the != 0.
Comment 8 Damien Miller 2020-08-28 13:26:31 AEST
This was fixed in openssh-8.1 last year
Comment 9 Damien Miller 2021-04-23 15:03:32 AEST
closing resolved bugs as of 8.6p1 release
Comment 10 Ahmed Sayeed 2021-10-14 01:40:18 AEDT
[spam removed]