git.vger.kernel.org archive mirror
 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; 18+ 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] 18+ 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; 18+ 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] 18+ 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; 18+ 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] 18+ messages in thread

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-03-11 21:59                         ` brian m. carlson
@ 2015-03-12 13:09                           ` Dan Langille (dalangil)
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-03-12 13:09 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Junio C Hamano, git, Jeff King

> On Mar 11, 2015, at 5:59 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On Wed, Mar 11, 2015 at 07:33:05PM +0000, Dan Langille (dalangil) wrote:
>>> On Mar 10, 2015, at 6:29 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>>> Does it work with a ticket if you specify a username, as in the
>>> following URL?
>>> https://bmc@git.crustytoothpaste.net/git/bmc/homedir.git
>> 
>> Yes, that does work.  Our project is 98% of the way there now.
>> 
>> I looked at both libcurl and git environment variables to see if there
>> was a way to specify the user without putting it in the URL.  I didn’t see one.
>> 
>> My next step is the git configuration, either server or client.  Do you know
>> if I should stop looking now because it’s not there?
> 
> You might try looking at git config --help.  It looks like there's a
> credential.username option that might do what you want.

Brian et al,

Thank you for your help and patience with this.  It is appreciated.

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




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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-03-11 19:33                       ` Dan Langille (dalangil)
@ 2015-03-11 21:59                         ` brian m. carlson
  2015-03-12 13:09                           ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: brian m. carlson @ 2015-03-11 21:59 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: Junio C Hamano, git, Jeff King

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

On Wed, Mar 11, 2015 at 07:33:05PM +0000, Dan Langille (dalangil) wrote:
>> On Mar 10, 2015, at 6:29 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>> Does it work with a ticket if you specify a username, as in the
>> following URL?
>> https://bmc@git.crustytoothpaste.net/git/bmc/homedir.git
>
>Yes, that does work.  Our project is 98% of the way there now.
>
>I looked at both libcurl and git environment variables to see if there
>was a way to specify the user without putting it in the URL.  I didn’t see one.
>
>My next step is the git configuration, either server or client.  Do you know
>if I should stop looking now because it’s not there?

You might try looking at git config --help.  It looks like there's a
credential.username option that might do what you want.
-- 
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] 18+ messages in thread

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-03-10 22:29                     ` brian m. carlson
@ 2015-03-11 19:33                       ` Dan Langille (dalangil)
  2015-03-11 21:59                         ` brian m. carlson
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-03-11 19:33 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Junio C Hamano, git, Jeff King

> On Mar 10, 2015, at 6:29 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On Tue, Mar 10, 2015 at 06:05:46PM +0000, Dan Langille (dalangil) wrote:
>>> We have made progress I think.
>>> 
>>> With stock git:
>>> 
>>> tl;dr: 1 - with a ticket, you get prompted, but hitting ENTER succeeds.
>>>      2 - without a ticket, nothing works
>>> 
>>> 
>>> With patched git:
>>> 
>>> tl;dr: 1 - with a ticket,    entering credentials, SUCCEEDS; just hit enter, failure
>> 
>> If I have a valid ticket, why am I being prompted for credentials?
> 
> libcurl won't even attempt authentication if you don't have a username
> specified.  I know that the web server should be able to figure it out
> from your credentials, so it shouldn't matter what username you provide.
> This is an unfortuate quirk of lib curl.

I understand.

> Also, are you using 2.3.0, or one of the earlier patched versions?  That
> might affect how it works.

I am using git-2.3.0

>> It appears patched git always wants credentials entered and ignores the
>> valid ticket.
> 
> So what I think is happening is that you didn't specify a username, but
> git got a 401, so it prompted.  Now it actually attempts to use the
> password you provided, whereas before it did not.
> 
> Does it work with a ticket if you specify a username, as in the
> following URL?
> https://bmc@git.crustytoothpaste.net/git/bmc/homedir.git

Yes, that does work.  Our project is 98% of the way there now. 

I looked at both libcurl and git environment variables to see if there
was a way to specify the user without putting it in the URL.  I didn’t see one.

My next step is the git configuration, either server or client.  Do you know 
if I should stop looking now because it’s not there?

Thank you for your help in getting us this far.  This helps us tremendously.

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

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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-03-10 18:05                   ` Dan Langille (dalangil)
@ 2015-03-10 22:29                     ` brian m. carlson
  2015-03-11 19:33                       ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: brian m. carlson @ 2015-03-10 22:29 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: Junio C Hamano, git, Jeff King

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

On Tue, Mar 10, 2015 at 06:05:46PM +0000, Dan Langille (dalangil) wrote:
>> We have made progress I think.
>>
>> With stock git:
>>
>> tl;dr: 1 - with a ticket, you get prompted, but hitting ENTER succeeds.
>>       2 - without a ticket, nothing works
>>
>>
>> With patched git:
>>
>> tl;dr: 1 - with a ticket,    entering credentials, SUCCEEDS; just hit enter, failure
>
>If I have a valid ticket, why am I being prompted for credentials?

libcurl won't even attempt authentication if you don't have a username
specified.  I know that the web server should be able to figure it out
from your credentials, so it shouldn't matter what username you provide.
This is an unfortuate quirk of libcurl.

Also, are you using 2.3.0, or one of the earlier patched versions?  That
might affect how it works.

>It appears patched git always wants credentials entered and ignores the
>valid ticket.

So what I think is happening is that you didn't specify a username, but
git got a 401, so it prompted.  Now it actually attempts to use the
password you provided, whereas before it did not.

Does it work with a ticket if you specify a username, as in the
following URL?
https://bmc@git.crustytoothpaste.net/git/bmc/homedir.git
-- 
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] 18+ messages in thread

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-02-25 20:59                 ` Dan Langille (dalangil)
@ 2015-03-10 18:05                   ` Dan Langille (dalangil)
  2015-03-10 22:29                     ` brian m. carlson
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-03-10 18:05 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Junio C Hamano, git, Jeff King

