Re: [PATCH] Allow SSH2_MSG_EXT_INFO when authenticated

[Anderson Sasaki <ansasaki@xxxxxxxxxx>]

Hello,

----- Original Message -----
> From: "Jakub Jelen" <jjelen@xxxxxxxxxx>
> To: libssh@xxxxxxxxxx
> Sent: Tuesday, December 11, 2018 12:10:34 PM
> Subject: Re: [PATCH] Allow SSH2_MSG_EXT_INFO when authenticated
> 
> On Mon, 2018-12-10 at 09:10 -0500, Anderson Sasaki wrote:
> > Hello,
> > 
> > Continuing the investigation of the curl issue [1], I found the
> > actual problem which is a regression introduced by the CVE-2018-10933
> > fix.
> > The SSH_MSG_EXT_INFO, used in key exchange, is being filtered when
> > the user is already authenticated. This breaks the re-keying.
> > Follows attached a patch to fix this regression. It can be reviewed
> > in gitlab [2].
> 
> I don't think your patch is right. The SSH_MSG_EXT_INFO is acceptable
> only during the first key exchange. See the discussion in OpenSSH bug
> about this + the RFC:
> 
> https://bugzilla.mindrot.org/show_bug.cgi?id=2929
> 
> >    o  As the next packet following the server's first
> SSH_MSG_NEWKEYS. [0]
> 
> The bug in OpenSSH server was that it send the EXT_INFO when the ext-
> info-c was sent by the libssh client in the rekey request (also
> wrongly, but already fixed in [1]).
> 
> [0] https://tools.ietf.org/html/rfc8308#section-2.4
> [1] https://gitlab.com/jjelen/libssh-mirror/commit/83f2ac4a

Thanks for pointing this.

Yes, I knew that, and discussed with Andreas about the RFC and that OpenSSH is not compliant.
We agreed that OpenSSH needs a patch, but also that we can allow it to not break users who are trying to access servers which are not compliant.
We know that being strict in this caused problems for users, like in curl case [0].

Sorry for not being clear about this in my patch..
I'm afraid we will have to keep this for a long time, even after OpenSSH is fixed, until the fixed version is widely adopted.

Anderson

[0] https://github.com/curl/curl/issues/3310