This is a combined patch consisting of upstream commits 8ae35102, 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 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 | 44 ++++++++++---------- tests/data/Makefile.am | 1 + tests/data/test1418 | 107 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/data/test1419 | 69 +++++++++++++++++++++++++++++++ 4 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 tests/data/test1418 create mode 100644 tests/data/test1419 diff --git a/lib/url.c b/lib/url.c index 5903628..6d5370c 100644 --- a/lib/url.c +++ b/lib/url.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2013, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2014, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -2886,8 +2886,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) ? TRUE : FALSE; + bool wantNTLMhttp = ((data->state.authhost.want & CURLAUTH_NTLM) || + (data->state.authhost.want & CURLAUTH_NTLM_WB)) && + (needle->handler->protocol & CURLPROTO_HTTP) ? TRUE : FALSE; struct connectbundle *bundle; *force_reuse = FALSE; @@ -3042,16 +3043,15 @@ 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; + 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 || @@ -3103,12 +3103,12 @@ ConnectionExists(struct SessionHandle *data, } if(match) { - /* If we are looking for an 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(wantNTLM) { + /* 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; @@ -3116,8 +3116,10 @@ ConnectionExists(struct SessionHandle *data, *force_reuse = TRUE; break; } - else - continue; + else if(credentialsMatch) + /* this is a backup choice */ + chosen = check; + continue; } if(canPipeline) { diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index b2a223b..0d583bb 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -119,6 +119,7 @@ test1396 \ \ test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \ test1408 test1409 test1410 test1412 test1413 test1414 test1415 \ +test1418 test1419 \ \ test1500 test1501 test1502 test1503 test1504 test1505 test1506 test1507 \ test1508 test1509 test1510 test1511 test1512 \ 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