> On Feb 25, 2015, at 3:59 PM, Dan Langille (dalangil) <dalangil@cisco.com> wrote:
> 
>> On Feb 24, 2015, at 4:03 PM, Dan Langille (dalangil) <dalangil@cisco.com> wrote:
>> 
>>> On Feb 19, 2015, at 3:35 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>>> 
>>> On Wed, Feb 18, 2015 at 04:17:46PM +0000, Dan Langille (dalangil) wrote:
>>>> I just built from ‘master’, on FreeBSD 9.3:
>>>> 
>>>> cd ~/src
>>>> git clone https://github.com/git/git.git
>>>> cd git
>>>> gmake
>>>> 
>>>> Then tried ~/src/git/git clone https://OUR_REPO
>>>> 
>>>> It cores too, and I see: git-remote-https.core
>>> 
>>> Can you compile with debugging symbols and provide a backtrace?  I'm not 
>>> seeing any such behavior on my end, and I'm not sure whether it's my 
>>> patch or something else that might be present in master.
>> 
>> The problem originally occurred under VMware Fusion and I’m unable to get a backtrace from it.
>> I suspect memory constraints are a factor.  There’s only 5GB RAM available to this VM.
>> 
>> I have tried in another VM and that succeeds.  All good there. It has 40GB RAM.
>> 
>> I am going to try this on a third system. At present, we’re just 50/50 on success.
> 
> 
> We have made progress I think.
> 
> With stock git:
> 
> tl;dr: 1 - with a ticket, you get prompted, but hitting ENTER succeeds.
>       2 - without a ticket, nothing works
> 
> 
> With patched git:
> 
> tl;dr: 1 - with a ticket,    entering credentials, SUCCEEDS; just hit enter, failure

If I have a valid ticket, why am I being prompted for credentials?

It appears patched git always wants credentials entered and ignores the valid ticket.

>       2 - without a ticket, entering credentials, SUCCEEDS
> 
> Here is my test, with a valid kerberos ticket:
> 
> $ git clone https://git.example.com/git/clamav-bytecode-compiler
> Cloning into 'clamav-bytecode-compiler'...
> Username for 'https://git.example.com': 
> Password for 'https://git.example.com': 
> ^Cmote: Counting objects: 224546   
> $
> 


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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-02-24 21:03               ` Dan Langille (dalangil)
@ 2015-02-25 20:59                 ` Dan Langille (dalangil)
  2015-03-10 18:05                   ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-02-25 20:59 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Junio C Hamano, git, Jeff King

> On Feb 24, 2015, at 4:03 PM, Dan Langille (dalangil) <dalangil@cisco.com> wrote:
> 
>> On Feb 19, 2015, at 3:35 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>> 
>> On Wed, Feb 18, 2015 at 04:17:46PM +0000, Dan Langille (dalangil) wrote:
>>> I just built from ‘master’, on FreeBSD 9.3:
>>> 
>>> cd ~/src
>>> git clone https://github.com/git/git.git
>>> cd git
>>> gmake
>>> 
>>> Then tried ~/src/git/git clone https://OUR_REPO
>>> 
>>> It cores too, and I see: git-remote-https.core
>> 
>> Can you compile with debugging symbols and provide a backtrace?  I'm not 
>> seeing any such behavior on my end, and I'm not sure whether it's my 
>> patch or something else that might be present in master.
> 
> The problem originally occurred under VMware Fusion and I’m unable to get a backtrace from it.
> I suspect memory constraints are a factor.  There’s only 5GB RAM available to this VM.
> 
> I have tried in another VM and that succeeds.  All good there. It has 40GB RAM.
> 
> I am going to try this on a third system. At present, we’re just 50/50 on success.


