{"thread":{"id":"44652","subject":"[REGRESSION 2.10.2] problematic \"empty auth\" changes","startedAt":"2016-12-08T14:48:12Z","lastAt":"2016-12-10T15:23:51Z","messageCount":7,"participants":["Johannes Schindelin","David Turner","Junio C Hamano","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"307251","messageId":"alpine.DEB.2.20.1612081538260.23160@virtualbox","threadId":"44652","inReplyTo":null,"subject":"[REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-12-08T14:47:33Z","receivedAt":"2016-12-08T14:48:12Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Dave,\n\nI got a couple of bug reports that claim that 2.10.2 regressed on using\nnetwork credentials. That is, users regularly hit Enter twice when being\nasked for user name and password while fetching via https://, and cURL\nautomatically used to fall back to using the login credentials (i.e.\nauthenticating via the Domain controller).\n\nTurns out those claims are correct: hitting Enter twice (or using URLs\nwith empty user name/password such as https://:tfs:8080/) work in 2.10.1\nand yield \"Authentication failed\" in 2.10.2.\n\nI tracked this down to 5275c3081c (http: http.emptyauth should allow empty\n(not just NULL) usernames, 2016-10-04) which all of a sudden disallowed\nempty user names (and now only handles things correctly when\nhttp.emptyAuth is set to true specifically).\n\nThis smells like a real bad regression to me, certainly given the time I\nhad to spend to figure this out (starting from not exactly helpful bug\nreports, due to being very specific to their setups being private).\n\nI am *really* tempted to change the default of http.emptyAuth to true, *at\nleast* for Windows (where it is quite common to use your login credentials\nto authenticate to corporate servers).\n\nBefore I do anything rash, though: Do you see any downside to that?\n\nCiao,\nDscho\n"},{"id":"307335","messageId":"1481231552.20894.20.camel@frank","threadId":"44652","inReplyTo":"alpine.DEB.2.20.1612081538260.23160@virtualbox","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2016-12-08T21:12:32Z","receivedAt":"2016-12-09T09:13:21Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2016-12-08 at 15:47 +0100, Johannes Schindelin wrote:\n> Hi Dave,\n> \n> I got a couple of bug reports that claim that 2.10.2 regressed on using\n> network credentials. That is, users regularly hit Enter twice when being\n> asked for user name and password while fetching via https://, and cURL\n> automatically used to fall back to using the login credentials (i.e.\n> authenticating via the Domain controller).\n> \n> Turns out those claims are correct: hitting Enter twice (or using URLs\n> with empty user name/password such as https://:tfs:8080/) work in 2.10.1\n> and yield \"Authentication failed\" in 2.10.2.\n> \n> I tracked this down to 5275c3081c (http: http.emptyauth should allow empty\n> (not just NULL) usernames, 2016-10-04) which all of a sudden disallowed\n> empty user names (and now only handles things correctly when\n> http.emptyAuth is set to true specifically).\n> \n> This smells like a real bad regression to me, certainly given the time I\n> had to spend to figure this out (starting from not exactly helpful bug\n> reports, due to being very specific to their setups being private).\n> \n> I am *really* tempted to change the default of http.emptyAuth to true, *at\n> least* for Windows (where it is quite common to use your login credentials\n> to authenticate to corporate servers).\n> \n> Before I do anything rash, though: Do you see any downside to that?\n\nI know of no reason that shouldn't work.  Indeed, it's what we use do\ninternally.  So far, nobody has reported problems.  That said, we have\nexactly three sets of git servers that most users talk to (two different\ninternal; and occasionally github.com for external stuff).  So our\ncoverage is not very broad.\n\nIf you're going to do it, tho, don't just do it for Windows users -- do\nit for everyone.  Plenty of Unix clients connect to Windows-based auth\nsystems.\n\nThat said, I could imagine that there are cases where it would cause\nfailures for a different set of users.  I don't know of any off the top\nof my head, but this is not my area of expertise.\n\nWe could move closer to the old behavior with something like:\n\n        if (!http_auth.username || !*http_auth.username) {\n                if (curl_empty_auth)\n                        curl_easy_setopt(result, CURLOPT_USERPWD, \":\");\n                if (!http_auth.username)\n                        return;\n        }\n\nThis is very ad-hoc, in that I have not examined exactly when\nhttp_auth.username would be null vs empty; it merely attempts to get as\nclose as possible to the old behavior.\n\n[Side note: I was curious if 26a7b23429 would have been a better fix for\nour problem.  It appears that the answer is no; using that patch but\nminus my 5275c3081c does not work for us.]\n\n\n"},{"id":"307365","messageId":"alpine.DEB.2.20.1612091048540.23160@virtualbox","threadId":"44652","inReplyTo":"1481231552.20894.20.camel@frank","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-12-09T15:58:31Z","receivedAt":"2016-12-09T15:58:43Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi David,\n\nOn Thu, 8 Dec 2016, David Turner wrote:\n\n> On Thu, 2016-12-08 at 15:47 +0100, Johannes Schindelin wrote:\n> \n> > I got a couple of bug reports that claim that 2.10.2 regressed on\n> > using network credentials. That is, users regularly hit Enter twice\n> > when being asked for user name and password while fetching via\n> > https://, and cURL automatically used to fall back to using the login\n> > credentials (i.e.  authenticating via the Domain controller).\n> > \n> > Turns out those claims are correct: hitting Enter twice (or using URLs\n> > with empty user name/password such as https://:tfs:8080/) work in\n> > 2.10.1 and yield \"Authentication failed\" in 2.10.2.\n> > \n> > I tracked this down to 5275c3081c (http: http.emptyauth should allow\n> > empty (not just NULL) usernames, 2016-10-04) which all of a sudden\n> > disallowed empty user names (and now only handles things correctly\n> > when http.emptyAuth is set to true specifically).\n> > \n> > This smells like a real bad regression to me, certainly given the time\n> > I had to spend to figure this out (starting from not exactly helpful\n> > bug reports, due to being very specific to their setups being\n> > private).\n> > \n> > I am *really* tempted to change the default of http.emptyAuth to true,\n> > *at least* for Windows (where it is quite common to use your login\n> > credentials to authenticate to corporate servers).\n> > \n> > Before I do anything rash, though: Do you see any downside to that?\n> \n> I know of no reason that shouldn't work.  Indeed, it's what we use do\n> internally.  So far, nobody has reported problems.\n\nGood.\n\n> That said, we have exactly three sets of git servers that most users\n> talk to (two different internal; and occasionally github.com for\n> external stuff).  So our coverage is not very broad.\n\nOkay. I think I will extend that coverage rather boldly, then ;-)\n\n> If you're going to do it, tho, don't just do it for Windows users -- do\n> it for everyone.  Plenty of Unix clients connect to Windows-based auth\n> systems.\n\nMakes sense.\n\n> That said, I could imagine that there are cases where it would cause\n> failures for a different set of users.\n\nLet's see. At the moment, my main concern is that Git for Windows is\nbroken for corporate users (i.e. users who rely on the implicit\nlogin authentication provided through their domain accounts). I cannot\nimagine that defaulting http.emptyAuth=true could cause any worse\nbreakage.\n\nIt would be different, of course, if http.emptyAuth would *not* allow the\nuser to type their credentials when accessing something like\nhttps://github.com/dscho/shhh-secret-repository, *only* trying the login\ncredentials. But that is not the case, with http.emptyAuth=true, login\ncredentials are attempted first, and when they fail, the user is still\nasked interactively for their credentials.\n\nAll I can see is that this would be *an improvement*: corporate users\ntrying to access a Git repository that requires their login credentials\nwould now not even need to enter empty user name/password.\n\nThis alone would be already a good reason to change the default, IMHO.\n\nSo here is my plan:\n\n- change the default of http.emptyAuth to true in the next Git for Windows\n  version\n\n- publish a prerelease for early adopters to test\n\n- contribute this patch here on the Git mailing list, in the hope that it\n  will make it into the next major version\n\nCiao,\nDscho\n"},{"id":"307390","messageId":"xmqqoa0krjaf.fsf@gitster.mtv.corp.google.com","threadId":"44652","inReplyTo":"alpine.DEB.2.20.1612091048540.23160@virtualbox","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-09T19:31:36Z","receivedAt":"2016-12-09T19:31:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> It would be different, of course, if http.emptyAuth would *not* allow the\n> user to type their credentials when accessing something like\n> https://github.com/dscho/shhh-secret-repository, *only* trying the login\n> credentials. But that is not the case, with http.emptyAuth=true, login\n> credentials are attempted first, and when they fail, the user is still\n> asked interactively for their credentials.\n>\n> All I can see is that this would be *an improvement*: corporate users\n> trying to access a Git repository that requires their login credentials\n> would now not even need to enter empty user name/password.\n\nYup, my thought process after seeing your first message to David\nexactly mirrored the above two paragraphs.  It sounds like you two\nhave a good plan ;-)\n\nThanks.\n\n> This alone would be already a good reason to change the default, IMHO.\n>\n> So here is my plan:\n>\n> - change the default of http.emptyAuth to true in the next Git for Windows\n>   version\n>\n> - publish a prerelease for early adopters to test\n>\n> - contribute this patch here on the Git mailing list, in the hope that it\n>   will make it into the next major version\n>\n> Ciao,\n> Dscho\n"},{"id":"307397","messageId":"20161209221854.re6qf3e5225wxvge@genre.crustytoothpaste.net","threadId":"44652","inReplyTo":"1481231552.20894.20.camel@frank","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2016-12-09T22:19:44Z","receivedAt":"2016-12-09T22:19:55Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Dec 08, 2016 at 04:12:32PM -0500, David Turner wrote:\n> I know of no reason that shouldn't work.  Indeed, it's what we use do\n> internally.  So far, nobody has reported problems.  That said, we have\n> exactly three sets of git servers that most users talk to (two different\n> internal; and occasionally github.com for external stuff).  So our\n> coverage is not very broad.\n> \n> If you're going to do it, tho, don't just do it for Windows users -- do\n> it for everyone.  Plenty of Unix clients connect to Windows-based auth\n> systems.\n\nLet me echo this.  This would make Kerberos (and probably other forms of\nSPNEGO) work out of the box, which would reduce a lot of confusion that\npeople have.\n\nI can confirm enabling http.emptyAuth works properly with Kerberos,\nincluding with fallback to Basic, so I see no reason why we shouldn't do\nit.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"307439","messageId":"alpine.DEB.2.20.1612101551100.23160@virtualbox","threadId":"44652","inReplyTo":"20161209221854.re6qf3e5225wxvge@genre.crustytoothpaste.net","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-12-10T14:52:39Z","receivedAt":"2016-12-10T14:53:11Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brian,\n\nOn Fri, 9 Dec 2016, brian m. carlson wrote:\n\n> On Thu, Dec 08, 2016 at 04:12:32PM -0500, David Turner wrote:\n> > I know of no reason that shouldn't work.  Indeed, it's what we use do\n> > internally.  So far, nobody has reported problems.  That said, we have\n> > exactly three sets of git servers that most users talk to (two\n> > different internal; and occasionally github.com for external stuff).\n> > So our coverage is not very broad.\n> > \n> > If you're going to do it, tho, don't just do it for Windows users --\n> > do it for everyone.  Plenty of Unix clients connect to Windows-based\n> > auth systems.\n> \n> Let me echo this.  This would make Kerberos (and probably other forms of\n> SPNEGO) work out of the box, which would reduce a lot of confusion that\n> people have.\n> \n> I can confirm enabling http.emptyAuth works properly with Kerberos,\n> including with fallback to Basic, so I see no reason why we shouldn't do\n> it.\n\nOne of my colleagues offered a legitimate concern: it potentially adds\nanother round-trip.\n\nDo you happen to know whether regular HTTPS negotiation will have an extra\nround-trip if Kerberos is attempted, but we have to fall back to\ninteractively prompt for (or use stored) credentials?\n\nCiao,\nJohannes\n"},{"id":"307440","messageId":"20161210152338.vzd4w3pr7wy6p4ki@genre.crustytoothpaste.net","threadId":"44652","inReplyTo":"alpine.DEB.2.20.1612101551100.23160@virtualbox","subject":"Re: [REGRESSION 2.10.2] problematic \"empty auth\" changes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2016-12-10T15:23:39Z","receivedAt":"2016-12-10T15:23:51Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Dec 10, 2016 at 03:52:39PM +0100, Johannes Schindelin wrote:\n> One of my colleagues offered a legitimate concern: it potentially adds\n> another round-trip.\n> \n> Do you happen to know whether regular HTTPS negotiation will have an extra\n> round-trip if Kerberos is attempted, but we have to fall back to\n> interactively prompt for (or use stored) credentials?\n\nWith Kerberos (using tickets), you have 7 request/response pairs, and an\nadditional round trip for the 100 Continue if your push is larger than\nhttp.postBuffer.  You only have 6 for Basic using Kerberos.\n\nHowever, libcurl is generally going to be able to figure out whether\nyour Kerberos credentials can be used, so when it falls back to Basic,\nit does so because it knows you have nothing to use with Negotiate (e.g.\nyou have no ticket), and therefore it doesn't even try.  I suppose if I\ntried to push to a server that offered Negotiate and Basic, but didn't\naccept my Kerberos credentials, it might fall back in such a way,\nthough.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"}]}