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

Proxy requests socket's closure. WinSSL writes to it anyway? #1239

Closed
Antony74 opened this Issue Feb 2, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@Antony74

Antony74 commented Feb 2, 2017

Version numbers

Curl must be compiled with WinSSL support. I'm using Curl 7.52.1 and Windows 8.1.

versions

My VC12 build configuration is: DLL Release - DLL Windows SSPI - DLL WinIDN x64.

Dummy proxy

We don't have access to the proxy server where this issue was originally observed.

Therefore to reproduce this problem we require a proxy server which always responds with a 407 (proxy authentication required) and asks for the connection to be closed.

I achieved this with the following NodeJS script:

//
// usage: node dummy-proxy.js
//

var port = 8080;
var msg = "HTTP/1.0 407 Proxy Authentication Required";
var headers = "Proxy-Authenticate: Negotiate\n"
            + "Connection: close";

var net = require('net');

var server = net.createServer(function(socket) {

  console.log('New connection');

  socket.on('data', function() {
    socket.write(msg + '\n' + headers + "\n\n" + msg);
    console.log(msg);
  });

  socket.on('error', function() {
    console.log('error');
  });

});

server.listen(port, function() {
  console.log("Dummy proxy running on port " + port);
});

Curl command line

Once the dummy proxy is running, we can enter the following curl command:

curl --proxy localhost:8080 --proxy-negotiate --proxy-user : https://google.com/

command

Bug

curl: (56) Send failure: Descriptor is not a socket

Expected

The correct error message for curl to give in this scenario is:

curl: (56) Received HTTP code 407 from proxy after CONNECT

@jay jay added the SSL/TLS label Feb 3, 2017

Antony74 added a commit to Antony74/curl that referenced this issue Feb 3, 2017

curl#1239 Proxy requests socket's closure. WinSSL writes to it anyway?
This bug can probably be fixed with a single if-statement.  With my very limited knowledge of the code I'd tentatively suggest this if-statement.  Works for me in the two simple test cases I've tried.
@Antony74

This comment has been minimized.

Show comment
Hide comment
@Antony74

Antony74 Feb 7, 2017

Can be of any assistance while this is still fresh in my memory? (I think my bug report and possible fix are adequate in this respect, but just wanted to double-check).

Antony74 commented Feb 7, 2017

Can be of any assistance while this is still fresh in my memory? (I think my bug report and possible fix are adequate in this respect, but just wanted to double-check).

@mkauf

This comment has been minimized.

Show comment
Hide comment
@mkauf

mkauf Feb 19, 2017

Contributor

@Antony74 thank you for the bug report and the dummy proxy code!

I think the bug should be solved like this (I've used the same comment as in Curl_proxyCONNECT):

diff --git a/lib/http.c b/lib/http.c
index 8db86cd..ef5d007 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1359,6 +1359,10 @@ CURLcode Curl_http_connect(struct connectdata *conn, bool *done)
   if(result)
     return result;
 
+  if(conn->bits.proxy_connect_closed)
+    /* this is not an error, just part of the connection negotiation */
+    return CURLE_OK;
+
   if(CONNECT_FIRSTSOCKET_PROXY_SSL())
     return CURLE_OK; /* wait for HTTPS proxy SSL initialization to complete */
 
Contributor

mkauf commented Feb 19, 2017

@Antony74 thank you for the bug report and the dummy proxy code!

I think the bug should be solved like this (I've used the same comment as in Curl_proxyCONNECT):

diff --git a/lib/http.c b/lib/http.c
index 8db86cd..ef5d007 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1359,6 +1359,10 @@ CURLcode Curl_http_connect(struct connectdata *conn, bool *done)
   if(result)
     return result;
 
+  if(conn->bits.proxy_connect_closed)
+    /* this is not an error, just part of the connection negotiation */
+    return CURLE_OK;
+
   if(CONNECT_FIRSTSOCKET_PROXY_SSL())
     return CURLE_OK; /* wait for HTTPS proxy SSL initialization to complete */
 
@Antony74

This comment has been minimized.

Show comment
Hide comment
@Antony74

Antony74 Feb 20, 2017

@mkauf thanks! Yes that works for me :-)

Much more nicely written than the similar attempt at a fix I posted.

Antony74 commented Feb 20, 2017

@mkauf thanks! Yes that works for me :-)

Much more nicely written than the similar attempt at a fix I posted.

@mkauf

This comment has been minimized.

Show comment
Hide comment
@mkauf

mkauf Mar 7, 2017

Contributor

@jay May I mention you as a reviewer in the commit message (Reviewed-by) ?

Contributor

mkauf commented Mar 7, 2017

@jay May I mention you as a reviewer in the commit message (Reviewed-by) ?

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Mar 7, 2017

Member

yes

Member

jay commented Mar 7, 2017

yes

@mkauf mkauf closed this in e84a863 Mar 11, 2017

@mkauf

This comment has been minimized.

Show comment
Hide comment
@mkauf

mkauf Mar 11, 2017

Contributor

Fixed, thanks!

Contributor

mkauf commented Mar 11, 2017

Fixed, thanks!

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.