We have made progress I think.

With stock git:

tl;dr: 1 - with a ticket, you get prompted, but hitting ENTER succeeds.
       2 - without a ticket, nothing works


With patched git:

tl;dr: 1 - with a ticket,    entering credentials, SUCCEEDS; just hit enter, failure
       2 - without a ticket, entering credentials, SUCCEEDS

Here is my test, with a valid kerberos ticket:

$ git clone https://git.example.com/git/clamav-bytecode-compiler
Cloning into 'clamav-bytecode-compiler'...
Username for 'https://git.example.com': 
Password for 'https://git.example.com': 
^Cmote: Counting objects: 224546   
$


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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-02-19 20:35             ` brian m. carlson
@ 2015-02-24 21:03               ` Dan Langille (dalangil)
  2015-02-25 20:59                 ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-02-24 21:03 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Junio C Hamano, git, Jeff King

> On Feb 19, 2015, at 3:35 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On Wed, Feb 18, 2015 at 04:17:46PM +0000, Dan Langille (dalangil) wrote:
>> I just built from ‘master’, on FreeBSD 9.3:
>> 
>> cd ~/src
>> git clone https://github.com/git/git.git
>> cd git
>> gmake
>> 
>> Then tried ~/src/git/git clone https://OUR_REPO
>> 
>> It cores too, and I see: git-remote-https.core
> 
> Can you compile with debugging symbols and provide a backtrace?  I'm not 
> seeing any such behavior on my end, and I'm not sure whether it's my 
> patch or something else that might be present in master.

The problem originally occurred under VMware Fusion and I’m unable to get a backtrace from it.
I suspect memory constraints are a factor.  There’s only 5GB RAM available to this VM.

I have tried in another VM and that succeeds.  All good there. It has 40GB RAM.

I am going to try this on a third system. At present, we’re just 50/50 on success.

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




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

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

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

On Wed, Feb 18, 2015 at 04:17:46PM +0000, Dan Langille (dalangil) wrote:
> I just built from ‘master’, on FreeBSD 9.3:
> 
> cd ~/src
> git clone https://github.com/git/git.git
> cd git
> gmake
> 
> Then tried ~/src/git/git clone https://OUR_REPO
> 
>  It cores too, and I see: git-remote-https.core

