Subject: Re: [PATCH] Follow RFC4253 section 11.4

Re: [PATCH] Follow RFC4253 section 11.4

From: Henrik Nordström <henrik_at_henriknordstrom.net>
Date: Fri, 02 Mar 2012 13:40:52 +0100

tor 2012-03-01 klockan 23:37 +0100 skrev Peter Stuge:

> > You only need to track the sequence number of the pending packets
> > you currently expect a response to.
>
> And all previous packets which have not yet been responded to. The
> problem is that sending anything with want reply = 0 means that no
> response is expected, yet there is one.

I argue that it's perfectly fine to ignore the error response if you do
not expect a response.

Please put want_reply aside for now. That's unique to GLOBAL_REQUEST
packets, not general flag for SSH packets. The only thing it adds to
this discussion is to show that the same packet type may or may not
expect a response depending on it's content.

> > Getting UNIMPLEMENTED as response to a packet you are not expecting
> > a response to can safely be ignored I think.
>
> It depends on what the packet was, but all sent packets which could
> possibly return UNIMPL would have to be recorded, if UNIMPL sent by
> the server out of the blue can be distinguished from actual
> responses.

UNIMPLEMENTED responses are identified by the sequence number of the
packet they respond to.

> If that does not matter, then simply ignoring UNIMPL always works
> too.

Not when you expect a response. For example the case just discussed with
a want_reply keep-alive request sent before connection service is active
would hang if implemented right and UNIMPLEMENTED is ignored.

> > It's not really about global requests as such. You may in theory
> > see UNIMPLEMENTED as response to any packet.
>
> Sure, but not all packets have a want reply field. If want reply = 0
> it will be not very nice for libssh2 to have to keep track of the
> packet anyway because maybe there will be a response.

Yes. Have never said anything along those lines. Only the opposite,
there is no need to track packets to which you do not expect a response.

> > In this specific case it's seen because global channel requests
> > (keep-alive) are sent before the connection protocol is activated.
>
> The keepalive is a global request, unrelated to any channel, it only
> lives inside the session. "Note that both the client and server MAY
> send global requests at any time" so it's fine to send it also before
> userauth is completed.

No it's not. The connection protocol needs to be activated first
(SSH_MSG_SERVICE_REQUEST "ssh-connection"). You can not use connection
protocol messages without the connection protocol active.

> I was hoping that they already were sent without other outstanding
> packets, but since they are not, the only other easy fix I can think
> of is to always ignore UNIMPLEMENTED in _libssh2_packet_requirev().

Yes and is why I proposed that for now we should ignore and warn.

Long term we probably need to deal with them, which requires keeping
track of the sequence number of packets one expects a response to so the
error can be correctly routed up the stack to the application. This
requires a bit of changes in transport api to indicate if responses are
expected or not when sending a packet to transport.

Regards
Henrik

_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-03-02