Subject: Re: Feedback on draft-ssh-ext-info-00

From: Damien Miller <[email-masked]>
Date: Thu, 3 Dec 2015 14:10:04 +1100 (AEDT)

On Thu, 3 Dec 2015, denis bider wrote:

> > IMO it would be best to make it a MUST that the flags
> > may only be sent as the last element. What rationale is
> > there for putting it anywhere else?
> 
> In the event that there are two or more competing extension mechanisms, only
> one of them can be last.

If there's a functioning extension mechanism, then why would there be
competition?

Even if there were, then it's up to whoever writes the second one to
deal with making their fit.

> > and don't "replace" any messages like
> > SSH_MSG_SERVICE_REQUEST.
> 
> But but but I waaaant to replace this awful SSH_MSG_SERVICE_REQUEST. :)
> 
> Does no one agree that SERVICE_REQUEST + ACCEPT are pointless?
> 
> A whole round-trip delay? For no benefit - in any known usage scenario?

IMO replacing messages in the protocol is something best saved for
SSH-3.x. IIRC I've heard of at least one server that doesn't implement
ssh-userauth.

> > OpenSSH has sent various @openssh.com global
> > requests for years, so most of the implementations
> > that violate this should have long since been shaken out.
> 
> Do you happen to know:
> 
> - Since which version the OpenSSH server has been sending global requests?

3.8ish, released early 2004

> - Since which version the OpenSSH client has been tolerating them?

at least that long :)

> There are still folks out there using OpenSSH 3.x and 4.x. I'm pretty sure I
> saw our software being connected to by a client like that in the past week.

4.x should be fine, older 3.x might have problems - I haven't checked.

Honestly though, anyone using OpenSSH 3.x has bigger problems than
channel requests upsetting their connections.

> > (e.g. it's impossible to have one channel without the
> > "handbrake" and one with normal flow control)
> 
> As discussed, this is on purpose. If you want multiplexing, implement
> multiplexing, including flow control.
> 
> If even one channel does not have flow control, this affects the transport
> layer also. It affects keep-alive mechanisms, key exchange, and flow control
> at the transport level.

Right, "no-handbrake" breaks all of these. I'm not sure what the gain is
either compared to making the SSH window match the TCP window size. It's
not like SSH can go send more data than that.

-d

---

From: [email-masked] (Niels Möller)
Date: Thu, 03 Dec 2015 09:31:34 +0100

>> A whole round-trip delay? For no benefit - in any known usage

I think the entire point of the "awful" design, where you don't get a
second chance, just a disconnect in case the service isn't available, is
to eliminate that roundtrip delay.

You can send a service request for "ssh-userauth", followed back-to-back
by userauth requests for "none", PK_OK, whatever querying you like. And
one roundtrip later, you can compute the needed signature for a
likely successful publickey userauth request.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.

---

From: [email-masked] (Niels Möller)
Date: Thu, 03 Dec 2015 11:58:54 +0100

Regarding sending SERVICE_REQUEST and userauth messages back-to-back,

denis bider <[email-masked]> writes:

> I'm not sure that we have a guarantee that the server must properly
> handle this chaining.

Have you seen any implementations broken is this way?

I'd expect any reasonable server to process incoming messages properly,
and in case it for some reason isn't ready to process the next logical
message, simply delay reading and processing messages until it is ready.

After all, the protocol is very intentionally designed to minimize the
number of roundtrip delays.

> One would imagine people would not do this, but I've dealt with an
> implementation that discards KEXINIT if it's received in the same
> network frame as the SSH version string.

I think that's a different bug, and not very relevant. That's the point
where you switch from new-line-delimited text data to the binary data,
which needs some care to get right.

Regards,
/Niels

---

From: denis bider <[email-masked]>
Date: Thu, 3 Dec 2015 08:39:48 +0000

I'm not sure that we have a guarantee that the server must properly handle this chaining.

