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

Segfault when re-using handle with FTP + HTTPS CONNECT proxy + CURLOPT_HTTPPROXYTUNNEL #5340

Closed
FBNeal opened this issue May 5, 2020 · 3 comments
Closed

Segfault when re-using handle with FTP + HTTPS CONNECT proxy + CURLOPT_HTTPPROXYTUNNEL #5340

FBNeal opened this issue May 5, 2020 · 3 comments
Labels
connecting & proxies crash FTP

Comments

@FBNeal
Copy link

FBNeal commented May 5, 2020

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, "ftp://mirror.oss.ou.edu/centos/RPM-GPG-KEY-CentOS-7.noexist");

    /* Perform the request (twice), res will get the return code */
    res = curl_easy_perform(curl);
    /* Check for errors */
    if(res != CURLE_OK) {
        fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(res));
    }

    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;
}

(gdb) bt
#0  0x00007ffff7569734 in SSL_write () from /lib64/libssl.so.10
#1  0x00007ffff7bbb770 in ossl_send () from /data/users/neal/curl/lib/libcurl.so.4
#2  0x00007ffff7ba39f4 in Curl_write () from /data/users/neal/curl/lib/libcurl.so.4
#3  0x00007ffff7b8c71b in Curl_add_buffer_send () from /data/users/neal/curl/lib/libcurl.so.4
#4  0x00007ffff7b915a7 in Curl_proxyCONNECT () from /data/users/neal/curl/lib/libcurl.so.4
#5  0x00007ffff7b92124 in Curl_proxy_connect () from /data/users/neal/curl/lib/libcurl.so.4
#6  0x00007ffff7b80cad in ftp_do_more () from /data/users/neal/curl/lib/libcurl.so.4
#7  0x00007ffff7b9b9d0 in multi_runsingle () from /data/users/neal/curl/lib/libcurl.so.4
#8  0x00007ffff7b9c603 in curl_multi_perform () from /data/users/neal/curl/lib/libcurl.so.4
#9  0x00007ffff7b7b593 in curl_easy_perform () from /data/users/neal/curl/lib/libcurl.so.4
#10 0x0000000000400b12 in main ()

You'll have to replace proxy.example.com with a proxy supporting HTTPS CONNECT: I looked to try and find an easy example but wasn't able to do so, unfortunately. That being said I'd be happy to test out any proposed fixes.

I expected the following

The code runs without segfaulting.

curl/libcurl version

curl 7.70.0 (x86_64-pc-linux-gnu) libcurl/7.70.0 OpenSSL/1.0.2k-fips zlib/1.2.7
Release-Date: 2020-04-29
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL UnixSockets

operating system

CentOS 7

I spent quite a bit of time today trying to figure out where the issue is here, but my lack of experience in the Curl codebase made it a bit challenging. In case it's useful, here's what I found:

  • When we make the initial Curl request, we send the request via the HTTPS CONNECT proxy. We then use the secondary socket (which also has to go through the proxy) to fetch the file.
  • At the end of the initial curl_easy_perform the secondary socket is closed:

curl/lib/ftp.c

Lines 3233 to 3241 in 4a4b63d

if(conn->ssl[SECONDARYSOCKET].use) {
/* The secondary socket is using SSL so we must close down that part
first before we close the socket for real */
Curl_ssl_close(conn, SECONDARYSOCKET);
/* Note that we keep "use" set to TRUE since that (next) connection is
still requested to use SSL */
}
close_secondarysocket(conn);

curl/lib/ftp.c

Lines 217 to 224 in 4a4b63d

static void close_secondarysocket(struct connectdata *conn)
{
if(CURL_SOCKET_BAD != conn->sock[SECONDARYSOCKET]) {
Curl_closesocket(conn, conn->sock[SECONDARYSOCKET]);
conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD;
}
conn->bits.tcpconnect[SECONDARYSOCKET] = FALSE;
}

  • When the second curl_easy_perform is called, connection re-use in lib/url.c pulls back the connection from the first request (the segfault can be avoided by using CURLOPT_FRESH_CONNECT) including the primary and secondary sockets.
  • The segfault appears to occur because we are trying to connect out on the secondary socket but the underlying SSL socket was closed out, so when we try to write data it fails.
  • I was able to "fix" the issue by changing the Curl_ssl_close call in lib/ftp.c to Curl_ssl_shutdown (to fully close the socket on the Curl side) plus setting conn->bits. proxy_ssl_connected[SECONDARYSOCKET] to FALSE in close_secondarysocket (so that when we are inlib/http_proxy.c we actually try to reconnect). But I don't see anywhere else in the codebase managing connections like that, so that feels like a kludge rather than a real fix. I wasn't able to understand the socket lifecycle well enough to come up with a better solution here.
@bagder
Copy link
Member

bagder commented May 13, 2020 •
edited

To let me understand better what you did to fix (or work around) this issue. Can you paste a diff here or something? It will help me understand things.

Never mind. I read it again and I follow it and I think your solution looks correct. I'll file a PR and see ...

@bagder
Copy link
Member

bagder commented May 13, 2020

BTW, a "semi easy" way to get a HTTPS proxy up is to just front your ordinary HTTP proxy with stunnel. I use this simple script for it.

bagder added a commit that referenced this issue May 13, 2020
@bagder
ftp: shut dowen the secondary connection better when SSL is used
9952a27 
Reported-by: Neal Poole
Fixes #5340
@bagder bagder mentioned this issue May 13, 2020
Closed
@FBNeal
Copy link
Author

FBNeal commented May 13, 2020

Never mind. I read it again and I follow it and I think your solution looks correct. I'll file a PR and see ...

Just read your PR and those were my exact changes. :-)

bagder added a commit that referenced this issue May 14, 2020
@bagder
ftp: shut dowen the secondary connection better when SSL is used
537e7ee 
Reported-by: Neal Poole
Fixes #5340
bagder added a commit that referenced this issue May 14, 2020
@bagder
ftp: shut down the secondary connection properly when SSL is used
eaeb6cb 
Reported-by: Neal Poole
Fixes #5340
Closes #5385
@bagder bagder closed this as completed in f002c85 May 15, 2020
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 FTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ftp: shut down the secondary connection better when SSL is used curl/curl
2 participants
@bagder @FBNeal