Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-25 Thread Johannes Schindelin
Hi Peff, On Thu, 23 Feb 2017, Jeff King wrote: > For Git for Windows, [PATCH 2/2] seems like the auto behavior would be a > strict improvement over the "true" default they've been shipping. Absolutely. Thank you for your tremendous help! Ciao, Dscho

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Jeff King
On Thu, Feb 23, 2017 at 12:37:25PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I suspect it isn't enough to help without 2/2. This will tell curl that > > the server does not do Negotiate, so it will skip the probe request. But > > Git will still feed curl the bogus

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Junio C Hamano
Jeff King writes: > I suspect it isn't enough to help without 2/2. This will tell curl that > the server does not do Negotiate, so it will skip the probe request. But > Git will still feed curl the bogus empty credential. > > That's what 2/2 tries to fix: only kick in the

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Jeff King
On Thu, Feb 23, 2017 at 06:08:49PM +0100, Johannes Schindelin wrote: > > I suspect the patch above could probably be generalized as: > > > > /* cut out methods we know the server doesn't support */ > > http_auth_methods &= results.auth_avail; > > > > and let curl figure it out from there. >

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Jeff King
On Thu, Feb 23, 2017 at 11:11:05AM -0800, Junio C Hamano wrote: > >> As far as Kerberos, this is a desirable feature to have enabled, with > >> little downside. I just don't know about the security of the NTLM part, > >> and I don't think we should take this patch unless we're sure we know > >>

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Junio C Hamano
Jeff King writes: > On Wed, Feb 22, 2017 at 11:34:19PM +, brian m. carlson wrote: > >> Browsers usually disable this feature by default, as it basically will >> attempt to authenticate to any site that sends a 401. For Kerberos >> against a malicious site, the user will

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Junio C Hamano
Johannes Schindelin writes: > On Wed, 22 Feb 2017, Jeff King wrote: >> This patch drops the useless probe request: > ... >> but setting http.emptyauth adds back in the useless request. I think >> that could be fixed by skipping the empty-auth thing when >>

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Johannes Schindelin
Hi Peff, On Wed, 22 Feb 2017, Jeff King wrote: > On Wed, Feb 22, 2017 at 01:16:33PM -0800, Junio C Hamano wrote: > > > David Turner writes: > > > > > Always, no. For failed authentication (or authorization), > > > apparently, yes. I tested this by setting the

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-23 Thread Mantas Mikulėnas
On 2017-02-23 03:03, David Turner wrote: > Actually, though, I am not sure this is as bad as it seems, because gssapi > might protect us. When I locally tried a fake server, git (libcurl) refused > to > send my Kerberos credentials because "Server not found in Kerberos > database". I don't

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread brian m. carlson
On Thu, Feb 23, 2017 at 01:03:39AM +, David Turner wrote: > So, I guess, this patch might be considered a security risk. But on the > other hand, even *without* this patch, and without http.allowempty at > all, I think a config which simply uses a https:// url without the magic :@ > would

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
Jeff King writes: > On Wed, Feb 22, 2017 at 11:34:19PM +, brian m. carlson wrote: > >> Browsers usually disable this feature by default, as it basically will >> attempt to authenticate to any site that sends a 401. For Kerberos >> against a malicious site, the user will

RE: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread David Turner
> -Original Message- > From: brian m. carlson [mailto:sand...@crustytoothpaste.net] > > This is SPNEGO. It will work with NTLM as well as Kerberos. > > Browsers usually disable this feature by default, as it basically will > attempt to > authenticate to any site that sends a 401. For

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 11:34:19PM +, brian m. carlson wrote: > Browsers usually disable this feature by default, as it basically will > attempt to authenticate to any site that sends a 401. For Kerberos > against a malicious site, the user will either not have a valid ticket > for that

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread brian m. carlson
.tur...@twosigma.com> > > Cc: git@vger.kernel.org; sand...@crustytoothpaste.net; Johannes Schindelin > > <johannes.schinde...@gmx.de>; Eric Sunshine > > <sunsh...@sunshineco.com>; Jeff King <p...@peff.net> > > Subject: Re: [PATCH] http(s): automatically try NTLM authen

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 02:35:11PM -0800, Junio C Hamano wrote: > A solution along your line would help Negotiate users OOB experience > without hurting the servers that do not offer Negotiate, but until > that materializes, users can set the lazier http.emptyAuth on > (without selectively

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
Jeff King writes: > On Wed, Feb 22, 2017 at 01:57:28PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > On Wed, Feb 22, 2017 at 01:25:11PM -0800, Junio C Hamano wrote: >> >> >> >> Thanks for your thoughts. I'd think that we should take this change >>

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 01:57:28PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > On Wed, Feb 22, 2017 at 01:25:11PM -0800, Junio C Hamano wrote: > >> > >> Thanks for your thoughts. I'd think that we should take this change > >> and leave the optimization for later,

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
Jeff King writes: > On Wed, Feb 22, 2017 at 01:25:11PM -0800, Junio C Hamano wrote: >> >> Thanks for your thoughts. I'd think that we should take this change >> and leave the optimization for later, then. It's not like the >> change of the default is making the normal situation

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 01:25:11PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I don't think it incurs an extra round-trip now, because of the way > > libcurl works. Though I think it _does_ make it harder for curl to later > > optimize out that extra round-trip. > >

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 01:16:33PM -0800, Junio C Hamano wrote: > David Turner writes: > > > Always, no. For failed authentication (or authorization), apparently, yes. > > > > I tested this by setting the variable to false and then true, and trying > > to > >

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
Jeff King writes: > I don't think it incurs an extra round-trip now, because of the way > libcurl works. Though I think it _does_ make it harder for curl to later > optimize out that extra round-trip. > ... > In the current trace, you can see that libcurl insists on making a >

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
David Turner writes: > Always, no. For failed authentication (or authorization), apparently, yes. > I tested this by setting the variable to false and then true, and trying to > Push to a github repository which I didn't have write access to, with > both an empty

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Jeff King
On Wed, Feb 22, 2017 at 12:19:56PM -0800, Junio C Hamano wrote: > > It would be one thing if cURL would not let the user specify credentials > > interactively after attempting NTLM authentication (i.e. login > > credentials), but that is not the case. > > > > It would be another thing if

RE: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread David Turner
elin > <johannes.schinde...@gmx.de>; Eric Sunshine > <sunsh...@sunshineco.com>; Jeff King <p...@peff.net> > Subject: Re: [PATCH] http(s): automatically try NTLM authentication first > > David Turner <dtur...@twosigma.com> writes: > > > Fro

Re: [PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread Junio C Hamano
David Turner writes: > From: Johannes Schindelin > > It is common in corporate setups to have permissions managed via a > domain account. That means that the user does not really have to log in > when accessing a central repository via https://,

[PATCH] http(s): automatically try NTLM authentication first

2017-02-22 Thread David Turner
From: Johannes Schindelin It is common in corporate setups to have permissions managed via a domain account. That means that the user does not really have to log in when accessing a central repository via https://, but that the login credentials are used to