All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails
       [not found] <pull.849.git.1611921008282.gitgitgadget@gmail.com>
@ 2021-02-16 16:57 ` Christopher via GitGitGadget
  2021-03-22 11:51   ` [PATCH v3] " Christopher via GitGitGadget
       [not found]   ` <xmqq35xvpr8q.fsf@gitster.c.googlers.com>
  0 siblings, 2 replies; 14+ messages in thread
From: Christopher via GitGitGadget @ 2021-02-16 16:57 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Christopher, Christopher Schenk

From: Christopher Schenk <christopher@cschenk.net>

When the username and password are supplied in a url like this
https://myuser:secret@git.exampe/myrepo.git and the server supports the
negotiate authenticaten method git does not fall back to basic auth and
libcurl hardly tries to authenticate with the negotiate method.

Stop using the Negotiate authentication method after the first failure
because if it fails on the first try it will never succeed.

V1 of this patch somehow did not make it to the mailing list so i will
try to send this patch again

Signed-off-by: Christopher Schenk <christopher@cschenk.net>
---
    remote-curl: fall back to basic auth if Negotiate fails
    
    When the username and password are supplied in a url like this
    https://myuser:secret@git.exampe/myrepo.git and the server supports the
    negotiate authenticaten method git does not fall back to basic auth and
    libcurl hardly tries to authenticate with the negotiate method.
    
    Stop using the Negotiate authentication method after the first failure
    because if it fails on the first try it will never succeed.
    
    Signed-off-by: Christopher Schenk christopher@cschenk.net

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-849%2Fchschenk%2Fkerberos-basic-fallback-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-849/chschenk/kerberos-basic-fallback-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/849

Range-diff vs v1:

 1:  285a8a568444 ! 1:  7bfc0b431910 remote-curl: fall back to basic auth if Negotiate fails
     @@ Commit message
          Stop using the Negotiate authentication method after the first failure
          because if it fails on the first try it will never succeed.
      
     +    V1 of this patch somehow did not make it to the mailing list so i will
     +    try to send this patch again
     +
          Signed-off-by: Christopher Schenk <christopher@cschenk.net>
      
       ## http.c ##


 http.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/http.c b/http.c
index 8b23a546afdf..36f113d46c23 100644
--- a/http.c
+++ b/http.c
@@ -1642,6 +1642,14 @@ static int handle_curl_result(struct slot_results *results)
 		return HTTP_MISSING_TARGET;
 	else if (results->http_code == 401) {
 		if (http_auth.username && http_auth.password) {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+			if (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {
+				http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+				http_auth_methods &= results->auth_avail;
+				http_auth_methods_restricted = 1;
+				return HTTP_REAUTH;
+			}
+#endif
 			credential_reject(&http_auth);
 			return HTTP_NOAUTH;
 		} else {

base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3] remote-curl: fall back to basic auth if Negotiate fails
  2021-02-16 16:57 ` [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails Christopher via GitGitGadget
@ 2021-03-22 11:51   ` Christopher via GitGitGadget
       [not found]   ` <xmqq35xvpr8q.fsf@gitster.c.googlers.com>
  1 sibling, 0 replies; 14+ messages in thread
From: Christopher via GitGitGadget @ 2021-03-22 11:51 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Christopher, Christopher Schenk

From: Christopher Schenk <christopher@cschenk.net>

When the username and password are supplied in a url like this
https://myuser:secret@git.exampe/myrepo.git and the server supports the
negotiate authenticaten method, git does not fall back to basic auth and
libcurl hardly tries to authenticate with the negotiate method.

Stop using the Negotiate authentication method after the first failure
because if it fails on the first try it will never succeed.

Signed-off-by: Christopher Schenk <christopher@cschenk.net>
---
    remote-curl: fall back to basic auth if Negotiate fails
    
    When the username and password are supplied in a url like this
    https://myuser:secret@git.exampe/myrepo.git and the server supports the
    negotiate authenticaten method git does not fall back to basic auth and
    libcurl hardly tries to authenticate with the negotiate method.
    
    Stop using the Negotiate authentication method after the first failure
    because if it fails on the first try it will never succeed.
    
    Signed-off-by: Christopher Schenk christopher@cschenk.net

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-849%2Fchschenk%2Fkerberos-basic-fallback-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-849/chschenk/kerberos-basic-fallback-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/849

Range-diff vs v2:

 1:  7bfc0b431910 ! 1:  52de7fa42f88 remote-curl: fall back to basic auth if Negotiate fails
     @@ Commit message
      
          When the username and password are supplied in a url like this
          https://myuser:secret@git.exampe/myrepo.git and the server supports the
     -    negotiate authenticaten method git does not fall back to basic auth and
     +    negotiate authenticaten method, git does not fall back to basic auth and
          libcurl hardly tries to authenticate with the negotiate method.
      
          Stop using the Negotiate authentication method after the first failure
          because if it fails on the first try it will never succeed.
      
     -    V1 of this patch somehow did not make it to the mailing list so i will
     -    try to send this patch again
     -
          Signed-off-by: Christopher Schenk <christopher@cschenk.net>
      
       ## http.c ##
      @@ http.c: static int handle_curl_result(struct slot_results *results)
     + 	} else if (missing_target(results))
       		return HTTP_MISSING_TARGET;
       	else if (results->http_code == 401) {
     - 		if (http_auth.username && http_auth.password) {
      +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
     -+			if (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {
     -+				http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
     -+				http_auth_methods &= results->auth_avail;
     -+				http_auth_methods_restricted = 1;
     -+				return HTTP_REAUTH;
     -+			}
     ++		http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
     ++		if (results->auth_avail) {
     ++			http_auth_methods &= results->auth_avail;
     ++			http_auth_methods_restricted = 1;
     ++			return HTTP_REAUTH;
     ++		}
      +#endif
     + 		if (http_auth.username && http_auth.password) {
       			credential_reject(&http_auth);
       			return HTTP_NOAUTH;
       		} else {
     +-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
     +-			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
     +-			if (results->auth_avail) {
     +-				http_auth_methods &= results->auth_avail;
     +-				http_auth_methods_restricted = 1;
     +-			}
     +-#endif
     + 			return HTTP_REAUTH;
     + 		}
     + 	} else {


 http.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/http.c b/http.c
index 8b23a546afdf..4b4cfee8185c 100644
--- a/http.c
+++ b/http.c
@@ -1641,17 +1641,18 @@ static int handle_curl_result(struct slot_results *results)
 	} else if (missing_target(results))
 		return HTTP_MISSING_TARGET;
 	else if (results->http_code == 401) {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+		http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+		if (results->auth_avail) {
+			http_auth_methods &= results->auth_avail;
+			http_auth_methods_restricted = 1;
+			return HTTP_REAUTH;
+		}
+#endif
 		if (http_auth.username && http_auth.password) {
 			credential_reject(&http_auth);
 			return HTTP_NOAUTH;
 		} else {
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
-			if (results->auth_avail) {
-				http_auth_methods &= results->auth_avail;
-				http_auth_methods_restricted = 1;
-			}
-#endif
 			return HTTP_REAUTH;
 		}
 	} else {

base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails
       [not found]   ` <xmqq35xvpr8q.fsf@gitster.c.googlers.com>
@ 2021-03-22 16:08     ` Christopher Schenk
  0 siblings, 0 replies; 14+ messages in thread
From: Christopher Schenk @ 2021-03-22 16:08 UTC (permalink / raw)
  To: Junio C Hamano, Christopher via GitGitGadget
  Cc: git, brian m. carlson, Jeff King



On 2/16/21 11:44 PM, Junio C Hamano wrote:
> "Christopher via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Christopher Schenk <christopher@cschenk.net>
>>
>> When the username and password are supplied in a url like this
>> https://myuser:secret@git.exampe/myrepo.git and the server supports the
>> negotiate authenticaten method git does not fall back to basic auth and
> 
> s/method git/method, git/;
> 
>> libcurl hardly tries to authenticate with the negotiate method.
> 
> Thanks.
> 
>> Stop using the Negotiate authentication method after the first failure
>> because if it fails on the first try it will never succeed.
> 
> Is this patch needed because we are using cURL library incorrectly,
> or is it the limitation of the cURL library?
I'm no cURL expert, but in my opinon this is a limitation of the cURL 
Libary.
> 
>> V1 of this patch somehow did not make it to the mailing list so i will
>> try to send this patch again
> 
> The last paragraph does not belong to the commit log message; if
> nobody on the list saw the "v1", as far as the project is concerned,
> it never happened.
I have adapted the commit message accordingly.
> 
>> Signed-off-by: Christopher Schenk <christopher@cschenk.net>
>> ---
> 
>> diff --git a/http.c b/http.c
>> index 8b23a546afdf..36f113d46c23 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -1642,6 +1642,14 @@ static int handle_curl_result(struct slot_results *results)
>>   		return HTTP_MISSING_TARGET;
>>   	else if (results->http_code == 401) {
>>   		if (http_auth.username && http_auth.password) {
>> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
>> +			if (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {
>> +				http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
>> +				http_auth_methods &= results->auth_avail;
>> +				http_auth_methods_restricted = 1;
>> +				return HTTP_REAUTH;
>> +			}
>> +#endif
>>   			credential_reject(&http_auth);
>>   			return HTTP_NOAUTH;
>>   		} else {
> 
> Hmph, is this an extension to what 4dbe6646 (remote-curl: fall back
> to Basic auth if Negotiate fails, 2015-01-08) tried to do?  What
> happens on the "else" side after the context of this patch, which
> seems to have come from:
> 
>   - 4dbe6646 (remote-curl: fall back to Basic auth if Negotiate
>     fails, 2015-01-08)
>   
>   - 840398fe (http: restrict auth methods to what the server
>     advertises, 2017-02-22), and
> 
>   - 40a18fc7 (http: add an "auto" mode for http.emptyauth,
>     2017-02-25),
> 
> looks essentially the same as what this patch is adding, and I am
> wondering if there is a room for simplification.  It almost looks
> to me that the only difference between "credential fully given" and
> other case is if we "reject" the credential after this patch.
> 
> Asking contributors who made these past contributions for input.
> 
> Thanks.
> 

I have simplified the code and sent the patch again.

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-06 16:07         ` Dan Langille (dalangil)
@ 2015-01-08  0:02           ` brian m. carlson
  0 siblings, 0 replies; 14+ messages in thread
From: brian m. carlson @ 2015-01-08  0:02 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Tue, Jan 06, 2015 at 04:07:01PM +0000, Dan Langille (dalangil) wrote:
>< HTTP/1.1 401 Authorization Required
>< Date: Tue, 06 Jan 2015 16:02:48 GMT
>< Server: Apache
>< WWW-Authenticate: Negotiate

Your server is set up incorrectly.  You should see a Negotiate line and 
a Basic line as well.  Right now, you only have Negotiate set up, so if 
you don't have a ticket, it's going to fail.

I'd recommend making sure that you can access it using a web browser 
after running kdestroy.  That will ensure that it's working properly.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-06 15:31       ` Dan Langille (dalangil)
  2015-01-06 15:41         ` Dan Langille (dalangil)
@ 2015-01-06 16:07         ` Dan Langille (dalangil)
  2015-01-08  0:02           ` brian m. carlson
  1 sibling, 1 reply; 14+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-06 16:07 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

> On Jan 6, 2015, at 10:31 AM, Dan Langille (dalangil) <dalangil@cisco.com> wrote:
> 
> On Jan 5, 2015, at 6:53 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>> 
>> On Mon, Jan 05, 2015 at 09:23:32PM +0000, Dan Langille (dalangil) wrote:
>>> I have tried both patches.  Neither succeeds here.  I patched git version 2.2.1 but I don’t think that affects this.
>> 
>> You are patching the client side, correct?  That's the side that needs patching here.
> 
> Yes, I am.
> 
>> Just so the list knows, I will be sending a reroll to the existing patch, but the patches I've posted do indeed work in my testing.
> 
> I appreciate the patches.  I blame something here.
> 
> The patches don’t t apply cleanly to git-2.2.1 or to the latest git source I just cloned.  I had to copy/paste some two chunks in http.c and everything in remote-curl.c.

I tried again, with a proper copy of your patch. Cleanly patched versions of git 2.2.1 and from
a recent clone of the repo both give 'Authentication failed’ when doing a clone without a ticket.

I’m sure there’s something here which is at fault.

Here is the client side debugging.  Is this what is expected?  Tested with a clone of git from this morning.

[dan@porter93 ~/tmp/tmp]$ ~/src/git/git clone https://us.example.org/git/clamav-bytecode-compiler
Cloning into 'clamav-bytecode-compiler'...
warning: templates not found /usr/home/dan/share/git-core/templates
* Couldn't find host us.example.org in the .netrc file; using defaults
* Hostname was NOT found in DNS cache
*   Trying 10.0.0.1...
* Connected to us.example.org (10.0.0.1) port 443 (#0)
* successfully set certificate verify locations:
*   CAfile: /usr/local/share/certs/ca-root-nss.crt
  CApath: none
* SSL connection using TLSv1.0 / DHE-RSA-CAMELLIA256-SHA
* Server certificate:
* 	 subject: REDACTED
* 	 start date: REDACTED
* 	 expire date: REDACTED
* 	 issuer: REDACTED
* 	 SSL certificate verify result: self signed certificate in certificate chain (19), continuing anyway.
> GET /git/clamav-bytecode-compiler/info/refs?service=git-upload-pack HTTP/1.1
User-Agent: git/2.2.1.212.gc5b9256.dirty
Host: us.example.org
Accept: */*
Accept-Encoding: gzip
Pragma: no-cache

< HTTP/1.1 401 Authorization Required
< Date: Tue, 06 Jan 2015 16:02:38 GMT
< Server: Apache
< WWW-Authenticate: Negotiate
< Content-Length: 401
< Content-Type: text/html; charset=iso-8859-1
< 
* Connection #0 to host us.example.org left intact
Username for 'https://us.example.org': 
Password for 'https://dan@us.example.org': 
* Couldn't find host us.example.org in the .netrc file; using defaults
* Connection 0 seems to be dead!
* Closing connection 0
* Hostname was found in DNS cache
*   Trying 10.0.0.1...
* Connected to us.example.org (10.0.0.1) port 443 (#1)
* successfully set certificate verify locations:
*   CAfile: /usr/local/share/certs/ca-root-nss.crt
  CApath: none
* SSL re-using session ID
* SSL connection using TLSv1.0 / DHE-RSA-CAMELLIA256-SHA
* Server certificate:
* 	 subject: REDACTED
* 	 start date: REDACTED
* 	 expire date: REDACTED
* 	 issuer: REDACTED
* 	 SSL certificate verify result: self signed certificate in certificate chain (19), continuing anyway.
> GET /git/clamav-bytecode-compiler/info/refs?service=git-upload-pack HTTP/1.1
User-Agent: git/2.2.1.212.gc5b9256.dirty
Host: us.example.org
Accept: */*
Accept-Encoding: gzip
Pragma: no-cache

< HTTP/1.1 401 Authorization Required
< Date: Tue, 06 Jan 2015 16:02:48 GMT
< Server: Apache
< WWW-Authenticate: Negotiate
< Content-Length: 401
< Content-Type: text/html; charset=iso-8859-1
< 
* Connection #1 to host us.example.org left intact
fatal: Authentication failed for 'https://us.example.org/git/clamav-bytecode-compiler/'
[dan@porter93 ~/tmp/tmp]$ 





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-06 15:31       ` Dan Langille (dalangil)
@ 2015-01-06 15:41         ` Dan Langille (dalangil)
  2015-01-06 16:07         ` Dan Langille (dalangil)
  1 sibling, 0 replies; 14+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-06 15:41 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

> On Jan 6, 2015, at 10:31 AM, Dan Langille (dalangil) <dalangil@cisco.com> wrote:
> 
> On Jan 5, 2015, at 6:53 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>> 
>> On Mon, Jan 05, 2015 at 09:23:32PM +0000, Dan Langille (dalangil) wrote:
>>> I have tried both patches.  Neither succeeds here.  I patched git version 2.2.1 but I don’t think that affects this.
>> 
>> You are patching the client side, correct?  That's the side that needs patching here.
> 
> Yes, I am.
> 
>> Just so the list knows, I will be sending a reroll to the existing patch, but the patches I've posted do indeed work in my testing.
> 
> I appreciate the patches.  I blame something here.
> 
> The patches don’t t apply cleanly to git-2.2.1 or to the latest git source I just cloned.  I had to copy/paste some two chunks in http.c and everything in remote-curl.c.

NOTE: I now realize these problems are caused by tabs being converted to spaces when downloading the patch.  I’m going to test again.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-05 23:53     ` brian m. carlson
@ 2015-01-06 15:31       ` Dan Langille (dalangil)
  2015-01-06 15:41         ` Dan Langille (dalangil)
  2015-01-06 16:07         ` Dan Langille (dalangil)
  0 siblings, 2 replies; 14+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-06 15:31 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

On Jan 5, 2015, at 6:53 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On Mon, Jan 05, 2015 at 09:23:32PM +0000, Dan Langille (dalangil) wrote:
>> I have tried both patches.  Neither succeeds here.  I patched git version 2.2.1 but I don’t think that affects this.
> 
> You are patching the client side, correct?  That's the side that needs patching here.

Yes, I am.

> Just so the list knows, I will be sending a reroll to the existing patch, but the patches I've posted do indeed work in my testing.

I appreciate the patches.  I blame something here.

The patches don’t t apply cleanly to git-2.2.1 or to the latest git source I just cloned.  I had to copy/paste some two chunks in http.c and everything in remote-curl.c.

Looking at the source, the place to patch is there, just on a different line number.

[dvl@porter93 /usr/ports/devel/git/work/git-2.2.1]$ sudo patch < ~dvl/patch2.txt
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/http.c b/http.c
|index 040f362..815194d 100644
|--- a/http.c
|+++ b/http.c
--------------------------
Patching file http.c using Plan A...
Hunk #1 succeeded at 62.
Hunk #2 succeeded at 988 with fuzz 2.
Hunk #3 failed at 1047.
Hunk #4 failed at 1154.
2 out of 4 hunks failed--saving rejects to http.c.rej
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/http.h b/http.h
|index 473179b..71943d3 100644
|--- a/http.h
|+++ b/http.h
--------------------------
Patching file http.h using Plan A...
Hunk #1 succeeded at 98 with fuzz 2.
Hunk #2 succeeded at 114.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/remote-curl.c b/remote-curl.c
|index dd63bc2..4ca5447 100644
|--- a/remote-curl.c
|+++ b/remote-curl.c
--------------------------
Patching file remote-curl.c using Plan A...
Hunk #1 failed at 467.
Hunk #2 failed at 513.
Hunk #3 failed at 538.
Hunk #4 failed at 625.
4 out of 4 hunks failed--saving rejects to remote-curl.c.rej
done


>> Before I flood the list with debug runs, I wanted to make sure I was testing with an appropriate configuration:
>> 
>> <Location /git>
>> SSLOptions +StdenvVars
>> Options +ExecCGI +FollowSymLinks +SymLinksIfOwnerMatch
>> 
>>  # By default, allow access to anyone.
>>  Order allow,deny
>>  Allow from All
>> 
>>  # Enable Kerberos authentication using mod_auth_kerb.
>> AuthType           Kerberos
>> AuthName           “us.example.org"
>> KrbAuthRealms      us.example.org
>> # I have tried both with and without the following line:
>> KrbServiceName     HTTP/us.example.org
>> Krb5Keytab         /usr/local/etc/apache22/repo-test.keytab
>>  KrbMethodNegotiate on
>>  KrbSaveCredentials on
>>  KrbVerifyKDC on
>>  KrbServiceName Any
>> # I have tried with and without this line:
>> KrbMethodk5Passwd  on
>>  Require valid-user
>> </Location>
> 
> I'm not sure why it's not working for you.  Here's a snippet from my config:
> 
> SetEnv GIT_HTTP_EXPORT_ALL 1
> SetEnv REMOTE_USER $REDIRECT_REMOTE_USER
> AuthType Kerberos
> AuthName "Kerberos Login"
> KrbMethodNegotiate on
> KrbMethodK5Passwd off
> KrbAuthRealms CRUSTYTOOTHPASTE.NET
> Krb5Keytab /etc/krb5.apache.keytab
> 
> When I was testing, I set KrbMethodK5Passwd to on and it did in fact work.  I'm using Debian's Apache 2.4.10-9 with mod_auth_kerb 5.4-2.2.

That didn’t seem to help, but perhaps the patching is the issue.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-05 21:23   ` Dan Langille (dalangil)
@ 2015-01-05 23:53     ` brian m. carlson
  2015-01-06 15:31       ` Dan Langille (dalangil)
  0 siblings, 1 reply; 14+ messages in thread
From: brian m. carlson @ 2015-01-05 23:53 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

On Mon, Jan 05, 2015 at 09:23:32PM +0000, Dan Langille (dalangil) wrote:
>I have tried both patches.  Neither succeeds here.  I patched git 
>version 2.2.1 but I don’t think that affects this.

You are patching the client side, correct?  That's the side that needs 
patching here.

Just so the list knows, I will be sending a reroll to the existing 
patch, but the patches I've posted do indeed work in my testing.

>Before I flood the list with debug runs, I wanted to make sure I was 
>testing with an appropriate configuration:
>
><Location /git>
> SSLOptions +StdenvVars
> Options +ExecCGI +FollowSymLinks +SymLinksIfOwnerMatch
>
>   # By default, allow access to anyone.
>   Order allow,deny
>   Allow from All
>
>   # Enable Kerberos authentication using mod_auth_kerb.
>  AuthType           Kerberos
>  AuthName           “us.example.org"
>  KrbAuthRealms      us.example.org
>  # I have tried both with and without the following line:
>  KrbServiceName     HTTP/us.example.org
>  Krb5Keytab         /usr/local/etc/apache22/repo-test.keytab
>   KrbMethodNegotiate on
>   KrbSaveCredentials on
>   KrbVerifyKDC on
>   KrbServiceName Any
>  # I have tried with and without this line:
>  KrbMethodk5Passwd  on
>   Require valid-user
></Location>

I'm not sure why it's not working for you.  Here's a snippet from my 
config:

  SetEnv GIT_HTTP_EXPORT_ALL 1
  SetEnv REMOTE_USER $REDIRECT_REMOTE_USER
  AuthType Kerberos
  AuthName "Kerberos Login"
  KrbMethodNegotiate on
  KrbMethodK5Passwd off
  KrbAuthRealms CRUSTYTOOTHPASTE.NET
  Krb5Keytab /etc/krb5.apache.keytab

When I was testing, I set KrbMethodK5Passwd to on and it did in fact 
work.  I'm using Debian's Apache 2.4.10-9 with mod_auth_kerb 5.4-2.2.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-01 19:56 ` [PATCH v2] " brian m. carlson
  2015-01-03 11:19   ` Jeff King
  2015-01-05 16:02   ` Dan Langille (dalangil)
@ 2015-01-05 21:23   ` Dan Langille (dalangil)
  2015-01-05 23:53     ` brian m. carlson
  2 siblings, 1 reply; 14+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-05 21:23 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

I have tried both patches.  Neither succeeds here.  I patched git version 2.2.1 but I don’t think that affects this.

Before I flood the list with debug runs, I wanted to make sure I was testing with an appropriate configuration:

<Location /git>
 SSLOptions +StdenvVars
 Options +ExecCGI +FollowSymLinks +SymLinksIfOwnerMatch

   # By default, allow access to anyone.
   Order allow,deny
   Allow from All

   # Enable Kerberos authentication using mod_auth_kerb.
  AuthType           Kerberos
  AuthName           “us.example.org"
  KrbAuthRealms      us.example.org
  # I have tried both with and without the following line:
  KrbServiceName     HTTP/us.example.org
  Krb5Keytab         /usr/local/etc/apache22/repo-test.keytab
   KrbMethodNegotiate on
   KrbSaveCredentials on
   KrbVerifyKDC on
   KrbServiceName Any
  # I have tried with and without this line:
  KrbMethodk5Passwd  on
   Require valid-user
</Location>

With a valid ticket, the above works for a git clone.

— 
Dan Langille
Infrastructure & Operations
Talos Group
Sourcefire, Inc.

> On Jan 1, 2015, at 2:56 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> Apache servers using mod_auth_kerb can be configured to allow the user
> to authenticate either using Negotiate (using the Kerberos ticket) or
> Basic authentication (using the Kerberos password).  Often, one will
> want to use Negotiate authentication if it is available, but fall back
> to Basic authentication if the ticket is missing or expired.
> 
> However, libcurl will try very hard to use something other than Basic
> auth, even over HTTPS.  If Basic and something else are offered, libcurl
> will never attempt to use Basic, even if the other option fails.
> Teach the HTTP client code to stop trying authentication mechanisms that
> don't use a password (currently Negotiate) after the first failure,
> since if they failed the first time, they will never succeed.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> http.c        | 16 ++++++++++++++++
> http.h        |  3 +++
> remote-curl.c | 11 ++++++++++-
> 3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/http.c b/http.c
> index 040f362..815194d 100644
> --- a/http.c
> +++ b/http.c
> @@ -62,6 +62,8 @@ static const char *user_agent;
> 
> static struct credential cert_auth = CREDENTIAL_INIT;
> static int ssl_cert_password_required;
> +/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
> +int http_passwordless_auth = 1;
> 
> static struct curl_slist *pragma_header;
> static struct curl_slist *no_pragma_header;
> @@ -986,6 +988,16 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
> 		strbuf_addstr(charset, "ISO-8859-1");
> }
> 
> +void disable_passwordless_auth(struct active_request_slot *slot)
> +{
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH,
> +			 CURLAUTH_ANY & ~HTTP_AUTH_PASSWORDLESS);
> +#endif
> +}
> +
> +
> /* http_request() targets */
> #define HTTP_REQUEST_STRBUF	0
> #define HTTP_REQUEST_FILE	1
> @@ -1035,6 +1047,9 @@ static int http_request(const char *url,
> 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	ret = run_one_slot(slot, &results);
> 
> 	if (options && options->content_type) {
> @@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
> 	}
> 
> 	credential_fill(&http_auth);
> +	http_passwordless_auth = 0;
> 
> 	return http_request(url, result, target, options);
> }
> diff --git a/http.h b/http.h
> index 473179b..71943d3 100644
> --- a/http.h
> +++ b/http.h
> @@ -98,6 +98,8 @@ extern int handle_curl_result(struct slot_results *results);
> int run_one_slot(struct active_request_slot *slot,
> 		 struct slot_results *results);
> 
> +void disable_passwordless_auth(struct active_request_slot *slot);
> +
> #ifdef USE_CURL_MULTI
> extern void fill_active_slots(void);
> extern void add_fill_function(void *data, int (*fill)(void *));
> @@ -112,6 +114,7 @@ extern int active_requests;
> extern int http_is_verbose;
> extern size_t http_post_buffer;
> extern struct credential http_auth;
> +extern int http_passwordless_auth;
> 
> extern char curl_errorstr[CURL_ERROR_SIZE];
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index dd63bc2..4ca5447 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -467,6 +467,9 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
> 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	err = run_slot(slot, results);
> 
> 	curl_slist_free_all(headers);
> @@ -510,8 +513,10 @@ static int post_rpc(struct rpc_state *rpc)
> 
> 		do {
> 			err = probe_rpc(rpc, &results);
> -			if (err == HTTP_REAUTH)
> +			if (err == HTTP_REAUTH) {
> 				credential_fill(&http_auth);
> +				http_passwordless_auth = 0;
> +			}
> 		} while (err == HTTP_REAUTH);
> 		if (err != HTTP_OK)
> 			return -1;
> @@ -533,6 +538,9 @@ retry:
> 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	if (large_request) {
> 		/* The request body is large and the size cannot be predicted.
> 		 * We must use chunked encoding to send it.
> @@ -617,6 +625,7 @@ retry:
> 	err = run_slot(slot, NULL);
> 	if (err == HTTP_REAUTH && !large_request) {
> 		credential_fill(&http_auth);
> +		http_passwordless_auth = 0;
> 		goto retry;
> 	}
> 	if (err != HTTP_OK)
> -- 
> 2.2.1.209.g41e5f3a
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-01 19:56 ` [PATCH v2] " brian m. carlson
  2015-01-03 11:19   ` Jeff King
@ 2015-01-05 16:02   ` Dan Langille (dalangil)
  2015-01-05 21:23   ` Dan Langille (dalangil)
  2 siblings, 0 replies; 14+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-05 16:02 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano

I’ve found the latest patch.  Trying this now.  Thanks.
— 
Dan Langille
Infrastructure & Operations
Talos Group
Sourcefire, Inc.

> On Jan 1, 2015, at 2:56 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> Apache servers using mod_auth_kerb can be configured to allow the user
> to authenticate either using Negotiate (using the Kerberos ticket) or
> Basic authentication (using the Kerberos password).  Often, one will
> want to use Negotiate authentication if it is available, but fall back
> to Basic authentication if the ticket is missing or expired.
> 
> However, libcurl will try very hard to use something other than Basic
> auth, even over HTTPS.  If Basic and something else are offered, libcurl
> will never attempt to use Basic, even if the other option fails.
> Teach the HTTP client code to stop trying authentication mechanisms that
> don't use a password (currently Negotiate) after the first failure,
> since if they failed the first time, they will never succeed.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> http.c        | 16 ++++++++++++++++
> http.h        |  3 +++
> remote-curl.c | 11 ++++++++++-
> 3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/http.c b/http.c
> index 040f362..815194d 100644
> --- a/http.c
> +++ b/http.c
> @@ -62,6 +62,8 @@ static const char *user_agent;
> 
> static struct credential cert_auth = CREDENTIAL_INIT;
> static int ssl_cert_password_required;
> +/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
> +int http_passwordless_auth = 1;
> 
> static struct curl_slist *pragma_header;
> static struct curl_slist *no_pragma_header;
> @@ -986,6 +988,16 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
> 		strbuf_addstr(charset, "ISO-8859-1");
> }
> 
> +void disable_passwordless_auth(struct active_request_slot *slot)
> +{
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH,
> +			 CURLAUTH_ANY & ~HTTP_AUTH_PASSWORDLESS);
> +#endif
> +}
> +
> +
> /* http_request() targets */
> #define HTTP_REQUEST_STRBUF	0
> #define HTTP_REQUEST_FILE	1
> @@ -1035,6 +1047,9 @@ static int http_request(const char *url,
> 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	ret = run_one_slot(slot, &results);
> 
> 	if (options && options->content_type) {
> @@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
> 	}
> 
> 	credential_fill(&http_auth);
> +	http_passwordless_auth = 0;
> 
> 	return http_request(url, result, target, options);
> }
> diff --git a/http.h b/http.h
> index 473179b..71943d3 100644
> --- a/http.h
> +++ b/http.h
> @@ -98,6 +98,8 @@ extern int handle_curl_result(struct slot_results *results);
> int run_one_slot(struct active_request_slot *slot,
> 		 struct slot_results *results);
> 
> +void disable_passwordless_auth(struct active_request_slot *slot);
> +
> #ifdef USE_CURL_MULTI
> extern void fill_active_slots(void);
> extern void add_fill_function(void *data, int (*fill)(void *));
> @@ -112,6 +114,7 @@ extern int active_requests;
> extern int http_is_verbose;
> extern size_t http_post_buffer;
> extern struct credential http_auth;
> +extern int http_passwordless_auth;
> 
> extern char curl_errorstr[CURL_ERROR_SIZE];
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index dd63bc2..4ca5447 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -467,6 +467,9 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
> 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	err = run_slot(slot, results);
> 
> 	curl_slist_free_all(headers);
> @@ -510,8 +513,10 @@ static int post_rpc(struct rpc_state *rpc)
> 
> 		do {
> 			err = probe_rpc(rpc, &results);
> -			if (err == HTTP_REAUTH)
> +			if (err == HTTP_REAUTH) {
> 				credential_fill(&http_auth);
> +				http_passwordless_auth = 0;
> +			}
> 		} while (err == HTTP_REAUTH);
> 		if (err != HTTP_OK)
> 			return -1;
> @@ -533,6 +538,9 @@ retry:
> 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> 
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
> 	if (large_request) {
> 		/* The request body is large and the size cannot be predicted.
> 		 * We must use chunked encoding to send it.
> @@ -617,6 +625,7 @@ retry:
> 	err = run_slot(slot, NULL);
> 	if (err == HTTP_REAUTH && !large_request) {
> 		credential_fill(&http_auth);
> +		http_passwordless_auth = 0;
> 		goto retry;
> 	}
> 	if (err != HTTP_OK)
> -- 
> 2.2.1.209.g41e5f3a
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-03 17:45     ` brian m. carlson
@ 2015-01-03 20:14       ` Jeff King
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff King @ 2015-01-03 20:14 UTC (permalink / raw)
  To: git, Dan Langille (dalangil), Jonathan Nieder, Junio C Hamano

On Sat, Jan 03, 2015 at 05:45:09PM +0000, brian m. carlson wrote:

> >+	{
> >+		int flags = CURLAUTH_ANY;
> 
> I think this needs to be unsigned long or it can cause undefined behavior,
> since libcurl uses unsigned long in the flags.  I'll fix that up when I
> reroll.  I'll need your sign-off since it will essentially be your work.

I think curl typically uses signed "long" for flags, but certainly
check the docs to be sure.

I was thinking it would be integer-promoted in this case, but I'm not
sure that works always (certainly it does not if CURLAUTH_ANY needs high
bits, but depending on how curl_easy_setopt is implemented, it may also
be implicitly cast as a pointer or something).

And certainly you can have my signoff:

  Signed-off-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-03 11:19   ` Jeff King
@ 2015-01-03 17:45     ` brian m. carlson
  2015-01-03 20:14       ` Jeff King
  0 siblings, 1 reply; 14+ messages in thread
From: brian m. carlson @ 2015-01-03 17:45 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Dan Langille (dalangil), Jonathan Nieder, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]

On Sat, Jan 03, 2015 at 06:19:23AM -0500, Jeff King wrote:
>This pattern gets repeated in several places. Now that
>http_passwordless_auth is a global, can we handle it automatically for
>the callers, as below (which, aside from compiling, is completely
>untested by me)?

This looks good (although I haven't tested it).

>Note that this is in a slightly different boat than credential_fill.
>Ideally we would also handle picking up credentials on behalf of the
>callers of get_curl_handle/handle_curl_result. But that may involve
>significant work and/or prompting the user, which we _must_ avoid if we
>do not know if we are going to retry the request (and only the caller
>knows that for sure). However, in the case of http_passwordless_auth, we
>are just setting a flag, so it's OK to do it preemptively.

Right.  We already prompt the user for a username and password in that 
case, so we already have the credentials that we need.

>diff --git a/http.c b/http.c
>index 040f362..2bbcdf1 100644
>--- a/http.c
>+++ b/http.c
>@@ -62,6 +62,8 @@ static const char *user_agent;
>
> static struct credential cert_auth = CREDENTIAL_INIT;
> static int ssl_cert_password_required;
>+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
>+static int http_passwordless_auth = 1;
>
> static struct curl_slist *pragma_header;
> static struct curl_slist *no_pragma_header;
>@@ -318,7 +320,12 @@ static CURL *get_curl_handle(void)
> 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
> #endif
> #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
>-	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>+	{
>+		int flags = CURLAUTH_ANY;

I think this needs to be unsigned long or it can cause undefined 
behavior, since libcurl uses unsigned long in the flags.  I'll fix that 
up when I reroll.  I'll need your sign-off since it will essentially be 
your work.

>+		if (!http_passwordless_auth)
>+			flags &= ~CURLAUTH_GSSNEGOTIATE;
>+		curl_easy_setopt(result, CURLOPT_HTTPAUTH, flags);
>+	}
> #endif
>
> 	if (http_proactive_auth)
>@@ -870,6 +877,7 @@ int handle_curl_result(struct slot_results *results)
> 			credential_reject(&http_auth);
> 			return HTTP_NOAUTH;
> 		} else {
>+			http_passwordless_auth = 0;
> 			return HTTP_REAUTH;
> 		}
> 	} else {
>
>
>Note that you could probably drop http_passwordless_auth completely, and
>just keep a:
>
>  static int http_auth_methods = CURLAUTH_ANY;
>
>and then drop CURLAUTH_GSSNEGOTIATE from it instead of setting the
>passwordless_auth flag to 0 (again, it happens in one place, so I don't
>know that it needs an extra layer of abstraction).

This does seem to handle both cases well.  It also has the pleasant 
side effect of being static.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-01 19:56 ` [PATCH v2] " brian m. carlson
@ 2015-01-03 11:19   ` Jeff King
  2015-01-03 17:45     ` brian m. carlson
  2015-01-05 16:02   ` Dan Langille (dalangil)
  2015-01-05 21:23   ` Dan Langille (dalangil)
  2 siblings, 1 reply; 14+ messages in thread
From: Jeff King @ 2015-01-03 11:19 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Dan Langille (dalangil), Jonathan Nieder, Junio C Hamano

On Thu, Jan 01, 2015 at 07:56:27PM +0000, brian m. carlson wrote:

> +void disable_passwordless_auth(struct active_request_slot *slot)
> +{
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH,
> +			 CURLAUTH_ANY & ~HTTP_AUTH_PASSWORDLESS);
> +#endif
> +}

I like that you are trying to put a layer of abstraction around what
"passwordless" means here, but it seems like there are two layers. The
function itself abstracts the idea, and then there is an extra
HTTP_AUTH_PASSWORDLESS macro. Since the concept is already confined to
this function and used only once, it might be more readable to simply
get rid of HTTP_AUTH_PASSWORD.

> @@ -1035,6 +1047,9 @@ static int http_request(const char *url,
>  	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
>  	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
>  
> +	if (!http_passwordless_auth)
> +		disable_passwordless_auth(slot);
> +
>  	ret = run_one_slot(slot, &results);
>  
>  	if (options && options->content_type) {
> @@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
>  	}
>  
>  	credential_fill(&http_auth);
> +	http_passwordless_auth = 0;
>  
>  	return http_request(url, result, target, options);
>  }

This pattern gets repeated in several places. Now that
http_passwordless_auth is a global, can we handle it automatically for
the callers, as below (which, aside from compiling, is completely
untested by me)?

Note that this is in a slightly different boat than credential_fill.
Ideally we would also handle picking up credentials on behalf of the
callers of get_curl_handle/handle_curl_result. But that may involve
significant work and/or prompting the user, which we _must_ avoid if we
do not know if we are going to retry the request (and only the caller
knows that for sure). However, in the case of http_passwordless_auth, we
are just setting a flag, so it's OK to do it preemptively.

diff --git a/http.c b/http.c
index 040f362..2bbcdf1 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,8 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
+static int http_passwordless_auth = 1;
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -318,7 +320,12 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+	{
+		int flags = CURLAUTH_ANY;
+		if (!http_passwordless_auth)
+			flags &= ~CURLAUTH_GSSNEGOTIATE;
+		curl_easy_setopt(result, CURLOPT_HTTPAUTH, flags);
+	}
 #endif
 
 	if (http_proactive_auth)
@@ -870,6 +877,7 @@ int handle_curl_result(struct slot_results *results)
 			credential_reject(&http_auth);
 			return HTTP_NOAUTH;
 		} else {
+			http_passwordless_auth = 0;
 			return HTTP_REAUTH;
 		}
 	} else {


Note that you could probably drop http_passwordless_auth completely, and
just keep a:

  static int http_auth_methods = CURLAUTH_ANY;

and then drop CURLAUTH_GSSNEGOTIATE from it instead of setting the
passwordless_auth flag to 0 (again, it happens in one place, so I don't
know that it needs an extra layer of abstraction).

-Peff

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
  2014-12-27  4:01 [PATCH] remote-curl: fall back to Basic " brian m. carlson
@ 2015-01-01 19:56 ` brian m. carlson
  2015-01-03 11:19   ` Jeff King
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: brian m. carlson @ 2015-01-01 19:56 UTC (permalink / raw)
  To: git; +Cc: Dan Langille (dalangil), Jeff King, Jonathan Nieder, Junio C Hamano

Apache servers using mod_auth_kerb can be configured to allow the user
to authenticate either using Negotiate (using the Kerberos ticket) or
Basic authentication (using the Kerberos password).  Often, one will
want to use Negotiate authentication if it is available, but fall back
to Basic authentication if the ticket is missing or expired.

However, libcurl will try very hard to use something other than Basic
auth, even over HTTPS.  If Basic and something else are offered, libcurl
will never attempt to use Basic, even if the other option fails.
Teach the HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first failure,
since if they failed the first time, they will never succeed.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 http.c        | 16 ++++++++++++++++
 http.h        |  3 +++
 remote-curl.c | 11 ++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 040f362..815194d 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,8 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
+int http_passwordless_auth = 1;
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -986,6 +988,16 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
 		strbuf_addstr(charset, "ISO-8859-1");
 }
 
+void disable_passwordless_auth(struct active_request_slot *slot)
+{
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH,
+			 CURLAUTH_ANY & ~HTTP_AUTH_PASSWORDLESS);
+#endif
+}
+
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF	0
 #define HTTP_REQUEST_FILE	1
@@ -1035,6 +1047,9 @@ static int http_request(const char *url,
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
 
+	if (!http_passwordless_auth)
+		disable_passwordless_auth(slot);
+
 	ret = run_one_slot(slot, &results);
 
 	if (options && options->content_type) {
@@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
 	}
 
 	credential_fill(&http_auth);
+	http_passwordless_auth = 0;
 
 	return http_request(url, result, target, options);
 }
diff --git a/http.h b/http.h
index 473179b..71943d3 100644
--- a/http.h
+++ b/http.h
@@ -98,6 +98,8 @@ extern int handle_curl_result(struct slot_results *results);
 int run_one_slot(struct active_request_slot *slot,
 		 struct slot_results *results);
 
+void disable_passwordless_auth(struct active_request_slot *slot);
+
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);
 extern void add_fill_function(void *data, int (*fill)(void *));
@@ -112,6 +114,7 @@ extern int active_requests;
 extern int http_is_verbose;
 extern size_t http_post_buffer;
 extern struct credential http_auth;
+extern int http_passwordless_auth;
 
 extern char curl_errorstr[CURL_ERROR_SIZE];
 
diff --git a/remote-curl.c b/remote-curl.c
index dd63bc2..4ca5447 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -467,6 +467,9 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);
 
+	if (!http_passwordless_auth)
+		disable_passwordless_auth(slot);
+
 	err = run_slot(slot, results);
 
 	curl_slist_free_all(headers);
@@ -510,8 +513,10 @@ static int post_rpc(struct rpc_state *rpc)
 
 		do {
 			err = probe_rpc(rpc, &results);
-			if (err == HTTP_REAUTH)
+			if (err == HTTP_REAUTH) {
 				credential_fill(&http_auth);
+				http_passwordless_auth = 0;
+			}
 		} while (err == HTTP_REAUTH);
 		if (err != HTTP_OK)
 			return -1;
@@ -533,6 +538,9 @@ retry:
 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
 
+	if (!http_passwordless_auth)
+		disable_passwordless_auth(slot);
+
 	if (large_request) {
 		/* The request body is large and the size cannot be predicted.
 		 * We must use chunked encoding to send it.
@@ -617,6 +625,7 @@ retry:
 	err = run_slot(slot, NULL);
 	if (err == HTTP_REAUTH && !large_request) {
 		credential_fill(&http_auth);
+		http_passwordless_auth = 0;
 		goto retry;
 	}
 	if (err != HTTP_OK)
-- 
2.2.1.209.g41e5f3a

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-03-22 16:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <pull.849.git.1611921008282.gitgitgadget@gmail.com>
2021-02-16 16:57 ` [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails Christopher via GitGitGadget
2021-03-22 11:51   ` [PATCH v3] " Christopher via GitGitGadget
     [not found]   ` <xmqq35xvpr8q.fsf@gitster.c.googlers.com>
2021-03-22 16:08     ` [PATCH v2] " Christopher Schenk
2014-12-27  4:01 [PATCH] remote-curl: fall back to Basic " brian m. carlson
2015-01-01 19:56 ` [PATCH v2] " brian m. carlson
2015-01-03 11:19   ` Jeff King
2015-01-03 17:45     ` brian m. carlson
2015-01-03 20:14       ` Jeff King
2015-01-05 16:02   ` Dan Langille (dalangil)
2015-01-05 21:23   ` Dan Langille (dalangil)
2015-01-05 23:53     ` brian m. carlson
2015-01-06 15:31       ` Dan Langille (dalangil)
2015-01-06 15:41         ` Dan Langille (dalangil)
2015-01-06 16:07         ` Dan Langille (dalangil)
2015-01-08  0:02           ` brian m. carlson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.