Re: SSH Global Request

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Mon, 2020-03-09 at 12:23 +0100, Michal Vaško wrote:
> Hi Jakub,
> I believe the comment is quite clear, the intention was to return
> SSH_OK even when the request is denied because it should be denied.
> It works just fine for blocking sockets and the resulting code is
> nicer. I think the author simply forgot about non-blocking sockets
> and that ssh_global_request() can return SSH_AGAIN, that is all.

In that case you are already more expert than me and patches improving
this are welcomed. Bonus points for accompanying test case to make sure
it works as expected in both blocking and non-blocking case :)

Jakub

> Regards,
> Michal
> 
> On Monday, March 9, 2020 09:40 CET, Jakub Jelen <jjelen@xxxxxxxxxx>
> wrote: 
>  
> > On Mon, 2020-03-09 at 08:02 +0100, Michal Vaško wrote:
> > > Hi Jakub,
> > > thanks for the response. I was aware of the bug report but there
> > > was
> > > some information that I have missed. Hopefully understanding it
> > > better now I have one last request. I believe
> > > ssh_global_request()
> > > with reply set to 1 should block until a response is received,
> > > which
> > > is fine. But in case a non-blocking socket is used, it returns
> > > immediately. Could the function ssh_send_keepalive() simply
> > > return
> > > the value of ssh_global_request() instead of ignoring it? That
> > > way it
> > > could be checked for SSH_AGAIN. I am fine with returning SSH_OK
> > > irrespective of whether the client accepts or denies the request.
> > 
> > Hello,
> > it used to be this way, before the following commit:
> > 
> > https://gitlab.com/libssh/libssh-mirror/commit/59ada799
> > 
> > I am not completely clear about the reasoning for "dropping" the
> > return
> > value so better ask the author directly (in CC).
> > 
> > Regards,
> > Jakub
> > 
> > > Regards,
> > > Michal
> > > 
> > > On Friday, March 6, 2020 16:54 CET, Jakub Jelen <
> > > jjelen@xxxxxxxxxx>
> > > wrote: 
> > >  
> > > > On Fri, 2020-03-06 at 12:01 +0100, Michal Vaško wrote:
> > > > > Hi Jakub,
> > > > > right, I have not noticed the function before, thanks.
> > > > > However, I
> > > > > am
> > > > > not sure the API is suitable. What is the general behavior,
> > > > > meaning
> > > > > what happens if a reply is not sent back?
> > > > 
> > > > Hi,
> > > > see the discussion in, where we tried to understand this
> > > > functionality
> > > > in libssh (I did not implement that so I am not much better
> > > > suited
> > > > to
> > > > hint here):
> > > > 
> > > > https://bugs.libssh.org/T212
> > > > 
> > > > > Is the connection automatically disconnected?
> > > > 
> > > > The above function handles only the sending of the request. The
> > > > answer
> > > > is IGNORE message, which is handled with other packets. AFAIK
> > > > if
> > > > the
> > > > TCP socket gets disconnected, your application can check using
> > > > ssh_is_connected().
> > > > 
> > > > > Is it possible to use custom max probes (how many keep-alive
> > > > > packets
> > > > > are sent unsuccessfully before a connection is considered
> > > > > dead)
> > > > > and
> > > > > probe interval (the interval between keep-alive packets) or
> > > > > some
> > > > > specific values are always used?
> > > > 
> > > > This is up to the application. The above API does just what it
> > > > says
> > > > in
> > > > the name -- sends the keepalive packet to avoid disconnecting
> > > > on
> > > > TCP
> > > > level. If you want to count how many times it was send and
> > > > when.
> > > > There
> > > > is no simple way for libssh to handle this magically in
> > > > background.
> > > > 
> > > > > There is no documentation, that is why I am asking. Thanks
> > > > > for
> > > > > providing these details.
> > > > 
> > > > The server API is generally poorly documented so if you will
> > > > learn
> > > > more
> > > > how does it work in real environment, contributions are
> > > > welcomed.
> > > > 
> > > > Regards,
> > > > Jakub
> > > > 
> > > > > Regards,
> > > > > Michal
> > > > > 
> > > > > On Friday, March 6, 2020 11:29 CET, Jakub Jelen <
> > > > > jjelen@xxxxxxxxxx>
> > > > > wrote: 
> > > > >  
> > > > > > On Fri, 2020-03-06 at 09:11 +0100, Michal Vaško wrote:
> > > > > > > Hello,
> > > > > > > I was wondering if the authors of libssh would have
> > > > > > > anything
> > > > > > > against
> > > > > > > making ssh_global_request() function from channels.h
> > > > > > > public.
> > > > > > > It
> > > > > > > would
> > > > > > > enable implementing NETCONF Call Home SSH keep-alive [1].
> > > > > > > Thanks.
> > > > > > 
> > > > > > Hello,
> > > > > > in server side, there is already API for sending keepalive
> > > > > > messages:
> > > > > > https://gitlab.com/libssh/libssh-mirror/-/blob/master/include/libssh/server.h#L368
> > > > > > 
> > > > > > In the connection, only one of the peers needs to send the
> > > > > > keepalive
> > > > > > messages to keep a connection alive.
> > > > > > 
> > > > > > If you need the similar functionality in the client, I
> > > > > > would
> > > > > > rather
> > > > > > see
> > > > > > it implemented by creating a new API function than exposing
> > > > > > this
> > > > > > internal function.
> > > > > > 
> > > > > > Regards,
> > > > > > -- 
> > > > > > Jakub Jelen
> > > > > > Senior Software Engineer
> > > > > > Security Technologies
> > > > > > Red Hat, Inc.
> > > > > > 
> > > > > > 
> > > > >  
> > > > >  
> > > > > 
> > > > > 
> > > > -- 
> > > > Jakub Jelen
> > > > Senior Software Engineer
> > > > Security Technologies
> > > > Red Hat, Inc.
> > > > 
> > > > 
> > >  
> > >  
> > > 
> > > 
> > -- 
> > Jakub Jelen
> > Senior Software Engineer
> > Security Technologies
> > Red Hat, Inc.
> > 
> > 
>  
>  
> 
> 
-- 
Jakub Jelen
Senior Software Engineer
Security Technologies
Red Hat, Inc.