Bugs item #2221237, was opened at 2008-11-04 16:02
Message generated for change (Comment added) made by chkr
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: Open
Resolution: None
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: 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-11-24