Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

curl --fail --negotiate behavior changed between 7.64.0 and 7.65.0 #3992

Closed
nicowilliams opened this issue Jun 5, 2019 · 4 comments
Closed

curl --fail --negotiate behavior changed between 7.64.0 and 7.65.0 #3992

nicowilliams opened this issue Jun 5, 2019 · 4 comments
Labels
authentication help wanted HTTP

Comments

@nicowilliams
Copy link

Per #3726 I am opening a new issue for this.

$ curl --fail --negotiate -u: http://localhost:54321/x

fails with exit code 22 and no useful messages. Enabling verbose output also shows nothing telling about what happened.

The issue is that if even the initial gss_init_sec_context() call fails, then conn->data->state.authproblem gets set to TRUE.

Certainly for HTTP/Negotiate (and only HTTP/Negotiate) any failures of followup gss_init_sec_context() calls (when the input_token is non-empty) should be fatal, but not the initial one.
@nicowilliams nicowilliams mentioned this issue Jun 5, 2019
Closed
@bagder
Copy link
Member

bagder commented Jun 6, 2019

We should up our efforts in adding tests for GSS...

@nicowilliams
Copy link
Author

@bagder yeah... Heimdal has a useful tool for testing, kimpersonate, that might help that.

@kdudka
Copy link
Contributor

kdudka commented Jul 29, 2019

I do not think that you need to have a working GSS setup to test this class of bugs. These bugs are about changes of behavior in cases where HTTPAUTH_GSSNEGOTIATE is enabled but no valid GSS credentials (or even configuration) available.

Before 6c60355 there was either no failure at all (because gss_init_sec_context() was not called unless the server asked for GSS-based authentication) or the failure of gss_init_sec_context() did not result in a failed transfer (because the HTTP request succeeded without any authentication in the end).

@kdudka
Copy link
Contributor

kdudka commented Jul 29, 2019

It is not only --fail what is broken. If HTTPAUTH_GSSNEGOTIATE is used for a POST request and gss_init_sec_context() fails, the POST request is now sent with empty body. I am experimenting with the following patch:

--- a/lib/http_negotiate.c
+++ b/lib/http_negotiate.c
@@ -151,7 +151,7 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy)
       if(result == CURLE_LOGIN_DENIED) {
         /* negotiate auth failed, let's continue unauthenticated to stay
          * compatible with the behavior before curl-7_64_0-158-g6c6035532 */
-        conn->data->state.authproblem = TRUE;
+        authp->done = TRUE;
         return CURLE_OK;
       }
       else if(result)

... and will open a pull request once I have some test-coverage for it...

kdudka added a commit to kdudka/curl that referenced this issue Jul 30, 2019
@kdudka
http_negotiate: improve handling of gss_init_sec_context() failures
d541eb9 
If HTTPAUTH_GSSNEGOTIATE was used for a POST request and
gss_init_sec_context() failed, the POST request was sent
with empty body.  This commit also restores the original
behavior of `curl --fail --negotiate`, which was changed
by commit 6c60355.

Add regression tests 2077 and 2078 to cover this.

Fixes curl#3992
@kdudka kdudka mentioned this issue Jul 30, 2019
Closed
@kdudka kdudka changed the title curl --fail --negotiate behavior changed in 7.64.0 and 7.65.0 curl --fail --negotiate behavior changed between 7.64.0 and 7.65.0 Jul 30, 2019
@kdudka kdudka closed this as completed in 4c18704 Aug 1, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
authentication help wanted HTTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http_negotiate: improve handling of gss_init_sec_context() failures kdudka/curl
3 participants
@bagder @kdudka @nicowilliams