libssh2_sftp_close_handle and LIBSSH2_ERROR_EAGAIN

[PengZheng]

Describe the bug
When encountering a weird SFTP server, which refused to respond to our request, currently there is no way to cleanup local resources.

Expected behavior
If a user-provided timeout triggers and we have not finished client-server interaction, the user should have option to forcefully cleanup all local resources regardless of server behavior.

Version (please complete the following information):
This is a very old issue first reported 2008: https://marc.info/?l=libssh2-devel&m=121498802311288&w=4

Additional context
As mentioned curl/curl#8632 (comment), this issue has been raised multiple times.
libcurl did provide a workaround for sftp hang (curl/curl@fa34353), but did not address the resource leak issue.

NOTE

If the client keep trying to reconnect to these weird servers, this can be viewed as Server-Initiated DOS vulnerability, because finally the unfortunate client will leak all its available memory.

[PengZheng]

I noticed that BLOCK_ADJUST is used extensively to release various resources.
Considering playing complex finite state machine dancing without upstream blessing is dangerous. I propose an alternative Linux workaround without modifying libssh2 for others' benefits:

1. Set a reasonable libssh2 timeout, say 2 seconds, try release said resource in blocking way. If it succeeds, we're done. Otherwise:
2. Inject network IO error into the existing ssh connection without closing the associate socket, so that libssh2 is tricked into perform resource cleanup in case of network error. A possible way of doing this:
   a. set TCP_USER_TIMEOUT/SO_KEEPALIVE, so that errors other than EAGAIN triggered within libssh2 timeout, say 2 seconds
   b. shutdown(SHUT_WR), so that any writing attempt immediately trigger EPIPE. Noting that SHUT_RD does not work on Linux. Otherwise, we do not bother the other two steps.
   c. utilize iptable/other means to drop every packet for this connection.
3. Try release said resource in blocking way again. This time all local resource is guaranteed to be cleaned, assuming libssh2 can deal with networking error robustly.

@bagder

[willco007]

If you're running in non-blocking mode with a timeout set, the timeout isn't being hit?

[PengZheng]

If you're running in non-blocking mode with a timeout set, the timeout isn't being hit?

For non-blocking mode, sftp_close_handle will always return LIBSSH2_ERROR_EAGAIN if there is no io error and interaction with server can not be finished without blocking. _libssh2_wait_socket never has a chance to run:

do {
    time_t entry_time = time(((void *) 0));
    do {
        rc = sftp_close_handle(hnd);
        if ((rc != LIBSSH2_ERROR_EAGAIN) || !hnd->sftp->channel->session->api_block_mode)break;
        rc = _libssh2_wait_socket(hnd->sftp->channel->session, entry_time);
    }
    while (!rc);
}

[willco007]

Where inside of sftp_close_handle is it returning EAGAIN? It seems the likely cause would be sftp_packet_require which should probably allow a timeout but doesn't. However, it looks like sftp_packet_requirev does allow timing out and perhaps might be more appropriate to use.

[PengZheng]

In our environment, most of time it is sftp_packet_require that returns EAGAIN.
But I can not exclude _libssh2_channel_write as a possibility. It is hard to fix it without changing API semantics.
In an ugly hack, libssh2_sftp_force_close_handle is added, which cleans local resource without any server interaction. It is used as a last resort if libssh2_sftp_close_handle times out (in blocking mode).

After noting that BLOCK_ADJUST is used to do various resource cleanup, I realized such a hack is not enough