Can you compile with debugging symbols and provide a backtrace?  I'm not 
seeing any such behavior on my end, and I'm not sure whether it's my 
patch or something else that might be present in master.
-- 
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] 18+ messages in thread

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-02-17 23:36         ` Junio C Hamano
@ 2015-02-18 16:17           ` Dan Langille (dalangil)
  2015-02-19 20:35             ` brian m. carlson
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-02-18 16:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git, Jeff King

On Feb 17, 2015, at 6:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> "Dan Langille (dalangil)" <dalangil@cisco.com> writes:
> 
>>> On Jan 20, 2015, at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> 
>>> "Dan Langille (dalangil)" <dalangil@cisco.com> writes:
>>> 
>>>> I did not test this patch.  Is that holding up a commit?
>>> 
>>> I am hoping that you rebuilt the Git you use with this patch by the
>>> time you wrote the message I am responding to and have been using it
>>> for your daily Git needs ;-)
>>> 
>>> I believe it is queued on the 'next' branch so that others like you
>>> who need the change can verify the improvements, and others unlike
>>> you who do not need the change can make sure the change does not
>>> cause unintended consequences.
>> 
>> Is this the patch in question?
>> 
>> https://github.com/git/git/commit/4dbe66464b4fd695c5989cc272fa0edd6475037c
>> 
>> I ask because previous versions of the patch acted against http.h as
>> well and my failure with it.
>> 
>> Could I expect that patch work against 2.3.0?
>> 
>> It applies cleanly, compiles, but cores when I try a ‘git clone’.
>> Unmatched 2.3.0 succeeds.
> 
> It already is in 'master', so please holler if things break with
> that version.


I just built from ‘master’, on FreeBSD 9.3:

cd ~/src
git clone https://github.com/git/git.git
cd git
gmake

Then tried ~/src/git/git clone https://OUR_REPO

 It cores too, and I see: git-remote-https.core

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


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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-02-17 23:05       ` Dan Langille (dalangil)
@ 2015-02-17 23:36         ` Junio C Hamano
  2015-02-18 16:17           ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2015-02-17 23:36 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: brian m. carlson, git, Jeff King

"Dan Langille (dalangil)" <dalangil@cisco.com> writes:

>> On Jan 20, 2015, at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> "Dan Langille (dalangil)" <dalangil@cisco.com> writes:
>> 
>>> I did not test this patch.  Is that holding up a commit?
>> 
>> I am hoping that you rebuilt the Git you use with this patch by the
>> time you wrote the message I am responding to and have been using it
>> for your daily Git needs ;-)
>> 
>> I believe it is queued on the 'next' branch so that others like you
>> who need the change can verify the improvements, and others unlike
>> you who do not need the change can make sure the change does not
>> cause unintended consequences.
>
> Is this the patch in question?
>
>  https://github.com/git/git/commit/4dbe66464b4fd695c5989cc272fa0edd6475037c
>
> I ask because previous versions of the patch acted against http.h as
> well and my failure with it.
>
> Could I expect that patch work against 2.3.0?
>
> It applies cleanly, compiles, but cores when I try a ‘git clone’.
> Unmatched 2.3.0 succeeds.

It already is in 'master', so please holler if things break with
that version.

Thanks.

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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-21  0:22     ` Junio C Hamano
  2015-01-22 14:47       ` Dan Langille (dalangil)
@ 2015-02-17 23:05       ` Dan Langille (dalangil)
  2015-02-17 23:36         ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-02-17 23:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git, Jeff King

> On Jan 20, 2015, at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> "Dan Langille (dalangil)" <dalangil@cisco.com> writes:
> 
>> I did not test this patch.  Is that holding up a commit?
> 
> I am hoping that you rebuilt the Git you use with this patch by the
> time you wrote the message I am responding to and have been using it
> for your daily Git needs ;-)
> 
> I believe it is queued on the 'next' branch so that others like you
> who need the change can verify the improvements, and others unlike
> you who do not need the change can make sure the change does not
> cause unintended consequences.

Is this the patch in question?

 https://github.com/git/git/commit/4dbe66464b4fd695c5989cc272fa0edd6475037c

I ask because previous versions of the patch acted against http.h as well and my failure with it.

Could I expect that patch work against 2.3.0?

