This is a combined patch consisting of upstream commits 8ae35102, 638c6da9, 378af08c and d7650998 in series, which fixes CVE-2014-0015 and the regressions brought by the original upstream patch. ------ ConnectionExists: fix NTLM check for new connection When the requested authentication bitmask includes NTLM, we cannot re-use a connection for another username/password as we then risk re-using NTLM (connection-based auth). This has the unfortunate downside that if you include NTLM as a possible auth, you cannot re-use connections for other usernames/passwords even if NTLM doesn't end up the auth type used. Reported-by: Paras S Patched-by: Paras S Bug: http://curl.haxx.se/mail/lib-2014-01/0046.html proxy: make ConnectionExists() check credential of proxyconnections too Previously it only compared credentials if the requested needle connection wasn't using a proxy. This caused NTLM authentication failures when using proxies as the authentication code wasn't send on the connection where the challenge arrived. Added test 1215 to verify: NTLM server authentication through a proxy (This is a modified copy of test 67) ConnectionExists: reusing possible HTTP+NTLM connections better Make sure that the special NTLM magic we do is for HTTP+NTLM only since that's where the authenticated connection is a weird non-standard paradigm. Regression brought in 8ae35102c (curl 7.35.0) Bug: http://curl.haxx.se/mail/lib-2014-02/0100.html Reported-by: Dan Fandrich ConnectionExists: re-use connections better When allowing NTLM, the re-use connection logic was too focused on finding an existing NTLM connection to use and didn't properly allow re-use of other ones. This made the logic not re-use perfectly re-usable connections. Added test case 1418 and 1419 to verify. Regression brought in 8ae35102c (curl 7.35.0) Reported-by: Jeff King Bug: http://thread.gmane.org/gmane.comp.version-control.git/242213 --- lib/url.c | 57 ++++++++++++++------------ tests/data/Makefile.am | 3 +- tests/data/test1215 | 104 +++++++++++++++++++++++++++++++++++++++++++++++ tests/data/test1418 | 107 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/data/test1419 | 69 +++++++++++++++++++++++++++++++ 5 files changed, 314 insertions(+), 26 deletions(-) create mode 100644 tests/data/test1215 create mode 100644 tests/data/test1418 create mode 100644 tests/data/test1419 diff --git a/lib/url.c b/lib/url.c index 601d8d3..6bc0e9c 100644 --- a/lib/url.c +++ b/lib/url.c @@ -2942,8 +2942,9 @@ ConnectionExists(struct SessionHandle *data, struct connectdata *check; struct connectdata *chosen = 0; bool canPipeline = IsPipeliningPossible(data, needle); - bool wantNTLM = (data->state.authhost.want==CURLAUTH_NTLM) || - (data->state.authhost.want==CURLAUTH_NTLM_WB); + bool wantNTLMhttp = ((data->state.authhost.want & CURLAUTH_NTLM) || + (data->state.authhost.want & CURLAUTH_NTLM_WB)) && + (needle->handler->protocol & CURLPROTO_HTTP) ? TRUE : FALSE; for(i=0; i< data->state.connc->num; i++) { bool match = FALSE; @@ -3084,6 +3085,17 @@ ConnectionExists(struct SessionHandle *data, continue; } + if((needle->handler->protocol & CURLPROTO_FTP) || wantNTLMhttp) { + /* This is FTP or HTTP+NTLM, verify that we're using the same name + and password as well */ + if(!strequal(needle->user, check->user) || + !strequal(needle->passwd, check->passwd)) { + /* one of them was different */ + continue; + } + credentialsMatch = TRUE; + } + if(!needle->bits.httpproxy || needle->handler->flags&PROTOPT_SSL || (needle->bits.httpproxy && check->bits.httpproxy && needle->bits.tunnel_proxy && check->bits.tunnel_proxy && @@ -3117,17 +3129,6 @@ ConnectionExists(struct SessionHandle *data, continue; } } - if((needle->handler->protocol & CURLPROTO_FTP) || - ((needle->handler->protocol & CURLPROTO_HTTP) && wantNTLM)) { - /* This is FTP or HTTP+NTLM, verify that we're using the same name - and password as well */ - if(!strequal(needle->user, check->user) || - !strequal(needle->passwd, check->passwd)) { - /* one of them was different */ - continue; - } - credentialsMatch = TRUE; - } match = TRUE; } } @@ -3144,19 +3145,25 @@ ConnectionExists(struct SessionHandle *data, } if(match) { - chosen = check; - - /* If we are not looking for an NTLM connection, we can choose this one - immediately. */ - if(!wantNTLM) - break; + /* If we are looking for an HTTP+NTLM connection, check if this is + already authenticating with the right credentials. If not, keep + looking so that we can reuse NTLM connections if + possible. (Especially we must not reuse the same connection if + partway through a handshake!) */ + if(wantNTLMhttp) { + if(credentialsMatch && check->ntlm.state != NTLMSTATE_NONE) { + chosen = check; + break; + } + else if(credentialsMatch) + /* this is a backup choice */ + chosen = check; + continue; + } - /* Otherwise, check if this is already authenticating with the right - credentials. If not, keep looking so that we can reuse NTLM - connections if possible. (Especially we must reuse the same - connection if partway through a handshake!) */ - if(credentialsMatch && chosen->ntlm.state != NTLMSTATE_NONE) - break; + /* We have found a connection. Let's stop searching. */ + chosen = check; + break; } } diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 0528a25..1b515a5 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -77,7 +77,7 @@ test1110 test1111 test1112 test1113 test1114 test1115 test1116 test1117 \ test1118 test1119 test1120 test1121 test1122 test1123 test1124 test1125 \ test1126 test1127 test1128 test1129 test1130 test1131 test1132 \ test1200 test1201 test1202 test1203 test1204 test1205 test1206 test1207 \ -test1208 test1209 test1210 test1211 \ +test1208 test1209 test1210 test1211 test1215 \ test1220 \ test1300 test1301 test1302 test1303 test1304 test1305 \ test1306 test1307 test1308 test1309 test1310 test1311 test1312 test1313 \ @@ -93,6 +93,7 @@ test1379 test1380 test1381 test1382 test1383 test1384 test1385 test1386 \ test1387 test1388 test1389 test1390 test1391 test1392 test1393 \ test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \ test1408 test1409 test1410 test1411 test1412 test1413 \ +test1418 test1419 \ test1500 test1501 test1502 \ test2000 test2001 test2002 test2003 test2004 test2005 test2006 test2007 \ test2008 test2009 test2010 test2011 test2012 test2013 test2014 test2015 \ diff --git a/tests/data/test1215 b/tests/data/test1215 new file mode 100644 index 0000000..ea62eeb --- /dev/null +++ b/tests/data/test1215 @@ -0,0 +1,104 @@ + + +# This test is a copy of test 67, modified to use a HTTP proxy. + +HTTP +HTTP GET +HTTP NTLM auth +HTTP proxy + + +# Server-side + + + + + +HTTP/1.1 401 Now gimme that second request of crap +Server: Microsoft-IIS/5.0 +Content-Type: text/html; charset=iso-8859-1 +Content-Length: 34 +WWW-Authenticate: NTLM TlRMTVNTUAACAAAAAgACADAAAAAGgoEAc51AYVDgyNcAAAAAAAAAAG4AbgAyAAAAQ0MCAAQAQwBDAAEAEgBFAEwASQBTAEEAQgBFAFQASAAEABgAYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAwAsAGUAbABpAHMAYQBiAGUAdABoAC4AYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAAAAAA== + +This is not the real page either! + + +# This is supposed to be returned when the server gets the second +# Authorization: NTLM line passed-in from the client + +HTTP/1.1 200 Things are fine in server land swsclose +Server: Microsoft-IIS/5.0 +Content-Type: text/html; charset=iso-8859-1 +Content-Length: 32 + +Finally, this is the real page! + + + +HTTP/1.1 401 Now gimme that second request of crap +Server: Microsoft-IIS/5.0 +Content-Type: text/html; charset=iso-8859-1 +Content-Length: 34 +WWW-Authenticate: NTLM TlRMTVNTUAACAAAAAgACADAAAAAGgoEAc51AYVDgyNcAAAAAAAAAAG4AbgAyAAAAQ0MCAAQAQwBDAAEAEgBFAEwASQBTAEEAQgBFAFQASAAEABgAYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAwAsAGUAbABpAHMAYQBiAGUAdABoAC4AYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAAAAAA== + +HTTP/1.1 200 Things are fine in server land swsclose +Server: Microsoft-IIS/5.0 +Content-Type: text/html; charset=iso-8859-1 +Content-Length: 32 + +Finally, this is the real page! + + + + +# Client-side + + +NTLM + + +http + + +HTTP with server NTLM authorization using a proxy + + +# we force our own host name, in order to make the test machine independent +CURL_GETHOSTNAME=curlhost +# we try to use the LD_PRELOAD hack, if not a debug build +LD_PRELOAD=%PWD/libtest/.libs/libhostname.so + + +http://%HOSTIP:%HTTPPORT/1215 -u testuser:testpass --ntlm --proxy http://%HOSTIP:%HTTPPORT + + +chkhostname curlhost + + + +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET http://%HOSTIP:%HTTPPORT/1215 HTTP/1.1 +Authorization: NTLM TlRMTVNTUAABAAAABoIIAAAAAAAAAAAAAAAAAAAAAAA= +User-Agent: curl/7.30.0-DEV +Host: %HOSTIP:%HTTPPORT +Accept: */* +Proxy-Connection: Keep-Alive + +GET http://%HOSTIP:%HTTPPORT/1215 HTTP/1.1 +Authorization: NTLM TlRMTVNTUAADAAAAGAAYAEAAAAAYABgAWAAAAAAAAABwAAAACAAIAHAAAAAIAAgAeAAAAAAAAAAAAAAABoKBAFpkQwKRCZFMhjj0tw47wEjKHRHlvzfxQamFcheMuv8v+xeqphEO5V41xRd7R9deOXRlc3R1c2VyY3VybGhvc3Q= +User-Agent: curl/7.30.0-DEV +Host: %HOSTIP:%HTTPPORT +Accept: */* +Proxy-Connection: Keep-Alive + + + + diff --git a/tests/data/test1418 b/tests/data/test1418 new file mode 100644 index 0000000..fb4e91a --- /dev/null +++ b/tests/data/test1418 @@ -0,0 +1,107 @@ + + + +HTTP +HTTP GET +HTTP NTLM auth +connection re-use + + +# Server-side + + +connection-monitor + + + +HTTP/1.1 401 Authentication please! +Content-Length: 20 +WWW-Authenticate: Digest realm="loonie", nonce="314156592" +WWW-Authenticate: Basic + +Please auth with me + + +# This is supposed to be returned when the server gets the second +# Authorization: NTLM line passed-in from the client + +HTTP/1.1 200 Things are fine in server land +Server: Microsoft-IIS/5.0 +Content-Length: 4 + +moo + + + +HTTP/1.1 200 OK +Server: Another one/1.0 +Content-Length: 4 + +boo + + +# This is the first reply after the redirection + +HTTP/1.1 200 OK +Server: Microsoft-IIS/5.0 +Content-Type: text/html; charset=iso-8859-1 +Content-Length: 34 + +This is not the real page either! + + + +HTTP/1.1 401 Authentication please! +Content-Length: 20 +WWW-Authenticate: Digest realm="loonie", nonce="314156592" +WWW-Authenticate: Basic + +HTTP/1.1 200 Things are fine in server land +Server: Microsoft-IIS/5.0 +Content-Length: 4 + +moo + + + + +# Client-side + + +http + + +crypto + + +HTTP with --anyauth and connection re-use + + +http://%HOSTIP:%HTTPPORT/1418 -u testuser:testpass --anyauth http://%HOSTIP:%HTTPPORT/14180003 + + + +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /1418 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /1418 HTTP/1.1 +Authorization: Digest username="testuser", realm="loonie", nonce="314156592", uri="/1418", response="986238b7e0077754944c966f56d9bc77" +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /14180003 HTTP/1.1 +Authorization: Digest username="testuser", realm="loonie", nonce="314156592", uri="/14180003", response="1c6390a67bac3283a9b023402f3b3540" +Host: %HOSTIP:%HTTPPORT +Accept: */* + +[DISCONNECT] + + + diff --git a/tests/data/test1419 b/tests/data/test1419 new file mode 100644 index 0000000..f29ce44 --- /dev/null +++ b/tests/data/test1419 @@ -0,0 +1,69 @@ + + + +HTTP +HTTP GET +HTTP NTLM auth +connection re-use + + +# Server-side + + +connection-monitor + + + +HTTP/1.1 200 fine! +Content-Length: 20 + +Feel free to get it + + + +HTTP/1.1 200 OK +Server: Another one/1.0 +Content-Length: 4 + +boo + + + +HTTP/1.1 200 fine! +Content-Length: 20 + +Feel free to get it + + + +# Client-side + + +http + + +HTTP with --anyauth (but no auth!) and connection re-use + + +http://%HOSTIP:%HTTPPORT/1419 --anyauth http://%HOSTIP:%HTTPPORT/14190003 + + + +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /1419 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /14190003 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +[DISCONNECT] + + + -- 1.8.1.5