A well-written server should. But a server could also perform preprocessing of SSH packets before sending responses, and it could notice that it has received USERAUTH_REQUEST before it has sent SERVICE_ACCEPT. The server could then choke on this.

One would imagine people would not do this, but I've dealt with an implementation that discards KEXINIT if it's received in the same network frame as the SSH version string.

Niels Möller <[email-masked]> , 12/3/2015 8:32 AM:

 
 
Regards, 
/Niels

---

From: denis bider <[email-masked]>
Date: Thu, 3 Dec 2015 00:33:40 +0000

> there for putting it anywhere else?

In the event that there are two or more competing extension mechanisms, only one of them can be last.

Do you happen to know:

There are still folks out there using OpenSSH 3.x and 4.x. I'm pretty sure I saw our software being connected to by a client like that in the past week.

> "handbrake" and one with normal flow control)

As discussed, this is on purpose. If you want multiplexing, implement multiplexing, including flow control.

If even one channel does not have flow control, this affects the transport layer also. It affects keep-alive mechanisms, key exchange, and flow control at the transport level.

> Given this, IMO this would be better off being replaced by a
> generic "these are the channel types I'm prepared to open"
> message, which would be useful even for implementation
> that don't want to turn off flow control but which do
> implement non-standard channel types (e.g. [email-masked]).

Sounds like an idea for another extension:

Name: supported-channels
Value: name-list of accepted channel types

? :-)

denis

----- Original Message -----
From: Damien Miller 
Sent: Wednesday, December 2, 2015 05:35
To: [email-masked] 
Subject: Feedback on draft-ssh-ext-info-00

Hi,

Here's some feedback on draft-ssh-ext-info-00.

> 3.1.  Signaling of Extension Negotiation in KEXINIT
...
> The indicator name is added without quotes, and MAY be added at any
> position in the name-list, subject to proper separation from other
> names as per name-list conventions. The suggested position is last in
> the list, but implementations MUST recognize these indicator names
> when included at any position.

IMO it would be best to make it a MUST that the flags may only be
sent as the last element. What rationale is there for putting it
anywhere else?

> 3.2.  Mechanism Enabling Criteria

This whole section seems over-complicated. I think a simpler scheme
is:

If a client or server offers "ext-info-c" or "ext-info-s" respectively,
then it must be prepared to accept a SSH_MSG_EXT_INFO message from the
peer. 

If this message is sent, then it will be the first message after the
initial SSH_MSG_NEWKEYS.

I.e. make it a simple advertisment that a client or server is willing
to accept a notification message that might optionally be sent.
Don't require both sides to offer acceptance, don't have the messages
offer any sort of negotiation between the client and server (just
statements of extensions/capabilities) and don't "replace" any
messages like SSH_MSG_SERVICE_REQUEST.

This would yield a much simpler specification IMO.

> 4. Initially Defined Extensions
> 4.2.  "client-req-ok"

I don't see the point of this: implementations that don't respond with
SSH_MSG_REQUEST_FAILURE to unknown SSH_MSG_GLOBAL_REQUEST messages are
incompliant with RFC4254 section 4.

OpenSSH has sent various @openssh.com global requests for years,
so most of the implementations that violate this should have long since
been shaken out.

> 4.3.  "no-handbrake"

This could be implemented more elegantly using new channel type names
in SSH_MSG_CHANNEL_OPEN messages rather than the complicated and
inflexible method proposed here (e.g. it's impossible to have one
channel without the "handbrake" and one with normal flow control).

It would be better to use the extension to signal that the
peer is willing to accept SSH_MSG_CHANNEL_OPEN messages with new channel
type names, and use these to selectively disable flow control.

Given this, IMO this would be better off being replaced by a generic
"these are the channel types I'm prepared to open" message, which would
be useful even for implementation that don't want to turn off flow control
but which do implement non-standard channel types (e.g. [email-masked]).

-d

---

