Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Curl 7.76.0 segfault during connection close (SFTP + HTTPPROXYTUNNEL + HTTPS proxy) #6898

Closed
mofarrell opened this issue Apr 15, 2021 · 6 comments
Closed

Curl 7.76.0 segfault during connection close (SFTP + HTTPPROXYTUNNEL + HTTPS proxy) #6898

mofarrell opened this issue Apr 15, 2021 · 6 comments
Labels
connecting & proxies crash regression SCP/SFTP

Comments

@mofarrell
Copy link

mofarrell commented Apr 15, 2021 •
edited

I did this

#include <stdio.h>
#include <curl/curl.h>

int main() {

  CURL *curl;
  CURLcode res;

  curl_global_init(CURL_GLOBAL_DEFAULT);

  curl = curl_easy_init();
  if(curl) {

    curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
    curl_easy_setopt(curl, CURLOPT_HTTPPROXYTUNNEL, 1);
    curl_easy_setopt(curl, CURLOPT_PROXY, "proxy.example.com");
    curl_easy_setopt(curl, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
    curl_easy_setopt(curl, CURLOPT_PROXY_SSLCERT, "/var/some/file");
    curl_easy_setopt(curl, CURLOPT_PROXY_SSLKEY, "/var/some/file");
    curl_easy_setopt(curl, CURLOPT_PROXY_CAINFO, "/var/some/other/file");
    curl_easy_setopt(curl, CURLOPT_URL, "sftp://somesftpthing.com");
    curl_easy_setopt(curl, CURLOPT_USERPWD, "somesuer:somepassword");

    res = curl_easy_perform(curl);
    /* Check for errors */
    if(res != CURLE_OK) {
        fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(res));
    }

    curl_easy_cleanup(curl);
  }
  curl_global_cleanup();
  return 0;
}

Curl segfaults during shutdown with a null connection:
https://github.com/curl/curl/blob/master/lib/vssh/libssh2.c#L3030

Stacktrace:

#0 0x7ff626e2b657 in ssh_tls_send /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:3037:12
#1 0x7ff626c26491 in _libssh2_transport_send /libssh2/1.9.0/src/libssh2-1.9.0/src/transport.c:888:11
#2 0x7ff626c08d28 in channel_send_eof /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2253:10
#3 0x7ff626c08d28 in _libssh2_channel_close /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2406:14
#4 0x7ff626c09190 in _libssh2_channel_free /libssh2/1.9.0/src/libssh2-1.9.0/src/channel.c:2583:14
#5 0x7ff626c1d6fd in sftp_shutdown /libssh2/1.9.0/src/libssh2-1.9.0/src/sftp.c:1079:10
#6 0x7ff626c1d6fd in libssh2_sftp_shutdown /libssh2/1.9.0/src/libssh2-1.9.0/src/sftp.c:1093:5
#7 0x7ff626e2c18c in ssh_statemach_act /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2488:14
#8 0x7ff626e2fd94 in ssh_block_statemach.constprop.0 /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2944:14
#9 0x7ff626e176b3 in Curl_disconnect /curl/7.76.0/src/curl-7.76.0/lib/url.c:851:5
#10 0x7ff626dd163f in Curl_conncache_close_all_connections /curl/7.76.0/src/curl-7.76.0/lib/conncache.c:553:11
#11 0x7ff626e01d28 in curl_multi_cleanup /curl/7.76.0/src/curl-7.76.0/lib/multi.c:2471:5
#12 0x7ff626e183e9 in Curl_close /curl/7.76.0/src/curl-7.76.0/lib/url.c:379:5
#13 0x7ff626ddc399 in curl_easy_cleanup /curl/7.76.0/src/curl-7.76.0/lib/easy.c:742:3

I expected the following

Curl to not crash during shutdown

curl/libcurl version

curl 7.76.0 (x86_64-facebook-linux-gnu) libcurl/7.76.0 OpenSSL/1.1.1k zlib/1.2.8 c-ares/1.13.0 libssh2/1.9.0 nghttp2/1.33.0
Release-Date: 2021-03-31
Protocols: file ftp ftps http https ldap ldaps mqtt rtsp scp sftp 
Features: alt-svc AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets

Known working in:

curl 7.72.0 (x86_64-facebook-linux-gnu) libcurl/7.72.0 OpenSSL/1.1.1k zlib/1.2.8 c-ares/1.13.0 libssh2/1.9.0 nghttp2/1.33.0
Release-Date: 2020-08-19
Protocols: file ftp ftps http https ldap ldaps rtsp scp sftp 
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets

