Version 2.11.0(3) does not ask for username/password #1034
Comments
This status code doesn't seem to be related to an authentication error 🤔 Could you run the following commands in Git Bash and attach the output? |
|
The server also returns HTTP 500 if I enter wrong login data (using the old git version without the bug). Here is the output of the commands you suggested: |
|
Interesting - I don't see a |
|
I believe that this indicates that cURL receives a header that suggests that NTLM authentication can be used (which it cannot). Maybe it works if you set |
@PKEuS ? |
|
This works indeed, but why does it get broken when a working installation is updated? I just updated it on another machine, and there it now fails, too (unless I manually edit the config). |
It was intended as a bug fix for a problem introduced by upstream Git v2.11.0. We discussed back and forth whether it is possible that the NTLM authentication is attempted by mistake, and concluded that it would be a server misconfiguration if that happened. Your report suggests that there might have been a mistake in our assessment? In any case, the problems that were solved by switching the default to allowing "empty" authentication appear to outweigh the downsides, as major platforms rely on that "empty" authentication to switch to NTLM authentication automatically (Visual Studio Team Services, and AFAIK GitHub for Enterprise, for example). I am far from an expert in the details of HTTPS authentication, but maybe there is a header that is sent back by the server from which you try to fetch that fools cURL into attempting NTLM authentication instead of falling back to asking for credentials interactively? |
Is the usage from command line or via TortoiseGit not an important use-case? Both are affected from this. Or is it a TortoiseGit bug which does not set the config properly? |
You misunderstand. Command-line usage of Git works plenty fine with the bug fix. What causes the problem here is the server you try to access. It apparently reports via the HTTP response header that it is okay for cURL to go ahead and attempt NTLM authentication, but that fails, so it was lying. |
We're seeing a similar problem. The server is responding with "WWW-Authenticate: Basic" so I don't see that this would be a signal that it's OK to attempt NTLM. Here's our trace (somewhat redacted) which is similar to the above although we're reporting 403 on the empty username attempt. < HTTP/1.1 401 Unauthorized
< HTTP/1.1 403 Forbidden
|
|
Based on these two RFCs, the server response in this case is correctly telling the client that Basic authentication is supported, and NOT NTLM https://tools.ietf.org/html/rfc2617#page-5 The Git client should use these headers to make the decision to send the empty username and password rather than just making it the default without any further checks as apparently was done here? 713b253 |
|
The 'right way' of doing such negatiation occurs on the git list a number of times with different fall backs depending on who is reporting a misconfigured server in the wild ;-) a Quick search of the list come up with Brian Carson's https://public-inbox.org/git/[email protected]/ Brian has teneded to be the person who has done the most work (AFAICT). Maybe have a look back at some of the discussion to see what solution was found for these tricky 'how to pass an empty username' issues. |
|
The server isn't misconfigured, it's correctly sending "WWW-Authenticate: Basic" which means the server does not support NTML so why would the client try to send an empty username and password by default when it would fail. The client, in this case is wrong. "Wrong" as in it's not following the RFC rules the rest of the world is following. The Git client should respect the HTTP header that's explicitly telling the client what authentication method it supports. |
|
@DaveRinker telling me that I should break the bug again that was fixed by the change is unlikely to have the intended effect. I would appreciate it if you could work with us to get this fixed both for you as well as for users of Visual Studio Team Services. As I stated before, I am far from an expert when it comes to NTLM authentication (and how cURL can figure out whether it is available or not). So it is even more important that you help with the resolution of the technical issue. |
|
@dscho fair enough. Do you have a git trace from VSTS by chance? |
|
I've got one... looks like VSTS sends "WWW-Authenticate: TFS-Federated". I think we can use the server response to decide if we need to send blank (e.g. if it's WWW-Authenticate: Basic don't send blank). Response from VSTS: HTTP/1.1 401 Unauthorized |
|
@DaveRinker that's true for .microsoft.com, but for local Windows hosted instances (like the ones enterprises deploy internally) they rely on Active Directory authentication via NTLM. In those cases the server will send If an empty username and password pair cannot be sent, there's no way for libcurl to enter into NTLM via negotiate. This cripples thousands of users who rely on Windows hosted services. |
|
@whoisj If the server sends only |
|
@DaveRinker I believe Git tries with no basic authentication initially, and if it gets a 401/403 error it'll then prompt you for credentials. Though, to be honest I'm not completely certain. |
|
Yes, that's right. So the flow is: Git Client -> request with no auth What changed recently was the second Git Client "request with auth" is now sending ":" by default and we'd like to make that only the default if "foo" is not basic. If the server sends 401 and only supports basic, a blank user ID and password is not going to work. This should continue to be a happy path for Negotiate and TFS but have the advantage of not breaking traditional servers that only use basic auth. As it stands now, people with traditional servers would have to override the server behavior of rejecting the well-formed but improper authentication (e.g. I have sent a blank user ID and password) instead of sending the 403, re-send the 401 challenge a second time. This causes the client to finally prompt/send a user ID and password. Changing this low-level auth logic may be tricky depending on the server. Once I have time, I'll see if I can find where we should make this change. |
|
I wonder if this additional side-effects from the |
I am quite certain about it. The question is whether we can ask cURL to set the credentials only when the first (401) reply contained a non-Basic header. |
|
@dscho right |
|
@DaveRinker any progress on that front? |
|
Changing the server to re-send a 401 challenge when receiving an empty auth (":") does work even if it's technically a bit weird. I haven't looked into the modification to be able to look at the response header from the server on the cURL side |
Thanks for the feedback. I had a very quick look and could not easily find out how to intercept when cURL chooses Negotiate vs Basic. But then, I am far from an expert how to use cURL. If you have the time, the best way forward is probably to connect with the cURL developers, describe the problem, and hope that they have a really easy suggestion how we can fix Git. |
|
Not a cURL expert either, but from looking at enough traces it appears that cURL evaluates potential authentication schemes in the order they are presented as options by the challenging host. |
Yes, I also came to the conclusion from my cursory look that there is not actually any provision to intercept or hook into that part. Hence my suggestion to involve the cURL developers (who are quite active and helpful). |
Turning on http.emptyAuth globally turned out to cause a regression in case of some servers that support Basic authentication but do not respond with *yet* another 401 upon receiving the empty user/password: git-for-windows#1034 Jeff King came up with a superior approach. Let's revert our change in preparation for including those patches. This reverts commit de4b3c5. Signed-off-by: Johannes Schindelin <[email protected]>
Git for Windows [no longer attempts to send empty credentials to HTTP(S) servers that handle only Basic and/or Digest authentication](git-for-windows/git#1034). Signed-off-by: Johannes Schindelin <[email protected]>
Setup
defaults?
to the issue you're seeing?
Details
The text was updated successfully, but these errors were encountered: