Bugs item #2221237, was opened at 2008-11-04 16:02
Message generated for change (Settings changed) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2221237&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: bad behaviour
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Christian Krause (chkr)
Assigned to: Daniel Stenberg (bagder)
Summary: infinite loop during GSS authentication

Initial Comment:
When using GSS authentication (krbv5) and the authentication fails, the call to "curl_easy_perform" doesn't return, because libcurl gets stuck internally in an infinite loop.

Here is the information I've collected so far:

a) overview:

- on client side the kerberos tickets are valid and it is possible to get a service ticket
- libcurl is configured with "curl_easy_setopt" to use CURLAUTH_GSSNEGOTIATE
- username and password are set to dummy values to trigger authentication code in libcurl
- finally curl_easy_perform is called, but it doesn't return

b) on the network:

1 curl sends HTTP request with kerberos "Negotiate" authentication data (service ticket)
2 server doesn't accept the ticket and returns 401 (this is an intended testcase, it is a valid error condition)
3 goto 1

c) expected behavior

When an HTTP client receives a 401 and it has already sent authentication data _and_ this data cannot be altered (e.g. by asking the user again for a username and password), then the client should not repeat the request but return the error to the upper layer.

d) inside libcurl
I've tracked down the problem to the following functions within libcurl:

- the do-loop in curl/lib/transfer.c in function Curl_perform loops forever
- the reason is, that "follow" is set to FOLLOW_REDIR because "data->req.newurl" was set
- data->req.newurl is set (when the server responds with the return code 401) in "Curl_http_input_auth" in curl/lib/http.c:

#ifdef HAVE_GSSAPI
  if(checkprefix("GSS-Negotiate", start) ||
      checkprefix("Negotiate", start)) {
    *availp |= CURLAUTH_GSSNEGOTIATE;
    authp->avail |= CURLAUTH_GSSNEGOTIATE;
    if(authp->picked == CURLAUTH_GSSNEGOTIATE) {
      /* if exactly this is wanted, go */
      int neg = Curl_input_negotiate(conn, (bool)(httpcode == 407), start);
      if(neg == 0) {
        DEBUGASSERT(!data->req.newurl);
        data->req.newurl = strdup(data->change.url);
        data->state.authproblem = (data->req.newurl == NULL);
      }
      else {
        infof(data, "Authentication problem. Ignoring this.\n");
        data->state.authproblem = TRUE;
      }
    }
  }
  else
#endif

- it looks like that authp->picked is only set after the 1st try sending the credentials
- this means, that if this code path is entered, usually already an authentication has failed
- so if I disable setting "data->req.newurl" and "data->state.authproblem" in this case, everything works fine

Basically curl should not resend GSS credentials in case of a 401 response. My change seems to correct this behavior, but I'm not 100% whether it is the correct way to fix the problem.

Best regards,
Christian

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-12-08 14:53

Message:
Thanks for the report, this problem is now fixed in CVS!

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-12-08 14:52

Message:
I'm fine with this, as I'm sure there will be bugs reported if people find
(more) problems. Thanks for your patience and feedback. I've committed the
fix now and I intend to close this entry.

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

Comment By: Christian Krause (chkr)
Date: 2008-12-05 19:19

Message:
from the original test cases:

1. server announces only one supported authentication methods or
2. server announces several supported authentication methods
and
3. curl library is configured to use only one method or
4. curl library is configured to use multiple methods

the following combinations were tested:

1 & 3: works fine with patch -v4
2 & 3: works fine with patch -v4

The combination with 4. are untested so far. Right now the "wsman" client
software I'm using for performing these test does not support to use the
curl library in a way where more than one auth method is requested by the
client.
So from my side I would consider this issue as fixed. If you think that
the outstanding combinations should be tested anyway, I would try to find
some time to modify the client to behave in this way and do the tests.

Thank you again for all your help!

Best regards,
Christian

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

Comment By: Christian Krause (chkr)
Date: 2008-12-05 18:56

Message:
File Added: curl-krb-test1-working-case-patch-v4.txt

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

Comment By: Christian Krause (chkr)
Date: 2008-12-05 18:55

Message:
File Added: curl-krb-test1-non-working-case-patch-v4.txt

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

Comment By: Christian Krause (chkr)
Date: 2008-12-05 18:54

Message:
Hi,

I'm very sorry for the late response, I was on a business trip early this
week.

I've tried out patch -v4 and it solved the problem. Thank you very much!
I'll attach the log files just for completeness.

Later I'll try out the other test cases (e.g. the server announces
multiple auth methods).

Best regards,
Christian

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-12-03 20:17

Message:
ping?

Without feedback I think I'll commit my latest version as I believe it is
at least better than what's in CVS now.

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-28 22:35

Message:
Thanks for your patient and reports. I just uploaded 2221237-v4.patch which
adds a 'state' variable in the negotiate struct to make it easier for us to
figure out what the code thinks about the negotiate state and how to react
if we get a 401 or 407 back after having sent out a Negotiate auth header
in the request.

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

