cURL / Mailing Lists / curl-library / Single Mail

curl-library

bug #39 -- let's fix it

From: Nico Williams <nico_at_cryptonector.com>
Date: Mon, 27 Apr 2015 17:26:53 -0500

This oldie but goodie:

http://curl.haxx.se/mail/lib-2007-01/0045.html

is a race condition that affects protocols like FTP and SSH -- any
protocol where the server speaks first or speaks without waiting for the
client to speak.

I maintain a proprietary [i.e., not yet open source] program to tunnel
over proxies using HTTP CONNECT. This progra uses libcurl internally.
It's effectively a netcat workalike, and it's used as a ProxyCommand for
the OpenSSH ssh client. Curl bug #39 causes SSH connections to fail
from time to time, depending on the phase of the moon and other things
-- it's a race condition.

The reply given then was:

http://curl.haxx.se/mail/lib-2007-01/0046.html

"
  The history of this function is sadly enough that we once did the reading byte
  by byte like in your patch, but we found cases where the proxy didn't behave
  properly so we had to give in and read as much as possible so that we could
  survive the cases when the proxy delivers some bonus junk after the response.
"

I'm not sure what proxies send garbage after the response, but such
proxies are clearly busted and should get fixed...

Even if such proxies exist, the code in Curl_proxyCONNECT() is racy when
speaking SSH (for example), so it seems to me that Steffen Rumler's fix
should be applied.

The fix to apply should be trimmed to just this:

index f3c54bd..94a384a 100644
--- a/lib/http_proxy.c
+++ b/lib/http_proxy.c
@@ -269,8 +269,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           break;
         default:
           DEBUGASSERT(ptr+BUFSIZE-nread <= data->state.buffer+BUFSIZE+1);
- result = Curl_read(conn, tunnelsocket, ptr, BUFSIZE-nread,
- &gotbytes);
+ result = Curl_read(conn, tunnelsocket, ptr, 1, &gotbytes);
           if(result==CURLE_AGAIN)
             continue; /* go loop yourself */
           else if(result)

(I.e., there's no need for the #if 0, nor the comment. IMO, of course,
your opinion might differ :)

Now, perhaps such proxies exist and are unfixable. If so, for the sake
of the victims of such proxies, I'd propose adding a CURLOPT/CURLMOPT
for signalling the need to drop garbage from the proxy, and perhaps even
a number of bytes to drop (this would not be racy, but requires knowing
how many bytes to drop). If desired, I'll write and contribute such a
patch. Please let me know.

There's nothing to do in curl(1) proper, just libcurl. Though the
curl-based netcat I maintain might be a useful contribution as well
(pending corporate approvals), if desired.

Comments?

Nico

-- 
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2015-04-28