From: Markus Friedl <[email-masked]>
Date: Thu, 3 Dec 2015 10:10:25 +0100

> Am 03.12.2015 um 01:33 schrieb denis bider <[email-masked]>:
> 

I don't think that the extension should redefine or replace existing prorotocl
messages. It should just be an additional message indicating the extensions.

-m

---

From: denis bider <[email-masked]>
Date: Thu, 3 Dec 2015 01:51:01 +0000

I have posted an updated EXT_INFO draft here:

https://tools.ietf.org/html/draft-ssh-ext-info-02

Most significant changes:
- Included an "accept-channels" extension, as per Damien's suggestion.
- Removed the "client-req-ok" extension.

Responses to comments:

(1) "The scheme is complex."

The scheme is in fact simple:

(a) Include indicator names in KEXINIT name-list.
(b) If both client and server indicate support, replace SERVICE_REQUEST + ACCEPT by sending EXT_INFO on both sides.

This does not seem complex to me.

The reason it may seem complex is because the spec tries to identify and specify as many corner cases as possible. This corner case coverage makes the mechanism seem complex. But this coverage is a feature.

A different mechanism may seem "simpler" without actually being simpler - if you realize that the "simplicity" comes from ignoring corner situations, and leaving them unspecified.

(2) "client-req-ok is unnecessary"

I have identified OpenSSH 3.1, released March 2002, as the first version that does not disconnect if the server sends a global request.

The oldest OpenSSH version I can confirm seeing recently is version 3.9. I also remember sporadically seeing old versions of SunSSH, which might be based either on OpenSSH 3.5, or OpenSSH 2.3. So there might still be software out there that disconnects if the server sends a global request, but it should be very old.

You are right, I have removed this extension.

(3) "Don't replace SERVICE_REQUEST + ACCEPT"

I still believe SERVICE_REQUEST + ACCEPT serve no purpose, and waste a round-trip for no reason. This seems like a great opportunity to do away with them.

(4) "no-handbrake should be per-channel"

As stated previously, if even one channel does not have flow control, this affects the  transport layer, too. It affects keep-alive mechanisms, key exchange,  and flow control at the transport level.

I have in fact strengthened the language, to exclude this extension from being used for sessions that require proper multiplexing.

If you want channel multiplexing, do it right.

denis

Do you happen to know:

Name: supported-channels

? :-)

denis

----- Original Message -----
From: Damien Miller 
Sent: Wednesday, December 2, 2015 05:35
To: [email-masked] 
Subject: Feedback on draft-ssh-ext-info-00

Hi,

...

-d

---

From: denis bider <[email-masked]>
Date: Thu, 3 Dec 2015 11:14:24 +0000

Point taken. You are probably right.

Niels Möller <[email-masked]> , 12/3/2015 10:59 AM:
Regarding sending SERVICE_REQUEST and userauth messages back-to-back, 

 

 

Regards, 
/Niels

---

From: [email-masked] (Niels Möller)
Date: Tue, 15 Dec 2015 09:32:10 +0100

denis bider <[email-masked]> writes:

> I still believe SERVICE_REQUEST + ACCEPT serve no purpose, and waste a
> round-trip for no reason. 

I still fail to see why SERVICE_REQUEST costs a roundtrip time. IIRC,
you claimed or guessed that some implementations can't handle NEWKEYS +
SERVICE_REQUEST + UERAUTH_REQUEST sent back-to-back. But I don't think
I've seen any details. And I really don't think replacing
SERVICE_REQUEST is an appropriate or proportionate workaround for such
bugs.

> This seems like a great opportunity to do away with them.

It's fine to have a proposed change to replace SERVICE_REQUEST depend on
the proposed extension mechanism. But please don't introduce a reverse
dependency. 

I don't think there's likely to be consensus that we should do anything
about SERVICE_REQUEST now, so it's going to be an obstacle to the
extension mechanism which is a lot easier to agree on.

Regards,
/Niels