Comment By: Christian Krause (chkr)
Date: 2008-11-28 18:41

Message:
> Regarding the output_auth_headers() function: won't the
> Curl_input_negotiate() be needed to get called to for example know if
it
> was Negotiate or GSS-Negotiate in the header? If not, we could probably
> move the calling of that function entirely to output_auth_headers() and
it
> would be able to send the auth header with one round-trip less.

Yes, you are right, this piece of information is needed (and the
additional round-trip doesn't hurt much ;-) ).

> Anyway, I'm of course again interested in your results with my v3
patch.

I've attached both logs (infinite loop still happens... :-( ).

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

Comment By: Christian Krause (chkr)
Date: 2008-11-28 18:24

Message:
File Added: curl-krb-test1-working-case-patch-v3.txt

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

Comment By: Christian Krause (chkr)
Date: 2008-11-28 18:24

Message:
File Added: curl-krb-test1-non-working-case-patch-v3.txt

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-25 23:03

Message:
Ah yes it makes perfect sense. The 'picked' field is set to the 'want'
bitmask by default, so when that is one bit only it'll fit that condition.
My v3 patch is now changing the Curl_http_input_auth() function again to
something that makes more sense to me.

Regarding the output_auth_headers() function: won't the
Curl_input_negotiate() be needed to get called to for example know if it
was Negotiate or GSS-Negotiate in the header? If not, we could probably
move the calling of that function entirely to output_auth_headers() and it
would be able to send the auth header with one round-trip less.

Anyway, I'm of course again interested in your results with my v3 patch.

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

Comment By: Christian Krause (chkr)
Date: 2008-11-24 19:48

Message:
I've attached both requested logfiles using the new patch.

I've seen in the debug output that even in the first iteration kerberos
was already
"picked": http_output_auth: HTTP auth host picked 4 want 4 .

After digging a little bit in the source I think that the auth data
is not sent in this first iteration, because the kerberos context
(data->state.negotiate.context) is still not initialized when
output_auth_headers is called the first time. The krb5 context is setup in
Curl_input_negotiate.

Nevertheless, I don't know what would happen even if the first try would
send out the auth data and the authentication would still fail - but I
would assume that it would end up in the infinite loop as well. :-(

But I'm not sure....

File Added: curl-krb-test1-working-case-patch-v2.log

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

Comment By: Christian Krause (chkr)
Date: 2008-11-24 19:46

Message:
File Added: curl-krb-test1-non-working-case-patch-v2.log

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-24 15:34

Message:
Ok, I wrote an updated patch with more verbose output added to it so try
that instead and show me the output. (It has no other new functionality
than the previous one.)

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-24 00:17

Message:
AFAICS, it would only say "Authentication problem. Ignoring this" (in the
working case) on receiving the first header if data->state.authhost.picked
is already set to CURLAUTH_GSSNEGOTIATE already. Can you figure out why it
is?

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

Comment By: Christian Krause (chkr)
Date: 2008-11-20 19:18

Message:
File Added: curl-krb-test1-working-case-patch.log

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

Comment By: Christian Krause (chkr)
Date: 2008-11-20 19:17

Message:
Hi,

I've applied your patch and tested both of the test cases (auth. works and
auth. fails) - but with the same results...
The only change is, that there is an additional debug output:
+* Authentication problem. Ignoring this.

I'll will attach both logfiles right now.

Best regards,
Christian
File Added: curl-krb-test1-non-working-case-patch.log

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-20 17:23

Message:
Great input. Now I have some ideas of where to start: (and I'll refer to
function in current CVS)

First, in Curl_http_input_auth() there's clearly an error:

The code that deals with "Negotiate" wants ->picked to be
CURLAUTH_GSSNEGOTIATE before it calls Curl_input_negotiate() but that's not
smart. This function gets called when the WWW-Authenticate header is
detected and at that point ->picked may not have been set. Thus, the
Curl_input_negotiate() function should either be called on all Negotiate
headers being received, or it should be saved (I believe it can be put on
hold since there's nothing in the header that the function needs to save or
keep around) and called first in the output_auth_headers() when the
outgoing headers are to be generated.

I think my previous mistake was to compare with the NTLM code when we
should rather look at the Digest as a better comparison.

I'll upload a suggested patch right now.

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

Comment By: Christian Krause (chkr)
Date: 2008-11-20 15:08

Message:
File Added: curl-krb-test1-non-working-case.log

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

Comment By: Christian Krause (chkr)
Date: 2008-11-20 15:07

Message:
Hi,

> Since I can't reproduce this case I think we should just slowly go over
> what the code does and what it should do (instead). First, I take it
your

Please apologize the late answer! Ok. Let's examine the cases one by one.
;-)

1. server announces only one supported authentication methods or
2. server announces several supported authentication methods

and

3. curl library is configured to use only one method or
4. curl library is configured to use multiple methods

These cases then could be combined.

> app asks for multiple auth methods and not only GSS?

No, not right now. My test case was a combination of
2. and 3.

