-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PUT with Expect header sends request body prematurely (upload is broken) #223
Comments
bisect in Windows VS2010 build shows request body behavior changed in 5dc68dd, 'HTTP: don't abort connections with pending Negotiate authentication'. @tvbuehler |
My guess is that the same problem existed before my change with NTLM instead of Negotiate (because the point of my patch was to make NTLM and Negotiate behave the same). Maybe forcing "auth_start" to false when the response code is not 401/407 could fix this at least partially. The main problem is that there is no separate request for "make sure authentication is ok for the following PUT/POST/...", instead we have to authenticate in the same connection with NTLM/Negotiate. So we send a request with a large body and get a 401/407 - now we can't abort the request, because we have to continue the authentication in the same connection. (chunked uploading would help too, because then you could just stop sending data and still keep the connection in a valid state. but some servers won't like chunked uploading / no content-length..) |
We have code that makes NTLM authentication work connection-oriented but we lack all that for Negotiate. The current situation is not working, the previous situation was not working. So, the questions are A) what we do short-term for the pending release and B) what we do long-term to get this fixed properly. Ideas? |
I have to make a stab here. I personally do not use and wouldn't use NTLM, I use either Kerberos or SPNEGO with Kerberos. In this case one can have request-level authentication because it always requires two tokens only. All is well after one request. The main point is, a client cannot make any assumptions about the server whether it will persist the authentication or not. @bagder, we have used the previous situation with SPNEGO against multiple servers with MIT Kerberos, JGSS and SSPI with various requests (GET, POST, PUT). We did not notice any problem before. Probably, I missed that discussion. If someone has some valueable patch, please let me know, I'll test. |
@michael-o, I appreciate your input on this and your use case that no longer works is a pretty strong argument for reverting the offending patch as a first remedy. The fact remains though that Negotiate is an authentication method that in my view is even worse than NTLM (and that is a pretty hard blow) and the fact that it may use NTLM in disguise makes it terribly annoying and hard to get right. I imagine that in your use cases it doesn't however and you end up with the kerberos version. Did you try simply reverting the 5dc68dd commit and see if that works for you? |
@bagder, I checked out the 7.40.0 tag from Git, did a
It does work! Though, I do not understand why curl is throwing away the connection when it receives 401?! I see no reason for. |
It closes the connection because of the "Excess found in a non pipelined read" condition. I can't quite understand it either right now, but libcurl does not typically close a connection just because of a 401. I reverted that commit just now. Thanks @jay for the bisect! Sorry @tvbuehler, I realize this then removes a fix you wanted but I figure keeping the prior functionality takes precedence here. I believe @Frenche (?) is also currently working on fixing how Negotiate is (not) re-using connections properly. |
It would perhaps be suitable to deal with that in a separate issue. I consider this particular issue fixed now, even if it left us with some open wounds to heal... |
@bagder, thanks for the quick solution. Very much appreciated. Just to be clear, do you want to open another issue on the read excess with connection close? @tvbuehler, if you have another patch. Ping me in the new ticket. I'll test. |
@michael-o: sure, but I can't promise I'll be able to work on it anytime soon... |
Well. My first thought was "Are you crazy? The fix actually fixed not working uploads". And tried to produce a test case with squid3. And now the result is: If squid3 responds with 407 and curl continues to upload the large body, and then (after the post body is finished) starts a new request in the same connection with the valid authentication, squid3 just ignores all data it receives and times out after 2 minutes... So I have no idea what curl actually should do :( |
@tvbuehler I realize your fix solved a problem, there's no denying of that but we're facing a release in less than 48 hours and it made a previously working use-case no longer work. Reverting a commit is never something we want to do, but in this case I simply favored to have the previous functionality there and re-work the new fix than the opposite. I'm sorry we're in this situation. The Negotiate authentication in libcurl will also get a security patch just before 7.42.0 is released that will cripple it further since the current implementation leaves it open to a security problem (we will announce that on Wednesday when the release goes out). All taken together: Negotiate support in libcurl needs (much) more work to get really good. |
Hi, On Mon, Apr 20, 2015 at 2:32 PM, Daniel Stenberg notifications@github.com
I found some interesting material to investigate and I'd like to test how |
@tvbuehler, I don't understand that behavior in Squid. Furthermore, as a client implementor, You would not make any assumptions whether the server will persist that auth or not. You cannot know and you won't. You should have a look at libserf. Lieven Govaerts invested quite a lot of time to make both auth stateful and stateless. I did the testing at work against various servers. @Frenche, I can confirm that this is used and is standard in Active Directory environment. We a farm of proxy servers with a cname for all of them. Microsoft TMG is running on that. I do authenticate against them with SPNEGO with libserf (for Subversion) and libcurl (for Git). This was actually my first initiative last year to get decent support in curl, in order to use external Git repos. |
@michael-o I have absolutely no idea what you are trying to tell me. But just for the record: both NTLM and Negotiate are supposed to authenticate the connection (not a single request), and the authentication steps should be done in a single connection - at least that is how I read some specs some months ago. |
On Mon, Apr 20, 2015 at 5:34 PM, Stefan Bühler notifications@github.com
From security perspectives I agree with Michael that the best way is to From connectivity perspectives however we can investigate how to guess best For a good guess we need to understand if NTLM was used and try to read I am aware that Negotiate is used with Proxies in practice but it doesn't One more note, when authenticating to a server with Negotiate and thru a If an HTTP proxy is used between the client and server, it must take |
Actually, I did not say that in such a way. All I said was that a client should not make any assumptions about the auth level (request or connection). Expect both behaviors. The behavior with IIS is a very good example. For instance,
Keep in mind that those headers are non-standard and are send by Microsoft products only. I would rather maintain an information on the connection whether auth is complete and reset when the server nags you again. This would ultimately mean that the auth was not persistent.
This is a quite interesting case I have thought about several months ago but how likely is this? Any Kerberos-protected host is supposed to be inside the company and not outside. For us, we have a PAC-enabled proxy server which has a lot of domains which are within the corporate network as well as in the local intranet zone. |
On Mon, Apr 20, 2015 at 8:31 PM, Michael Osipov notifications@github.com
Leaving security aside, if we guess it is likely to be a request-based (non
|
@Frenche, I agree on the rest if and only if it will be well documented. Though, I do not agree on this for several reasons:
|
On Mon, Apr 20, 2015 at 11:02 PM, Michael Osipov notifications@github.com
A client may initiate a connection to the server with an
|
Guys, what about starting over with a fresh issue/pull request and try to work on getting some code done to improve this? |
Guys, so what's the solution if I'm getting exactly the same problem? It's hard to figure this out from your conversation and I believe I'm not the only one stuck with this. |
If you have a problem, the same as this or another one, using a modern curl version (say 7.50.x) then please file a new issue. Adding comments to an old issue thread is very hard to do anything sensible with. |
No, I have |
I have recently upgraded from curl 7.38.0 to 7.41.0 to for testing purposes. After that
PUT
ting files was no longer possible to a server requiring auth.Though, we are using libcurl in a C app on HP-UX, I was able to reproduce the problem on Windows and FreeBSD with curl too.
I have tested every single version between 38 and 41 and identified that the issue appeared first in 7.40.0. 7.39.0 works flawlessly.
Call with curl 7.39.0:
Output:
As you can see,
PUT
withExpect
header is perfectly fine.Now the curl 7.40.0 call:
Output:
Server is sending
Bad Request
due to the broken upload.I ran both commands with
--trace
and identified that > 7.39.0 sends the request body even though there is no positive feedback from the server. Using-H 'Expect:'
has no avail.I assume that stuff have got broken somewhere here:
git log curl-7_39_0...curl-7_40_0
. I am willing to try a patch or bisect if we can narrow down the (broken) commits.The text was updated successfully, but these errors were encountered: