Bugs item #3545398, was opened at 2012-07-18 07:06
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3545398&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: wrong behaviour
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Joe Mason (jmasonrim)
Assigned to: Daniel Stenberg (bagder)
Summary: auth status not being cleared when handles are reset

Initial Comment:
Using libcurl 7.24.0:

I'm trying to set up an app to connect to a server that supports GSS-Negotiate and NTLM auth. If the client-side auth setup is incorrect I want to fall back to using NTLM.

So this is pretty simple:

void doRequest(const char* url, const char* username, const char* password, bool allowNegotiate)
{
    int numErrors = 0;

    CURL* handle = curl_easy_init();
    curl_easy_setopt(handle, CURLOPT_URL, url);
    curl_easy_setopt(handle, CURLOPT_HTTPGET, true);
    curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, writeCallback);
    curl_easy_setopt(handle, CURLOPT_HEADERFUNCTION, headerCallback); // increments numErrors on every 401
    curl_easy_setopt(handle, CURLOPT_HEADERDATA, &numErrors);
    curl_easy_setopt(handle, CURLOPT_USERNAME, username);
    curl_easy_setopt(handle, CURLOPT_PASSWORD, password);
    long allowedAuthSchemes = CURLAUTH_NTLM;
    if (allowNegotiate) {
        allowedAuthSchemes |= CURLAUTH_GSSNEGOTIATE;
    }
    curl_easy_setopt(handle, CURLOPT_HTTPAUTH, allowedAuthSchemes);
    curl_easy_perform(handle);
    curl_easy_cleanup(handle);

    // expect one 401 to be sent during auth setup; anything more than that means auth failed
    if (numErrors > 1 && allowNegotiate) {
        // try again without Negotiate
        doRequest(url, username, password, false);
    }
}

We set up a request allowing CURLAUTH_NTLM | CURLAUTH_GSSNEGOTIATE and, if it receives too many 401 errors, send it again with just CURLAUTH_NTLM.

To both requests, the server will send back:

WWW-Authenticate: Negotiate
WWW-Authenticate: NTLM

The problem comes in Curl_http_input_auth:

    if(checkprefix("GSS-Negotiate", start) ||
       checkprefix("Negotiate", start)) {
      int neg;
      *availp |= CURLAUTH_GSSNEGOTIATE;
      authp->avail |= CURLAUTH_GSSNEGOTIATE;

      if(authp->picked == CURLAUTH_GSSNEGOTIATE) {
        if(data->state.negotiate.state == GSS_AUTHSENT) {
          /* if we sent GSS authentication in the outgoing request and we get
             this back, we're in trouble */
          infof(data, "Authentication problem. Ignoring this.\n");
          data->state.authproblem = TRUE;
        }
        else {
          neg = Curl_input_negotiate(conn, (bool)(httpcode == 407), start);
          if(neg == 0) {
            DEBUGASSERT(!data->req.newurl);
            data->req.newurl = strdup(data->change.url);
            if(!data->req.newurl)
              return CURLE_OUT_OF_MEMORY;
            data->state.authproblem = FALSE;
            /* we received GSS auth info and we dealt with it fine */
            data->state.negotiate.state = GSS_AUTHRECV;
          }
          else
            data->state.authproblem = TRUE;
        }
      }
    }

The first time through, it reads the Negotiate header, updates "avail", and then does nothing since "picked" is 0. After reading all the headers, it will set set "picked" to CURLAUTH_GSSNEGOTIATE because that's the highest priority, and attempt to output a negotiate header. (Which will fail because, for this test, GSS auth is set up wrong.)

The second time through, it reads the Negotiate header, updates "avail", and then goes into the "if" statement because "picked" is still CURLAUTH_NEGOTIATE from the first pass. Even though we're using a different handle! It then calls Curl_input_negotiate, which will call some gss functions and return an error code when they fail. It then sets authproblem to TRUE, which means that even though it reads the NTLM header it will not continue to send the NTLM auth as we want. It seems to me that it should be ignoring this Negotiate header entirely, since in this second pass CURLAUTH_GSSNEGOTIATE isn't even included in CURLOPT_HTTPAUTH.

(I've also tried adding CURLOPT_FRESH_CONNECT when allowNegotiate is false, to force it to use a new connection, but "picked" is still set.)

So it looks like "picked" should be cleared at some point when Negotiate auth fails, so that it doesn't keep trying to use it for further requests. But I'm not sure where this is should be done.

Another option would be to change that if statement to "if(authp->picked == CURLAUTH_GSSNEGOTIATE && authp->want & CURLAUTH_GSSNEGOTIATE)", so that if we don't want negotiate it just ignores the value of picked here, but I think that would just be hiding the problem.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2012-08-05 15:38

Message:
Thanks for the report, this problem is now fixed in the git repository.

To try it out, you either checkout/update your git clone:
http://curl.haxx.se/source.html

or you try tomorrow's daily snapshot: http://curl.haxx.se/snapshots/

----------------------------------------------------------------------

Comment By: Joe Mason (jmasonrim)
Date: 2012-08-03 14:37

Message:
I now have a fix in
https://github.com/JoeNotCharles/curl/commits/reset_fix. There are 2
patches with separate independent fixes, 1 with new tests, and 5
refactoring the test suite:

24c43e9d34615236489bde4797ce50de4bb56a84 - add 9 new tests covering this
bug

50b87c4e689088fc3ddcf2fac163b75f839ef69a - if receiving a bare "NTLM"
header after NTLM auth has started, curl always considers this an "internal
error" and aborts. But if NTLM has reached NTLMSTATE_TYPE3, a bare "NTLM"
is expected - it just means the password has been rejected and the server
is saying to try again. In this case, we need to clear the NTLM state so
it will be restarted.

ce8311c7e49eca93c136b58efa6763853541ec97 - zero our state.authhost and
state.authproxy in Curl_pretransfer, to ensure that auth->picked is not
incorrectly inherited from the last transfer.

5 patches from Aug 5: refactor sws to allow multiple sockets and poll (this
still has an intermittent bug where sometimes sockets that haven't been
used for a while are closed, and then sws goes into an infinite loop. But
I've been able to run the test suite through several times, and without the
refactor it goes into this loop with the new tests 100% of the time.)

----------------------------------------------------------------------

Comment By: Joe Mason (jmasonrim)
Date: 2012-07-31 11:53

Message:
Using the -f option to runtest.pl (to make the server fork a new process to
handle each connection) also works around the NTLM loop, although it leaves
a bunch of copies of sws running when the test finishes. (Which prevents
it from running again until you kill them, because they're still using the
port.)

----------------------------------------------------------------------

Comment By: Joe Mason (jmasonrim)
Date: 2012-07-31 10:09

Message:
The infinite loop in the NTLM tests seems to be a limitation of the test
suite:

When sending a new NTLM request with a different password, this code forces
it to use a new connection:

        if((needle->handler->protocol & CURLPROTO_FTP) ||
           ((needle->handler->protocol & CURLPROTO_HTTP) &&
            ((data->state.authhost.want==CURLAUTH_NTLM) ||
             (data->state.authhost.want==CURLAUTH_NTLM_WB)))) {
          /* 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;
          }

But when that happens the old connection is never closed (I think this is
correct; it will be closed if the connection cache fills up, or by the
server on a timeout).

However the sws test server can't deal with requests coming on more than
one connection at once. It ignores the second request which is coming on a
second connection, and so the server is waiting forever for the client to
either close the first connection or send another request on it, and the
client is waiting forever for the server to respond to its second request.

I can cause the test to not infinite loop by adding "swsclose" to data100
and data1402 (the two requests which came right before an NTLM request that
has a different password) to close the socket from the server side. But I'd
prefer to fix the server so that this test doesn't depend so closely on the
internals of when connections can be reused.

(After working around the infinite loop, the test still fails: "NTLM
handshake failure (internal error)" in the logs.)

As an aside, I think the above check is too strict: if a connection has
never had an NTLM request (or another connection-oriented auth), it can be
reused for NTLM no matter what the password is. Currently it forces a new
connection if you send a Basic auth request with password "pass1" and then
an NTLM request with password "pass2". I think this should be ok. But what
it's doing is correct, just not efficient.

----------------------------------------------------------------------

Comment By: Joe Mason (jmasonrim)
Date: 2012-07-27 11:38

Message:
Better: I have a test suite! See the reset_test branch at
https://github.com/JoeNotCharles/curl.git. (Also attached as a patch.)

I added 9 tests (starting from test2023) which are each driven by a tool
called "libauthretry" - I chose to name it this rather than following the
pattern of naming it lib2023 because it's used from so many tests, I
thought a descriptive name was better.

The tool takes two auth types (for example Digest and NTLM) and sends the
following requests:

create new curl handle with curl_easy_init
(A) get url using testuser:wrongpass enabling only auth type 1 - expect
401
curl_easy_reset
(B) get url using testuser:testpass enabling only auth type 2 - expect 200
create new handle with curl_easy_cleanup / curl_easy_init
(C) get url using testuser:wrongpass enabling only auth type 1 - expect
401
curl_easy_reset
(D) get url using testuser:wrongpass enabling only auth type 2 - expect
401
curl_easy_reset
(E) get url using testuser:testpass enabling only auth type 2 - expect 200
curl_easy_cleanup

So for a given pair of auth types, this tests whether data persists across
a handle reuse (B), whether it persists with a connection even with a new
handle (C), whether an authproblem messes up a subsequent successful auth
(A -> B), whether an authproblem messes up a subsequenct failed auth (C ->
D), and whether an authproblem still messes up if the following auth is of
the same type as the original (D -> E).

The 9 tests run this tool on each pair of auth types, including having both
the "main" and "fallback" auth types be the same. For example, test2023 is
"basic basic", and tests whether retrying an auth with the same handle
using only Basic auth works; test2024 is "basic digest", and tests whether
retrying a failed Basic auth using Digest auth works; test2026 is "digest
basic", which is the opposite - start with digest and fall back to basic.

I made all these tests because in my quick attempts to fix this I found
that by fixing NTLM I caused new failures in Digest, so it seems like the
code paths for each auth type use different information. So it's important
to test each different auth type as both the first auth (which leaves state
behind) and the second auth (which reads the state).

I doubt the actual contents of the Digest and NTLM headers are correct -
the important thing right now is whether requests with Basic, Digest and
NTLM auth are sent at the right times. While fixing, if we get a test to
the point where it succeeds except that the hashes aren't correct, we can
fix the tests to use the hashes we see curl sending.

Running these I see:

test2023 - basic only - succeeds

test2024 - basic -> digest - curl continues to use Basic auth for
subsequent requests even though only Digest is enabled ("picked" is not
reset). Also it prints "Authentication problem. Ignoring this." before many
but not all WWW-Authenticate headers, suggesting authproblem is set
incorrectly.

test2025 - basic -> ntlm - gets in an infinite loop printing "additional
stuff not fine transfer.c:1037: 0 0"

test2026 - digest -> basic - the handle receives a 401 with digest nonce
"2" before the reset. After the reset, the first request sent out by curl
includes a digest with nonce "2", even though only Basic is enabled.
(Using that nonce would actually be correct for Digest, but I believe the
reset should have cleared curl's knowledge of auth schemes the site
supports, so there should be no way for curl to know that - I expect it to
send a new request with no authorization)

test2027 - digest only - as above, after the reset sends a request with the
nonce from before the reset (which succeeds because this is an acceptable
thing to do). However, when sending multiple failed digests in a with a
reset between, curl prints "Ignoring duplicate digest auth header." and
continues to use the same nonce over and over.

test2028 - digest -> ntlm - gets in an infinite loop printing "additional
stuff not fine transfer.c:1037: 0 0"

test2029 - ntlm -> basic - whenever switching to Basic auth after using
NTLM, curl prints "NTLM handshake failure (internal error)", showing that
it has saved the NTLM transaction state and is expecting this new request
to continue the previous NTLM auth transaction. Also it continues to
choose NTLM over Basic even when NTLM is not included in CURLOPT_AUTH

test2030 - ntlm -> digest - pretty much the same as 2029, except now I also
see "Authentication problem. Ignoring this." and "Ignoring duplicate digest
auth header." from Digest, although curl never chooses to actually use
digest.

test2031 - ntlm only - gets in an infinite loop printing "additional stuff
not fine transfer.c:1037: 0 0"

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2012-07-26 14:07

Message:
Thanks for the report.

Do you have any recipe to repeat/see this problem in a decent way?

----------------------------------------------------------------------

Comment By: Joe Mason (jmasonrim)
Date: 2012-07-18 07:54

Message:
Actually I think picked needs to be cleared even on a successful auth. I
have a server that serves most resources with "WWW-Authenticate: Negotiate"
and "WWW-Authenticate: Basic" headers, but one resource (call it B) that
only has Basic for some reason. When fetching a page that includes
subresources A, B, C, curl chooses Negotiate for A and Basic for B (since
there's no Negotiate header to trigger the code above). I would expect curl
to choose Negotiate again for C since it's most secure, but actually it
chooses Basic again because picked is still set from the last request.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3545398&group_id=976
Received on 2012-08-06