It applies cleanly, compiles, but cores when I try a ‘git clone’.  Unmatched 2.3.0 succeeds.

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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-21  0:22     ` Junio C Hamano
@ 2015-01-22 14:47       ` Dan Langille (dalangil)
  2015-02-17 23:05       ` Dan Langille (dalangil)
  1 sibling, 0 replies; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-22 14:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git, Jeff King

> On Jan 20, 2015, at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> "Dan Langille (dalangil)" <dalangil@cisco.com> writes:
> 
>> I did not test this patch.  Is that holding up a commit?
> 
> I am hoping that you rebuilt the Git you use with this patch by the
> time you wrote the message I am responding to and have been using it
> for your daily Git needs ;-)

Patch v2 has been used in our test environment with success.  I got diverted to other projects before I could test Patch v3.

> I believe it is queued on the 'next' branch so that others like you
> who need the change can verify the improvements, and others unlike
> you who do not need the change can make sure the change does not
> cause unintended consequences.

Thank you.

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



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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-20 16:40   ` Dan Langille (dalangil)
@ 2015-01-21  0:22     ` Junio C Hamano
  2015-01-22 14:47       ` Dan Langille (dalangil)
  2015-02-17 23:05       ` Dan Langille (dalangil)
  0 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2015-01-21  0:22 UTC (permalink / raw)
  To: Dan Langille (dalangil); +Cc: brian m. carlson, git, Jeff King

"Dan Langille (dalangil)" <dalangil@cisco.com> writes:

> I did not test this patch.  Is that holding up a commit?

I am hoping that you rebuilt the Git you use with this patch by the
time you wrote the message I am responding to and have been using it
for your daily Git needs ;-)

I believe it is queued on the 'next' branch so that others like you
who need the change can verify the improvements, and others unlike
you who do not need the change can make sure the change does not
cause unintended consequences.

Thanks.

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

* Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-08  0:29 ` [PATCH v3] " brian m. carlson
@ 2015-01-20 16:40   ` Dan Langille (dalangil)
  2015-01-21  0:22     ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Langille (dalangil) @ 2015-01-20 16:40 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Junio C Hamano

I did not test this patch.  Is that holding up a commit?
— 
Dan Langille
Infrastructure & Operations
Talos Group
Sourcefire, Inc.

> On Jan 7, 2015, at 7:29 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>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Peff's original change was to get_curl_handle; however, we retry the
> second time with the same slot and we may not call get_curl_handle
> again, so I had to move that change to get_active_slot.  This has been
> tested pushing with both Negotiate and Basic against an HTTPS server
> both when info/refs was protected and when it was not.
> 
> http.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/http.c b/http.c
> index 040f362..44b130c 100644
> --- a/http.c
> +++ b/http.c
> @@ -62,6 +62,9 @@ static const char *user_agent;
> 
> static struct credential cert_auth = CREDENTIAL_INIT;
> static int ssl_cert_password_required;
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +static unsigned long http_auth_methods = CURLAUTH_ANY;
> +#endif
> 
> static struct curl_slist *pragma_header;
> static struct curl_slist *no_pragma_header;
> @@ -580,6 +583,9 @@ struct active_request_slot *get_active_slot(void)
> 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
> 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
> 	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
> +#endif
> 	if (http_auth.password)
> 		init_curl_http_auth(slot->curl);
> 
> @@ -870,6 +876,9 @@ int handle_curl_result(struct slot_results *results)
> 			credential_reject(&http_auth);
> 			return HTTP_NOAUTH;
> 		} else {
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> +#endif
> 			return HTTP_REAUTH;
> 		}
> 	} else {
> @@ -986,6 +995,7 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
> 		strbuf_addstr(charset, "ISO-8859-1");
> }
> 
> +
> /* http_request() targets */
> #define HTTP_REQUEST_STRBUF	0
> #define HTTP_REQUEST_FILE	1
> -- 
> 2.2.1.209.g41e5f3a
> 


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

* [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
  2015-01-01 19:56 [PATCH v2] remote-curl: fall back to Basic " brian m. carlson
@ 2015-01-08  0:29 ` brian m. carlson
  2015-01-20 16:40   ` Dan Langille (dalangil)
  0 siblings, 1 reply; 18+ messages in thread
From: brian m. carlson @ 2015-01-08  0:29 UTC (permalink / raw)
  To: git; +Cc: Dan Langille (dalangil), Jeff King, 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>
Signed-off-by: Jeff King <peff@peff.net>
---
Peff's original change was to get_curl_handle; however, we retry the
second time with the same slot and we may not call get_curl_handle
again, so I had to move that change to get_active_slot.  This has been
tested pushing with both Negotiate and Basic against an HTTPS server
both when info/refs was protected and when it was not.

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

diff --git a/http.c b/http.c
index 040f362..44b130c 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,9 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+static unsigned long http_auth_methods = CURLAUTH_ANY;
+#endif
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -580,6 +583,9 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
+#endif
 	if (http_auth.password)
 		init_curl_http_auth(slot->curl);
 
@@ -870,6 +876,9 @@ int handle_curl_result(struct slot_results *results)
 			credential_reject(&http_auth);
 			return HTTP_NOAUTH;
 		} else {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+#endif
 			return HTTP_REAUTH;
 		}
 	} else {
@@ -986,6 +995,7 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
 		strbuf_addstr(charset, "ISO-8859-1");
 }
 
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF	0
 #define HTTP_REQUEST_FILE	1
-- 
2.2.1.209.g41e5f3a

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

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

Thread overview: 18+ 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
2015-01-01 19:56 [PATCH v2] remote-curl: fall back to Basic " brian m. carlson
2015-01-08  0:29 ` [PATCH v3] " brian m. carlson
2015-01-20 16:40   ` Dan Langille (dalangil)
2015-01-21  0:22     ` Junio C Hamano
2015-01-22 14:47       ` Dan Langille (dalangil)
2015-02-17 23:05       ` Dan Langille (dalangil)
2015-02-17 23:36         ` Junio C Hamano
2015-02-18 16:17           ` Dan Langille (dalangil)
2015-02-19 20:35             ` brian m. carlson
2015-02-24 21:03               ` Dan Langille (dalangil)
2015-02-25 20:59                 ` Dan Langille (dalangil)
2015-03-10 18:05                   ` Dan Langille (dalangil)
2015-03-10 22:29                     ` brian m. carlson
2015-03-11 19:33                       ` Dan Langille (dalangil)
2015-03-11 21:59                         ` brian m. carlson
2015-03-12 13:09                           ` Dan Langille (dalangil)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).