Skip to content
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

Closed
michael-o opened this issue Apr 16, 2015 · 24 comments
Closed
Assignees
Labels

Comments

@michael-o
Copy link
Contributor

I have recently upgraded from curl 7.38.0 to 7.41.0 to for testing purposes. After that PUTting 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:

C:\Users\osipovmi>d:\curl-7.39.0\curl  --verbose --negotiate -u : -H "Accept: application/json" --upload-file 2316975_A5E55802418A_2.docx http://<hostname>:8081/webapp/rest/files/2316975_A5E55802418A_2.docx

Output:

* Hostname was NOT found in DNS cache
*   Trying <server_ip>...
* Connected to <hostname> (<server_ip>) port 8081 (#0)
> PUT /webapp/rest/files/2316975_A5E55802418A_2.docx HTTP/1.1
> User-Agent: curl/7.39.0
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
> 
< HTTP/1.1 401 Unauthorized
< Server: Apache-Coyote/1.1
< WWW-Authenticate: Negotiate
< Content-Type: text/html;charset=utf-8
< Content-Length: 974
< Date: Thu, 16 Apr 2015 12:51:42 GMT
< 
* Excess found in a non pipelined read: excess = 974 url = /webapp/rest/files/2316975_A5E55802418A_2.docx (zero-length body)
* Closing connection 0
* Issue another request to this URL: 'http://<hostname>:8081/webapp/rest/files/2316975_A5E55802418A_2.docx'
* Hostname was found in DNS cache
*   Trying <server_ip>...
* Connected to <hostname> (<server_ip>) port 8081 (#1)
* Server auth using Negotiate with user ''
> PUT /webapp/rest/files/2316975_A5E55802418A_2.docx HTTP/1.1
> Authorization: Negotiate YIIQ1wYGKwYBB...juOWtcZ0e3wlRFWBg==
> User-Agent: curl/7.39.0
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
>
< HTTP/1.1 100 Continue
} [data not shown]
* We are completely uploaded and fine
< HTTP/1.1 201 Created
< Server: Apache-Coyote/1.1
< WWW-Authenticate: Negotiate oYHrMIH...SVm8YA==
< Connection: close
< Content-Type: application/json
< Content-Length: 24
< Date: Thu, 16 Apr 2015 12:51:42 GMT
< 
{"uid":"RxfVyT$nBv4vwA"}
* Closing connection 1

As you can see, PUT with Expect header is perfectly fine.

Now the curl 7.40.0 call:

C:\Users\osipovmi>d:\curl-7.40.0\curl --verbose --negotiate -u : -H "Accept: application/json" --upload-file 2316975_A5E55802418A_2.docx http://<hostname>:8081/webapp/rest/files/2316975_A5E55802418A_2.docx

Output:

*   Trying <server_ip>...
* Connected to <hostname> (<server_ip>) port 8081 (#0)
> PUT /webapp/rest/files/2316975_A5E55802418A_2.docx HTTP/1.1
> User-Agent: curl/7.40.0
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
> 
< HTTP/1.1 401 Unauthorized
< Server: Apache-Coyote/1.1
< WWW-Authenticate: Negotiate
< Content-Type: text/html;charset=utf-8
< Content-Length: 974
< Date: Thu, 16 Apr 2015 12:53:39 GMT
* Rewind stream after send
* Keep sending data to get tossed away!
<
* Ignoring the response-body
{ [974 bytes data]
* We are completely uploaded and fine
* Connection #0 to host <hostname> left intact
* Issue another request to this URL: 'http://<hostname>:8081/webapp/rest/files/2316975_A5E55802418A_2.docx'
* Found bundle for host <hostname>: 0x568820
* Re-using existing connection! (#0) with host <hostname>
* Connected to <hostname> (<server_ip>) port 8081 (#0)
* Server auth using Negotiate with user ''
> PUT /webapp/rest/files/2316975_A5E55802418A_2.docx HTTP/1.1
> Authorization: Negotiate YIIQ1wYG...MOnCd3OA==
> User-Agent: curl/7.40.0
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
> 
< HTTP/1.1 400 Bad Request
< Server: Apache-Coyote/1.1
< Transfer-Encoding: chunked
< Date: Thu, 16 Apr 2015 12:53:39 GMT
< Connection: close
* Keep sending data to get tossed away!
< 
{ [5 bytes data]
* Closing connection 0

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.

@michael-o michael-o changed the title PUT with Expect header sends request body prematurely (upload is borken) PUT with Expect header sends request body prematurely (upload is broken) Apr 16, 2015
@bagder bagder added the HTTP label Apr 16, 2015
@jay
Copy link
Member

jay commented Apr 16, 2015

bisect in Windows VS2010 build shows request body behavior changed in 5dc68dd, 'HTTP: don't abort connections with pending Negotiate authentication'. @tvbuehler

@tvbuehler
Copy link
Contributor

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..)

@bagder
Copy link
Member

bagder commented Apr 17, 2015

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?

@michael-o
Copy link
Contributor Author

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.

@bagder
Copy link
Member

bagder commented Apr 17, 2015

@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 bagder self-assigned this Apr 17, 2015
@michael-o
Copy link
Contributor Author

@bagder, I checked out the 7.40.0 tag from Git, did a git revert on that commit and ran the stuff again on FreeBSD:

*   Trying <server_ip>...
* Connected to <hostname> (<server_ip>) port 8081 (#0)
> PUT /webapp/rest/files/2316975_A5E55802418A_2.pdf HTTP/1.1
> User-Agent: curl/7.40.0-DEV
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
> 
< HTTP/1.1 401 Unauthorized
< Server: Apache-Coyote/1.1
< WWW-Authenticate: Negotiate
< Content-Type: text/html;charset=utf-8
< Content-Length: 974
< Date: Fri, 17 Apr 2015 16:52:45 GMT
< 
* Excess found in a non pipelined read: excess = 974 url = /webapp/rest/files/2316975_A5E55802418A_2.pdf (zero-length body)
* Closing connection 0
* Issue another request to this URL: 'http://<hostname>:8081/webapp/rest/files/2316975_A5E55802418A_2.pdf'
* Hostname <hostname> was found in DNS cache
*   Trying <server_ip>...
* Connected to <hostname> (<server_ip>) port 8081 (#1)
* Server auth using Negotiate with user ''
> PUT /webapp/rest/files/2316975_A5E55802418A_2.pdf HTTP/1.1
> Authorization: Negotiate YIIP7w...YOIrHaV3ZFuk8hD
> User-Agent: curl/7.40.0-DEV
> Host: <hostname>:8081
> Accept: application/json
> Content-Length: 7
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue
[7 bytes data]
* We are completely uploaded and fine
< HTTP/1.1 201 Created
< Server: Apache-Coyote/1.1
< WWW-Authenticate: Negotiate oYHtMIHqo...9iFqt
< Connection: close
< Content-Type: application/json
< Content-Length: 24
< Date: Fri, 17 Apr 2015 16:52:46 GMT
< 
{ [24 bytes data]
* Closing connection 1
{"uid":"x0VVIyjbBv4vwA"}

It does work! Though, I do not understand why curl is throwing away the connection when it receives 401?! I see no reason for.

bagder added a commit that referenced this issue Apr 17, 2015
…cation"

This reverts commit 5dc68dd.

Bug: #223
Reported-by: Michael Osipov
@bagder
Copy link
Member

bagder commented Apr 17, 2015

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.

@bagder
Copy link
Member

bagder commented Apr 17, 2015

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 bagder closed this as completed Apr 17, 2015
@michael-o
Copy link
Contributor Author

@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.

@bagder
Copy link
Member

bagder commented Apr 18, 2015

@michael-o: sure, but I can't promise I'll be able to work on it anytime soon...

@tvbuehler
Copy link
Contributor

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 :(

@bagder
Copy link
Member

bagder commented Apr 20, 2015

@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.

@iboukris
Copy link
Contributor

Hi,

On Mon, Apr 20, 2015 at 2:32 PM, Daniel Stenberg notifications@github.com
wrote:

@tvbuehler https://github.com/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.

And with Proxy it gets even more complicated, RFC 4559 initially states "This
mechanism is not used for HTTP authentication to HTTP proxies." however it
seem to be implemented by MS and others.
It is quite blurry to me what should be the behavior in regard to
persistent-auth and what's the expected behavior when regular server
authentication is also needed.

I found some interesting material to investigate and I'd like to test how
squid behaves but it's currently at the end of my priority list.
See this blog for example:
http://blogs.technet.com/b/isablog/archive/2009/07/30/excessive-authentication-traffic-accessing-an-iis-site-when-using-isa-server-2006-as-forward-proxy.aspx

@michael-o
Copy link
Contributor Author

@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.

@tvbuehler
Copy link
Contributor

@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.

@iboukris
Copy link
Contributor

On Mon, Apr 20, 2015 at 5:34 PM, Stefan Bühler notifications@github.com
wrote:

@michael-o https://github.com/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.

No, in IIS when Kerberos is used underneath it is usually request-based
unless you enable 'authPersistNonNTLM' (maybe the default settings have
changed in recent windows versions).
While when NTLM is used (either in Negotiate or in raw NTLM) it
authenticate the connction by default (unless you
set AuthPersistSingleRequest to true - I've written about it here:
http://curl.haxx.se/mail/lib-2015-02/0174.html).

From security perspectives I agree with Michael that the best way is to
always consider Negotiate as connection based and never re-use unless we
run with the same credentials.

From connectivity perspectives however we can investigate how to guess best
if we are connection or request based in order to decide the best upload
scheme (only NTLM related?) and if we initially add the 'Authorization'
header (which will reset auth with most server side implementations).

For a good guess we need to understand if NTLM was used and try to read
relevant HTTP headers hints (Persistent-Auth).

I am aware that Negotiate is used with Proxies in practice but it doesn't
make the behavior more clear or consistent.

One more note, when authenticating to a server with Negotiate and thru a
proxy we also need to verify a non-standard header for security reasons -
see below from RFC 4559:

If an HTTP proxy is used between the client and server, it must take
care to not share authenticated connections between different
authenticated clients to the same server. If this is not honored,
then the server can easily lose track of security context
associations. A proxy that correctly honors client to server
authentication integrity will supply the "Proxy-support: Session-
Based-Authentication" HTTP header to the client in HTTP responses
from the proxy. The client MUST NOT utilize the SPNEGO HTTP
authentication mechanism through a proxy unless the proxy supplies
this header with the "401 Unauthorized" response from the server.

@michael-o
Copy link
Contributor Author

@Frenche

From security perspectives I agree with Michael that the best way is to
always consider Negotiate as connection based and never re-use unless we
run with the same credentials.

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, mod_spnego (michael-o/mod_spnego) authenticates on the connection while in Apache Tomcat it is always authenticated on the quest.

From connectivity perspectives however we can investigate how to guess best
if we are connection or request based in order to decide the best upload
scheme (only NTLM related?) and if we initially add the 'Authorization'
header (which will reset auth with most server side implementations).
For a good guess we need to understand if NTLM was used and try to read
relevant HTTP headers hints (Persistent-Auth).

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.

The client MUST NOT utilize the SPNEGO HTTP
authentication mechanism through a proxy unless the proxy supplies
this header with the "401 Unauthorized" response from the server.

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.

@iboukris
Copy link
Contributor

On Mon, Apr 20, 2015 at 8:31 PM, Michael Osipov notifications@github.com
wrote:

@Frenche https://github.com/frenche

From security perspectives I agree with Michael that the best way is to
always consider Negotiate as connection based and never re-use unless we
run with the same credentials.

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, mod_spnego (michael-o/mod_spnego) authenticates on
the connection while in Apache Tomcat it is always authenticated on the
quest.

I agree that we can make no assumptions, and conclude the above.

From connectivity perspectives however we can investigate how to guess
best
if we are connection or request based in order to decide the best upload
scheme (only NTLM related?) and if we initially add the 'Authorization'
header (which will reset auth with most server side implementations).
For a good guess we need to understand if NTLM was used and try to read
relevant HTTP headers hints (Persistent-Auth).

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.

These headers are non-standard but if they are there and we can use the to
improve connectivity then why not.

Leaving security aside, if we guess it is likely to be a request-based (non
NTLM and no Persistent-Auth or Persistent-Auth eq false) then we should
re-issue an authorization header and save a round-trip with Kerberos.

The client MUST NOT utilize the SPNEGO HTTP
authentication mechanism through a proxy unless the proxy supplies
this header with the "401 Unauthorized" response from the server.

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.

IMHO for best security we should check this header although non-standard.

@michael-o
Copy link
Contributor Author

@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:

Leaving security aside, if we guess it is likely to be a request-based (non
NTLM and no Persistent-Auth or Persistent-Auth eq false) then we should
re-issue an authorization header and save a round-trip with Kerberos.

  1. HTTP does not encourage preemptive authentication
  2. You should not assume that the next request is going to be protected. Maybe it is just a unprotected resource. Consider that a token from a client could be several thousand bytes long. Outperforming the entire size of the request.
  3. Requests in corporate environments are extremely cheap. We have in most cases less than 20 ms for a roundtrip here.

@iboukris
Copy link
Contributor

On Mon, Apr 20, 2015 at 11:02 PM, Michael Osipov notifications@github.com
wrote:

Though, I do not agree on this for several reasons:

Leaving security aside, if we guess it is likely to be a request-based (non
NTLM and no Persistent-Auth or Persistent-Auth eq false) then we should
re-issue an authorization header and save a round-trip with Kerberos.

  1. HTTP does not encourage preemptive authentication

Any reference? RFC 4559 allows it:

A client may initiate a connection to the server with an
"Authorization" header containing the initial token for the server.
This form will bypass the initial 401 error from the server when the
client knows that the server will accept the Negotiate HTTP
authentication type.

  1. You should not assume that the next request is going to be
    protected. Maybe it is just a unprotected resource. Consider that a token
    from a client could be several thousand bytes long. Outperforming the
    entire size of the request.

If it may be unprotected then OR CURLOPT_HTTPAUTH
with CURLAUTH_GSSNEGOTIATE and CURLAUTH_ONLY.
That's how other authentication protocols behave while your reasons are
valid for them as well.

  1. Requests in corporate environments are extremely cheap. We have in
    most cases less than 20 ms for a roundtrip here.

So they should not have problem with big auth headers :)
Round trips seem to complicate the flow...

@bagder
Copy link
Member

bagder commented Apr 20, 2015

Guys, what about starting over with a fresh issue/pull request and try to work on getting some code done to improve this?

jgsogo pushed a commit to jgsogo/curl that referenced this issue Oct 19, 2015
…cation"

This reverts commit 5dc68dd.

Bug: curl#223
Reported-by: Michael Osipov
@innokenty
Copy link

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.

@bagder
Copy link
Member

bagder commented Sep 22, 2016

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.

@innokenty
Copy link

No, I have 7.43.0, I'll try to upgrade, thanks.

@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

6 participants