#939 SOCKS proxies do not work with multi interface

closed-fixed
libcurl (356)
5
2013-06-21
2010-07-24
No

I have found that connections via a SOCKS5 proxy are failing when using the multi interface. The symptoms are that the proxy does not receive the setup packets/auth packets and instead libcurl sends the request data directly resulting in a socks protocol error. I have looked through the code in git and done some tests, it appears that libcurl 7.20.0 works fine but 7.20.1 results in socks protocol errors. (I noticed this when upgrading libcurl used by my app from 7.19.6 to 7.21.0.

Further tests indicate that Commit:4b351d018e3d6691191fd653a17f14f4a31e0b4c introduced un expected behaviour with socks proxies and the multi interface.

The problem occurs in the multi interface state machine, because on the multi interface connects are non blocking have have a timeout of 0, the STATE_CONNECT returns before the socket to the proxy is connected and we enter the STATE_WAITCONNECT

case CURLM_STATE_WAITCONNECT:
/* awaiting a completion of an asynch connect */
easy->result = Curl_is_connected(easy->easy_conn,
FIRSTSOCKET,
&connected);
if(connected) {
/* see if we need to do any proxy magic first once we connected */
easy->result = Curl_connected_proxy(easy->easy_conn);

Curl_is_connected() since 4b351d018 sets conn->bits.tcpconnect = TRUE;

Curl_connected_proxy() tests for conn->bits.tcpconnect == TRUE and returns with out setting up the proxy

Attached is a patch that allows Curl_connected_proxy() to run when conn->bits.tcpconnect == TRUE and the multi interface is in use, comments in that function seem to suguest that this should have been allowed?

Discussion

  • Daniel Stenberg

    Daniel Stenberg - 2010-07-24

    Thanks a lot for your report and your help to improve libcurl!

    Hm, doesn't this patch go against the fix the 4b351d018e commit introduced?

    In general I think checks for interface in use is suspicious and I'm far from convinced right now that this patch really is the right thing.

    To me, it looks like we should do something about the confusion around the bits.tcpconnect struct memmber that seems to be used for a bit too many purposes. Its name suggests it is about "TCP connect" but still it is also used to check if the connection with the proxy is completed or whatever that those two things are just not the same and I'm sure that's one reason why we still get problems like this.

    Possibly we should convert it to a state variable: NONE. TCPCONNECTING, PROXYING, DONE and we should let everyone wait for DONE before we can actually assume anything.

    Thoughts?

    (it's a bit late here and I'm on vacation so I'm sorry if I overlooked something obvious...)

     
  • Daniel Stenberg

    Daniel Stenberg - 2010-07-30

    Remove the bits.tcpconnect check in Curl_connected_proxy

     
  • Daniel Stenberg

    Daniel Stenberg - 2010-07-30

    I looked into this again, and I now suggest an even simpler approach. See the newly attached file for the patch that unconditionally removes the check for bits.tcpconnect in the Curl_connected_proxy() function. I consider the check and condition for it completely wrong and I can run all existing test cases just fine with my change applied.

    Can you please confirm that this change also fixes the problem you're experiencing?

     
  • Robin Cornelius

    Robin Cornelius - 2010-07-31

    @bagder, sorry for the delay getting back to you. I'll test out your new patch ASAP. I believe you are correct in the fact that there are only two code paths that can land here one is if a connect occurs immedatly (with a 0 time out) but a connect is verified first so its safe and the 2nd one when we are in the state machine in a connected state so we know its safe as we are connected, so yes that check should be redundent and safe to remove.

     
  • Daniel Stenberg

    Daniel Stenberg - 2010-08-02
    • status: open --> closed-fixed
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks