Bugs item #2221237, was opened at 2008-11-04 16:02
Message generated for change (Comment added) 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: 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: 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-06