Re: global and channel requests -- more information on failure, more flexibility on success

[nisse%lysator.liu.se@localhost (Niels Möller)]

"Joseph Galbraith" <galb-list%vandyke.com@localhost> writes:

> Actually, old client to new server would be the worst,
> because the client sends a whole bunch of requests
> (pty, shell, subsystem, agent, etc.) and if any of
> them failed, the client would notice too much data
> in the response and report a protocol error.

This is the most problematic case, I guess. If for instance agent
forwarding is denied, the user would have to use some
--no-agent-forward option. But to be precise exactl which are the
requests that are sent by default and which will reasonably fail? pty
and agent forwarding are in that class (and I think they are used mostly
in interactive mode, not when scripting ssh).

shell and subsystem isn't in that class IMO: The typical use is to
send only one of them, and if it fails, there's not much point going
on anyway. I don't think it is a big problem if the user can sometimes
get a message  "protocol error..." rather than "the request you really
needed to send failed".

> Imagine an SFTP client -- you can't really disable
> subsystem requests, which might generate failures.

I don't think this case is no real problem. If the client wants to
start sftp, and the server doesn't let it, the client will fail in one
way or the other. It doesn't matter too much in which way.

> For example, something along
> the lines of "unless otherwise specified, a request
> uses the basic version of the response messages.  New
> requests SHOULD use the extended version of the failure
> message which is defines as follows ... "

The below is your proposal (I hope I found the latest one):

:	 byte      SSH_MSG_CHANNEL_FAILURE
:	 uint32    recipient_channel
:	 ...request specific data
:
:	These messages do not consume window space and can be sent even if
:	no window space is available.
:
:	Unless otherwise specified, a request uses the general form
:	of these messages, with no request specific failure or success
:	data.
:
:	If the recipient doesn't recognize the request, it
:	will send a SSH_MSG_CHANNEL_FAILURE without any
:	request specific data.  This is called the generic
:	form.
:
:	Requests that do send information during failure MUST
:	use the following format:
:
:	 byte      SSH_MSG_CHANNEL_FAILURE
:	 uint32    recipient_channel
:	 uint32    reason
:	 string    descriptive-text [UTF8]
:	 string    language
:	 ... further request-specific data follows
:
:	New requests SHOULD use this extended failure message
:	in order to provide end users and server administrators
:	with information about a failure.

I think I'd prefer something like

	 byte      SSH_MSG_CHANNEL_FAILURE
	 uint32    recipient_channel
	 uint32    reason
	 string    descriptive-text [UTF8]
	 string    language
	 ... request-specific data follows

	These messages do not consume window space and can be sent even if
	no window space is available.

	For backwards compatibility, a shorter form is sometimes used,

	 byte      SSH_MSG_CHANNEL_FAILURE

        If a certain request REQUIRES the shorter form is described
        together with that request.

That is mostly a wording change. I'd be happier to say that the
shorter form MAY be used for backwards compatibility, but that the
longer canonical form is always allowed.

[ about the new tcpip forwarding request types ]

> I actually I think the new messages are for the best,
> since the old drafts didn't actually disallow binding
> to port 0 (even if it wasn't specifically allowed either.)

It will add a significant amount of complexity to clients to say that
they should first try the new request, then fall back to the old ones
(on the server side, it's easier to support one or both at the same
time). I wouldn't do that in my client; I'd probably just use the good
old tcpip-forwarding request, except when the user wants to bind port
zero, in which case I'd use the new one.

To me, that indicates that the idea of having two parallell request
types with overlapping functionality is bad design. If there are two
ways to do the same thing, it is likely that one of the ways will be
the one used in practice, while the other one must still be there for
conformance. The implementation of the "unusual way" of doing things
will attract bugs, which may even break security. It would be better
to either

  Extend the existing tcpip-forward, saying that binding port 0 is
  allowed, and that it will result in a special success message. This
  is the solution that I would prefer.

  Or invent a new request type for the _sole_ purpose of binding port
  0.

I think both these alternatives are backwards compatible (I don't
think we have any obligation to support existing clients that use
tcpip-forward to bind port 0 and expects any particular behaviour).

I also feel that those changes can be made easily at any time, so I
don't think we need to do anything with the core specs in this regard
except perhaps to point out that they don't define the bahaviour of
tcpip-forward when binding port 0.

> SecureCRT probably doesn't check to make the port isn't
> zero -- and there might be other clients out there that
> would allow it to happen if the user requested it.
> 
> Also, do old servers actually refuse, or does it succeed --
> I'd bet succeed -- but I don't have easy access right now
> to test -- but it seems like I tested this once.

I don't think any of that is behaviour that any server, client or user
should use or depend on. Do you know of any real world usage?

Regards,
/Niels