While testing a a SFTP client of mine using libcurl + libssh2.18, I
discovered a bug when maintaining a persistent connection with the VanDyke
VShell Server (Windows based SFTP server).
My code would open a SFTP session to the VShell server, and poll a directory
every 60 seconds. My client would then download files of any interest. The
entire time the libcurl easy handle would keep the SFTP session open the
server. The VShell server would periodically (by default every 60 minutes
and at most every 10 minutes) send a KEXINIT message while my code was
"sleeping" between tasks.
libssh2 would only be able to detect this received KEXINIT message while it
executed another action for libcurl (for example libssh2_sftp_open_handle).
While waiting for a reply for the original action, the libssh2 code would
begin the key re-exchange. However, as everything was being executed in
non-blocking mode, libssh2 would return from the key re-exchange context to
the original SFTP context before the needed KEX_DH_GEX_GROUP message came
from the VShell. The SFTP context code would continue to wait for a SFTP
message reply, which would never come until the key re-exchange was
finished.
In some cases libcurl would time out, disconnect, reconnect and everything
would be okay until the next key re-exchange. At the worst, I sometimes
observed at a customer site that execution would be stuck indefinitely
waiting for a file that wouldn't come.
Please find attached a set of patches I've made that has at the very least
mitigated the situation. The ultimate bug seemed to be that in non-blocking
mode, libssh2 would start the key re-exchange, return EAGAIN when no data
was available, and then lose track that the key re-exchange was occuring.
In transport.c, I changed libssh2_packet_read so that if it detected a key
re-exchange was generally active, but not explictly active at that time, it
would redirect into libssh2_kex_exchange. Later on, when fullpacket returns
with PACKET_EAGAIN, I check the packAdd_state to see if adding the packet
fully succeded. This was necessary so I could reset the packet read state,
which was needed for the key re-exchange conversation to be read.
For the file kex.c, I've changed libssh2_kex_exchange to set the
LIBSSH2_KEX_ACTIVE bit upon function entrance, and to unset it at function
exit. I needed this to avoid a endless libssh2_packet_read ->
libssh2_kex_exchange -> ... -> libssh2_packet_read -> libssh2_kex_exchange
loop.
In libssh2_priv.h, I've added the LIBSSH2_KEX_ACTIVE flag.
In packet.c, I had to reset the readPack, fullpacket, and packAdd states so
the key re-exchange conversation could proceed. Adding a packet would also
clear out the packAdd_key_state object, which would create a core dump
during the key re-exchange. For all calls to kex_exchange, I swapped
packAdd_key_state with startup_key_state and everything seemed to be fine.
This fixed the key re-exchange problems for me. I'm now able to maintain a
persistent connection with the VShell without disconnects or endless loops.
Regards,
Sean
Index: kex.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/kex.c,v
retrieving revision 1.36
diff -r1.36 kex.c
1682a1683,1684
> session->state |= LIBSSH2_STATE_KEX_ACTIVE;
>
1713a1716
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
1718a1722,1723
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
> session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
1731a1737
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
1739a1746,1747
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
> session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
1765a1774
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
1784a1794
> session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
Index: packet.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/packet.c,v
retrieving revision 1.55
diff -r1.55 packet.c
916a917,936
>
> /*
> * The KEXINIT message has been added to the queue.
> * The packAdd and readPack states need to be reset
> * because libssh2_kex_exchange (eventually) calls upon
> * libssh2_packet_read to read the rest of the key exchange
> * conversation.
> */
> session->readPack_state = libssh2_NB_state_idle;
> session->packet.total_num = 0;
> session->packAdd_state = libssh2_NB_state_idle;
> session->fullpacket_state = libssh2_NB_state_idle;
>
> /*
> * Also, don't use packAdd_key_state for key re-exchange,
> * as it will be wiped out in the middle of the exchange.
> * How about re-using the startup_key_state?
> */
> memset(&session->startup_key_state, 0, sizeof(key_exchange_state_t));
>
921c941
< rc = libssh2_kex_exchange(session, 1, &session->packAdd_key_state);
---
> rc = libssh2_kex_exchange(session, 1, &session->startup_key_state);
Index: transport.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/transport.c,v
retrieving revision 1.13
diff -r1.13 transport.c
271a272,305
> int status;
>
> /*
> * All channels, systems, subsystems, etc eventually make it down here
> * when looking for more incoming data. If a key exchange is going on
> * (LIBSSH2_STATE_EXCHANGING_KEYS bit is set) then the remote end
> * will ONLY send key exchange related traffic. In non-blocking mode,
> * there is a chance to break out of the kex_exchange function with an
> * EAGAIN status, and never come back to it. If LIBSSH2_STATE_EXCHANGING_KEYS
> * is active, then we must redirect to the key exchange. However,
> * if kex_exchange is active (as in it is the one that calls this execution
> * of packet_read, then don't redirect, as that would be an infinite loop!
> */
>
> if (session->state & LIBSSH2_STATE_EXCHANGING_KEYS &&
> !(session->state & LIBSSH2_KEX_ACTIVE)) {
>
> /* Whoever wants a packet won't get anything until the key re-exchange
> * is done!
> */
> _libssh2_debug(session, LIBSSH2_DBG_TRANS, "Redirecting into the"
> " key re-exchange");
> status = libssh2_kex_exchange(session, 1, &session->startup_key_state);
> if (status == PACKET_EAGAIN) {
> libssh2_error(session, LIBHSSH2_ERROR_EAGAIN,
> "Would block exchanging encryption keys", 0);
> return PACKET_EAGAIN;
> } else if (state) {
> libssh2_error(session, LIBSSH2_ERROR_KEX_FAILURE,
> "Unable to exchange encryption keys",0);
> return LIBSSH2_ERROR_KEX_FAILURE;
> }
> }
>
530,531c564,578
< session->readPack_encrypted = encrypted;
< session->readPack_state = libssh2_NB_state_jump1;
---
>
> if (session->packAdd_state == libssh2_NB_state_idle)
> {
> /* fullpacket only returns PACKET_EAGAIN if
> * libssh2_packet_add returns PACKET_EAGAIN. If that
> * returns PACKET_EAGAIN but the packAdd_state is idle,
> * then the packet has been added to the brigade, but some
> * immediate action that was taken based on the packet
> * type (such as key re-exchange) is not yet complete.
> * Clear the way for a new packet to be read in.
> */
> session->readPack_encrypted = encrypted;
> session->readPack_state = libssh2_NB_state_jump1;
> }
>
Index: libssh2_priv.h
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/libssh2_priv.h,v
retrieving revision 1.35
diff -r1.35 libssh2_priv.h
835a836
> #define LIBSSH2_STATE_KEX_ACTIVE 0x00000008
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
libssh2-devel mailing list
libssh2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
