Re: [Curdle] Spencer Dawkins' Yes on draft-ietf-curdle-ssh-ext-info-12: (with COMMENT)

[Spencer Dawkins at IETF <[e-mail]>]

Hi, Denis,

On Wed, Sep 13, 2017 at 1:11 AM, denis bider <[e-mail]>
wrote:

> Hello,
>
> replies below:
>
>
> > In this text,
> >
> >  Implementations MUST NOT send an incorrect indicator name for their
> >  role. Implementations MAY disconnect if the counter-party sends an
> >  incorrect indicator. If "ext-info-c" or "ext-info-s" ends up being
> >  negotiated as a key exchange method, the parties MUST disconnect.
> >
> > why would a party that doen't support this extention disconnect?
>
> This language clarifies a corner case that's not expected to happen.
> Algorithm negotiation in SSH works like this:
>
> For each algorithm type (e.g. key exchange) and direction (for some
> algorithm types, e.g. encryption):
>
> - Server sends a comma-separated namelist of algorithm names in the
> initial KEX_INFO message. For example: "foo,bar,baz"
> - Client sends a similar list. For example: "baz,bar"
> - The negotiated algorithm is (1) the first algorithm in the client's list
> that (2) also appears in the server's list.
>
> In the above example, the negotiated algorithm is "baz".
>
> The "ext-info-s" and "ext-info-c" indicators are chosen so that two
> correct implementations will never negotiate either, because only clients
> send "ext-info-c", and only servers send "ext-info-s". If one of these
> algorithms is negotiated, it means either one of the sides is seriously
> buggy, or there's an attempt at some kind of attack. Therefore,
> implementations should disconnect.
>
> A party that doesn't support EXT_INFO will never see "ext-info-c" or
> "ext-info-s" being negotiated because it did not send it in its algorithm
> list. Therefore, it cannot be negotiated.
>

Bingo - that answers my question.

I was just making sure that a properly implemented SSH would disconnect
because this happens as a result of a protocol violation, so it works with
this way for SSLs that don't support this specification.

I'll leave determining the proper level of paranoia beyond that to the
professionals :-)


> This is unless the party that doesn't support EXT_INFO is sending randomly
> generated algorithm names and happens to hit upon that one. I trust no
> honest implementation does this, except for fuzzing. If something like
> fuzzing results in "ext-info-s" or "ext-info-c" being accidentally
> negotiated, the aware implementation - of course - is the one that
> disconnects.
>
>
> >   If this extension takes effect, the client MUST send the following
> >   message shortly after receiving SSH_MSG_USERAUTH_SUCCESS:
> >
> >      byte       SSH_MSG_NEWCOMPRESS (value 8)
> >
> > I THINK the point is that the client's SSH_MSG_NEWCOMPRESS
> > is sent after SSH_MSG_USERAUTH_SUCCESS, before the client
> > sends its first SSH message that's compressed using the newly
> > negotiated compression algorithm
>
> The implication here is that the client might have existing messages in
> flight to the server when the server sends SSH_MSG_USERAUTH_SUCCESS. This
> is not often the case, but it is possible: for example, user authentication
> takes some time, and the client sends a keep-alive message.
>
> Because of this potential race condition, the server must wait until it
> receives SSH_MSG_USERAUTH_SUCCESS before it assumes that compression in the
> client-to-server direction is in effect.
>
> The language says "shortly" because it's hard to say what delay is
> permissible here. Consider an intentionally pessimized case:
>
> - Suppose the server took extremely long to process the authentication
> attempt. It's not unheard of that login processing can take 5 minutes.
> - Suppose the client is set up to send frequent keep-alive messages to
> prevent routers from disconnecting the TCP session, but it does not require
> the server to respond to them.
> - Suppose the server blocks and does nothing while it's processing the
> login attempt.
>
> In these circumstances, the server may send SSH_MSG_USERAUTH_SUCCESS, only
> to find in its TCP input buffer a large number of keep-alive messages from
> the client which were sent while the server was processing. The server
> needs to be able to handle all of these messages without compression,
> before it can expect to receive the client's SSH_MSG_NEWCOMPRESS.
>
> My intent here was to encourage clients to send SSH_MSG_NEWCOMPRESS as
> soon as they are able, while at the same time encouraging servers to
> tolerate any reasonable number of packets from the client before this
> message is received - where "reasonable" may be, to some extent,
> implementation-defined.
>
> Based on this feedback, I'm leaning toward changing this language as
> follows:
>
> "If this extension takes effect, the client MUST send the following message
> within a reasonable number of outgoing messages after receiving
> SSH_MSG_USERAUTH_SUCCESS - but not necessarily as the first such outgoing
> message:"
>
> Maybe that makes it more clear. The existing, immediately following
> paragraph attempts to explain the context without trying to dwell on it too
> long:
>
>     The purpose of NEWCOMPRESS is to avoid a race condition where the
>     server cannot reliably know whether a message sent by the client was
>     sent before or after receiving the server's USERAUTH_SUCCESS.
>

Your proposed new text, together with the following paragraph, is clear
enough for me. Thanks for that.


> > I would find this easier to understand, if the paragraph defining the
> > terms was the first paragraph in the section, and then the description
> > of the extension (which uses the term "elevation") followed.
>
> No problem. Will move.
>
>
> > but I could imagine that adding elevation is the first step toward fewer
> > SSH server implementations that always run with administrative rights
> > just in case they ever need to use them, so the attack surface is
> > getting smaller?
>
> Yes, that is the case. If clients can't be expected to implement the
> "elevation" extension, Windows servers must elevate administrative users by
> default.
>
> In a parable - without "elevation", all sessions from administrative users
> have to run as "root" (on Windows), because there's no way to "sudo".
>
>
> > ... And now I see that Mirja also asked about elevation, and your
> answer to
> > her was pretty much what I had guessed.  Maybe it's worth summarizing
> > your answer in section 3.4.
>
> I will do so. :-)
>

This all seems perfect to me, Thanks.

Spencer