Menu

#833 POST + NTLM broken

closed-fixed
http (206)
5
2014-03-13
2009-05-31
Aaron Oneal
No

Using 7.19.5, it is not possible to use NTLM and do a POST against a server like IIS7.

Flow:
1. Request w/ content length but no body! <== bad libcurl! must send body or 0 content length
2. Response 401 authorization required
3. Request (NTLM authorized) w/ content length + body
4. Response 400 bad request

IIS correctly tells libcurl to go away with its authorized request because it didn't finish sending the body from the first request.

I discovered this while trying to do what I would expect is a very common scenario -- use libcurl (via Axis2/C) to send a SOAP request to a web service.

Discussion

1 2 > >> (Page 1 of 2)
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    Also worth mentioning, I'm using MinGW and building as follows:
    mingw32-make -f Makefile.m32 libcurl.a SSL=1 ZLIB=1 SSPI=1 IPV6=1

     
  • Dan Fandrich

    Dan Fandrich - 2009-05-31

    This is a common scenario and it has several tests in the curl test suite (e.g. 267, 553, 1100). How does your scenario differ from those? Are you sure you're not running into libcurl's Expect: 100-continue behaviour, which is desired? Can you post the log of your failing session so we can see exactly what's going on?

     
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    Netmon capture demonstrating issue

     
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    The Expect header is not being sent in the request and the server responds immediately with a 401. I looked at the Axis2/C code and it explicitly sets the header to "Expect:" which I presume is how you tell libcurl not to send the header, though I don't know why they would do that. I'll have a look at those tests to see if I can compare it with what the Axis2/C code is doing and will also attach the relevant file here.

    I have attached a NetMon 3.3 trace which demonstrates the problem.

    You can grab NetMon here if you don't have it handy:
    http://www.microsoft.com/downloads/details.aspx?FamilyID=983b941d-06cb-4658-b7f6-3088333d062f&displaylang=en

     
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    Axis2/C file calling libcurl

     
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    Libcurl verbose output

     
  • Aaron Oneal

    Aaron Oneal - 2009-05-31

    I checked out lib553.c and see it's using CURLOPT_READFUNCTION instead of CURLOPT_POSTFILEDS, but otherwise I don't notice anything out of the ordinary (besides it not using NTLM). The other tests are in a format I'm not familiar with.

    It could be the server the test suite runs against isn't waiting around for the body of the first request once it sends the 401 and so the test is passing.

    You can see from the traces though what's going across the wire and this would seem to indicate a problem.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-01

    isn't this then as test 267? It _does_ send a Content-Length: 0 btw so your case must obviously be different. The question is probably How?

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-02

    I got a debug build going and figured out what's going on. Axis2/C is actually setting a content-length header, and Curl is using it on the first request even though it knows it's not going to send the body. So, that's not right. Here's the problem area starting on line 2774 of http.c (and there are similar problems with PUT)

    case HTTPREQ_POST:
    /* this is the simple POST, using x-www-form-urlencoded style */

    if(conn->bits.authneg)
    postsize = 0;
    else {
    /* figure out the size of the postfields */
    postsize = (data->set.postfieldsize != -1)?
    data->set.postfieldsize:
    (data->set.postfields? (curl_off_t)strlen(data->set.postfields):0);
    }
    if(!data->req.upload_chunky) {
    /* We only set Content-Length and allow a custom Content-Length if
    we don't upload data chunked, as RFC2616 forbids us to set both
    kinds of headers (Transfer-Encoding: chunked and Content-Length) */

    if(!checkheaders(data, "Content-Length:")) {
    /* we allow replacing this header, although it isn't very wise to
    actually set your own */
    result = add_bufferf(req_buffer,
    "Content-Length: %" FORMAT_OFF_T"\r\n",
    postsize);
    if(result)
    return result;
    }
    }

    You can see that it correctly sets the postsize to 0 because of conn->bits.authneg, but because the caller specified content-length, it uses that value instead. Since Curl knows it's not going to send the body, it should remove the caller specified content-length on the first request and use it on the subsequent one when it actually intends to send the body.

     
  • Dan Fandrich

    Dan Fandrich - 2009-06-02

    And now you know why "it isn't very wise to actually set your own" Content-Length header. What's the matter with just setting CURLOPT_POSTFIELDSIZE_LARGE?

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-02

    As I mentioned, this is in the Axis2/C library (not my code), so I don't really know why the particular design was chosen. I don't see how CURLOPT_POSTFIELDSIZE_LARGE would particularly help either. The problem is in sending the pre-auth request when the content-length header is specified (and it still would be with CURLOPT_POSTFIELDSIZE_LARGE), so the two solutions I'm aware of are:
    1. Stop sending the header from Axis2/C
    2. Fix Curl to not do something obviously silly or at least detect the condition and fail with a more informative message

    I filed a bug with the Axis2/C team for #1, and welcome to #2.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-02

    Sending a custom Content-Length: is almost always fishy and somewhat of a weird thing to do. Thus: when an app does it, it should indicate that it truly know what its doing and it needs to do it. It then makes it a whole lot harder for libcurl to make any decisions on exactly what the app wants to achieve. How can libcurl tell the app doesn't want this weirdo request?

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-02

    The caller is instructing libcurl to:
    1. Use NTLM authentication
    2. Send a content length of X
    3. Send a body of Y

    That's a perfectly valid request and not weirdo at all. Libcurl is behaviing incorrectly.

    It either needs to:
    1. Honor the request as the caller made it, send the body, and let the caller handle the 401.
    2. Send the pre-auth request but with 0 content length and then use the caller's length and body information for the subsequent request.

    I prefer #2 as it's consistent with what happens when you don't specify a content-length, and it's just darn conveninent.

    In no case would it be expected behavior for a caller to make a request like above and have libcurl respond by sending a content-length and not body. If that's what the caller wanted, that's what they would have specified.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-02

    suggested fix

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-02

    The "weirdo" part is that the app you're using is _not_ following the 1-2-3 steps. Instead it has decided to use its own header in step (2) that's against our general recommendations and in general is a weird thing to do. I say this. I designed the API. I don't understand why you feel a need to contradict this. They don't "Send a content length of X" the way the API is designed to be used.

    I understand that you want your app to work, but that doesn't defend what they do in their libcurl use.

    The fact that we can (and should?) fix this particular problem doesn't make this a sensible use of libcurl.

    See my suggested fix for this mess in the uplodaed content-length-post-ntlm.patch

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-02

    scrap that patch, was far too simple

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-02

    Thanks to the benefits of open source, I patched my local copy of Axis and have submitted a patch for their review so both products can benefit from the synergy of having a libcurl transport in their web services stack. Point being, my app works and I believe in OSS, so self-interest isn't why I defend their usage.

    My defense centers around caller intent and protocol specifications. Libcurl is not responding in a way the caller can predict. You can qualify it with a disclaimer of, "deviate from our easy 1-2-3 process at your own peril," but I would love to see libcurl be a little more resiliant than that.

    If libcurl doesn't intend to fully support the scenario of setting the content-length and honoring the intent of that request, then it would be great to simply fail when that header is specified so the caller is given clear, enforceable messaging about how the API is intended to be used.

    If libcurl would like to continue to support setting that header, then I hope I've helped the product by raising awareness of an area for improvement. Thanks for your consideration.

     
  • Dan Fandrich

    Dan Fandrich - 2009-06-02

    The problem is that libcurl is sometimes used for wacky purposes, including not-quite-HTTP protocols, broken servers and custom applications. It's not always possible to second guess what the user was actually trying to do and correct it because libcurl might guess wrong and disallow an odd but desired operation. Having said that, it's hard to imagine how this particular behaviour with the Content-Length header would be desired.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-03

    2nd take at suggested cl fix

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-03

    Ok, so what do you think about this fix?

    content-length-post-ntlm-2.patch

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-04

    The POST patch looks good from what I can tell, and my test case passes when applied. PUT also needs to be handled similarly.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-04

    Doesn't the PUT case already work with this? My reading of the code says so.

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-04

    You're right, looks good. No idea what I was looking at before. Thanks!

     
  • Aaron Oneal

    Aaron Oneal - 2009-06-05

    I just discovered what I consider to be a related undesirable behavior. When a caller makes a request and auth negotiation occurs, the request of interest is not the first one that triggers the 401 challenge (since it's an artificial request crafted by libcurl), but the second one. That's why, in the case below, it made sense to use the supplied content length in the second request but not the first.

    The same is true for the response. The caller cares about the response from the authenticated request, not the one that triggered the 401 since that request/response was instigated by curl and not the caller.

    What's happening, however, is that the caller is being notified about headers from that first request for authneg. Register CURLOPT_HEADERFUNCTION and you'll get all the headers from the first request and the second one. This can cause all kinds of problems if, from a caller perspective, you're initiating one request but getting back two status code headers, etc.

    So, I think the larger issue here is that the artificial auth negotiation request is being given too much importance; the caller doesn't need to care or be notified about it. The point is for auth negotiation to be transparent from the caller's perspective, and right now it's not.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-06-05

    Related, yes. But an established behavior since day 1 and not something we just change. If you feel we should introduce a way to inhibit the responses libcurl deal with by itself you should bring that up for discussion on the curl-library list. It is not related to this particular bug.

    This bug report is now closed since I've committed the fix. Thanks a lot!

     
1 2 > >> (Page 1 of 2)