operating system

CentOS 8
@Spaceface16518 Spaceface16518 mentioned this issue Apr 16, 2021
Closed
@bagder
Copy link
Member

bagder commented Apr 17, 2021

So how is the connection NULL? It gets a connection attached to the transfer in Curl_disconnect(), doesn't it? Where does it go wrong from there?

@mofarrell
Copy link
Author

I spent a little time looking at a core, and here is what I have found:

(gdb) frame 15
#15 0x0000000000dbcc1d in Curl_close (datap=datap@entry=0x7fb6f1a470e8) at /curl/7.76.0/src/curl-7.76.0/lib/url.c:379
(gdb) p data
$35 = (struct Curl_easy *) 0x7fbdba7b2000
(gdb) frame
#12 0x0000000000ca2644 in Curl_disconnect (data=0x7fbcf127f000, conn=conn@entry=0x7fbad627ab00, dead_connection=dead_connection@entry=false) at /curl/7.76.0/src/curl-7.76.0/lib/url.c:851
(gdb) p data->conn
$3 = (struct connectdata *) 0x7fbad627ab00
(gdb) frame
#10 0x0000000005eefd2e in ssh_statemach_act (data=data@entry=0x7fbcf127f000, block=block@entry=0x7fb6f1a46a9f) at /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:2488
(gdb) frame
#3  0x0000000005eef2a8 in ssh_tls_send (sock=38147, buffer=0x7fb8dc740288, length=48, flags=<optimized out>, abstract=<optimized out>) at /curl/7.76.0/src/curl-7.76.0/lib/vssh/libssh2.c:3030
(gdb) p data
$4 = (struct Curl_easy *) 0x7fbdba7b2000
(gdb) p data->conn
$6 = (struct connectdata *) 0x0
(gdb) frame
#9  libssh2_sftp_shutdown (sftp=0x7fbcefbf0a00) at sftp.c:1093
(gdb) p sftp->channel->session->abstract
$24 = (void *) 0x7fbdba7b2000

The outer transfer in Curl_disconnect appears to be for the sftp connection, while the inner data, the one that is failing, appears to be the HTTPS proxy connection (inferred from looking at the contents of the Curl_easy struct).
It seems that libssh is caching the struct Curl_easy * for the proxy transfer in their session struct. This is not the transfer that gets a connection reattached during disconnection. The transfer getting the connection reattached appears to be the sftp transfer? As you can probably tell, I don't know how these tunneling requests are implemented, and I am not very familiar with the curl internals.

FWIW I am guessing 215db086e09665ee7af9b646ad6c4d6e281001ac is the source of the regression . It introduced a detaching of a connection during Curl_close prior to running the cleanup. In this case, this clears the connection for the proxy transfer across the cleanup call.

@bagder
Copy link
Member

bagder commented Apr 20, 2021

It seems totally reasonable to suspect that commit for having caused this regression. Unfortunately, that was a refactor that can't be reversed easily so the fix for this issue cannot be as easy as to revert that commit. This use case is a bit complicated and we clearly don't test this setup properly - as then we would've caught this in the CI before we merged this change.

I don't know when I'll get time to look at this any closer.

bagder added a commit that referenced this issue May 17, 2021
@bagder
conn: add 'attach' to protocol handler, make libssh2 use it
9cb8360 
The libssh2 backend has SSH session associated with the connection but
the callback context is the easy handle, so when a connection gets
attached to a transfer, the protocol handler now allows for a custom
function to get used to set things up correctly.

Reported-by: Michael O'Farrell
Fixes #6898
@bagder bagder mentioned this issue May 17, 2021
Closed
@bagder bagder closed this as completed in 0c55fba May 17, 2021
@PhilETaylor
Copy link

PhilETaylor commented Jun 10, 2021 •
edited

Is this the related to the report here? https://gitlab.alpinelinux.org/alpine/aports/-/issues/12733

@bagder
Copy link
Member

bagder commented Jun 10, 2021

Hardly, this issue involved SFTP - and is already fixed.

@PhilETaylor
Copy link

No worries, proves that I was searching before posting a new issue though ha ha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
connecting & proxies crash regression SCP/SFTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

conn: add 'attach' to protocol handler, make libssh2 use it curl/curl
3 participants
@bagder @PhilETaylor @mofarrell