Subject: Re: [PATCH] Follow RFC4253 section 11.4

Re: [PATCH] Follow RFC4253 section 11.4

From: Peter Stuge <peter_at_stuge.se>
Date: Thu, 1 Mar 2012 23:37:15 +0100

Henrik Nordström wrote:
> > This might already be managed within libssh2 though, that
> > keepalives are never sent in the middle of another request.
>
> In blocking mode keepalives are by default sent in the middle of other
> requests by _libssh2_wait_socket.
>
> And applications is very likely to do use libss2_keepalive_send in
> similar manner in non-blocking mode.

I see. Then the code that receives packets must be changed to take
care of those responses as well.

> Also be warned that libssh2_keepalive_send corrupts transport if
> only part of the keep-alive message can be sent.

Good fun.

> 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.

> 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.

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

> 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.

> 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.

> > I think want reply = 1 for keepalive would be fine, and isolates
> > the problem to that part of the code.
>
> Not really without also making keep-alives a round-trip blocking
> thing preventing any concurrent unanswered packets of any kind
> during the keep-alive probe,

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().

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