> Can you then please make a full protocol recording (possibly using
> CURLOPT_DEBUGFUNCTION) of the communication up until the situation
turns
> wrong and post it here?

Sure. I've started with the most trivial case (1. combined with 3.). Here
is the debug output using CURLOPT_VERBOSE:

curl-krb-test1-working-case.log:
- authentication was successful

curl-krb-test1-non-working-case.log:
- authentication failed (intentional testcase, because the credentials (in
this case kerberos)
are not recognized by the server)
- execution get stuck in the "do"-loop in Curl_perfrom (in
curl/lib/transfer.c)

If you need more information, please let me know.

Best regards,
Christian
                                            
File Added: curl-krb-test1-working-case.log

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-06 08:30

Message:
Since I can't reproduce this case I think we should just slowly go over
what the code does and what it should do (instead). First, I take it your
app asks for multiple auth methods and not only GSS?

Can you then please make a full protocol recording (possibly using
CURLOPT_DEBUGFUNCTION) of the communication up until the situation turns
wrong and post it here?

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

Comment By: Christian Krause (chkr)
Date: 2008-11-05 17:57

Message:
> Ok, I believe there's already such a marker in the
> 'data->state.authhost.done' variable as that is set TRUE when the auth
is
> believed to be "complete".

> Thus I suggest a fix similar to this and I'd love your feedback on it:

I've tried the patch, but unfortunately it breaks the GSS auth in the
working case. It never tries to send out the authentication data in this
case.

Playing around with my patch I've found out that the problem is even more
complicated:

My patch only works, because the server announces 2 possible
Authentication mechanisms:
- Digest
- Negotiate
This triggers the code in http_digest.c:
   /* We had a nonce since before, and we got another one now without
       'stale=true'. This means we provided bad credentials in the
previous
       request */
    if(before && !d->stale) {
      return CURLDIGEST_BAD;
    }
which in order sets data->state.authproblem in Curl_http_input_auth() and
so the Curl_perform loop is exited.
If the server only announces WWW-Authenticate: Negotiate, the problem
still happens even with my patch.

This means, that there is also a problem in the DIGEST code. The client
should not assume that there is an authentication error using
the digest mechanism, if it is not used by the client.

Sure, I assume that the described problems are corner cases (multiple
WWW-Authentication headers, Negotiate auth etc.) . ;-)
So I'm unsure right now how we should procede.

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-04 22:09

Message:
Ok, I believe there's already such a marker in the
'data->state.authhost.done' variable as that is set TRUE when the auth is
believed to be "complete".

Thus I suggest a fix similar to this and I'd love your feedback on it:

--- lib/http.c 28 Oct 2008 23:34:19 -0000 1.402
+++ lib/http.c 4 Nov 2008 21:07:01 -0000
@@ -739,7 +739,8 @@
     authp->avail |= CURLAUTH_GSSNEGOTIATE;
     if(authp->picked == CURLAUTH_GSSNEGOTIATE) {
       /* if exactly this is wanted, go */
- int neg = Curl_input_negotiate(conn, (bool)(httpcode == 407),
start);
+ int neg = authp->done ||
+ Curl_input_negotiate(conn, (bool)(httpcode == 407), start);
       if(neg == 0) {
         DEBUGASSERT(!data->req.newurl);
         data->req.newurl = strdup(data->change.url);

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

Comment By: Christian Krause (chkr)
Date: 2008-11-04 17:47

Message:
Hi,

> But what if you ask for more than one auth metod, then libcurl MUST
respond
> when it gets a 401 since it didn't ask for any auth at all in the
initial
> request!

Yes, that's correct. I've checked this again and it looks like that the
data flow on the network is really:

1 curl sends HTTP req without any auth data
2 server responds with 401
3 curl sends HTTP request with kerberos "Negotiate" authentication data
(service ticket)
4 server doesn't accept the ticket and returns 401 (this is an intended
testcase, it is a valid error condition)
5 goto 3

But nevertheless, if I just comment out the mentioned two lines, it still
works perfectly in the non-error condition.

> The problem is rather that we should "mark" when we send off GSS auth in
a
> request as then if we get back to that point in lib/http.c you mention
> above we know that it is wrong. What do you say?

Yes, something like this sounds good. I really don't know the source of
libcurl very well, but I could imagine that there is a data structure
holding the possible auth methods (from client side). In case an gss auth
went wrong, the auth method could be removed from it. Just by looking at
the source it looks like that pick->avail or pick->want hold the available
auth methods, but I'm unsure about their real sematics...

> (I've never used GSS myself and I have no way of testing it...)

Ok. ;-) If something needs to be tested or checked, I can easily help
out.

Thank you very much in advance.

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-04 16:44

Message:
But what if you ask for more than one auth metod, then libcurl MUST respond
when it gets a 401 since it didn't ask for any auth at all in the initial
request!

The problem is rather that we should "mark" when we send off GSS auth in a
request as then if we get back to that point in lib/http.c you mention
above we know that it is wrong. What do you say?

(I've never used GSS myself and I have no way of testing it...)

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

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