HTTP/1.1 connection reuse not seeming to work

[kinnison]

I did this

I have a multi-handle into which I add easy handles duphandle()d from a common configured handle. I make a request to localhost, which completes as a keepalive response requesting authentication. From that I get * Connection #1 to host localhost left intact. The easy handle
is thence cleaned up, but the multi handle persists.

Then my client attempts to make another request to the same place and instead of it reusing that connection it ends up attempting to make a fresh connection. I get * Found bundle for host localhost: 0x556e73066cf0 [can pipeline] and * Hostname localhost was found in DNS cache but eventually * Connected to localhost (127.0.0.1) port 2345 (#2)

I expected the following

I expected to see connection 1 reused.

curl/libcurl version

somniloquy% curl -V                                                                                                                                              
curl 7.64.0 (x86_64-pc-linux-gnu) libcurl/7.64.0 OpenSSL/1.1.1d zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.36.0 librtmp/2.3
Release-Date: 2019-02-06
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy PSL 
somniloquy%                                                                                                                                                      

operating system

Debian/Buster

Additional notes

The code driving cURL can be found at https://git.netsurf-browser.org/netsurf.git/tree/content/fetchers/curl.c though I appreciate that's a lot of code to look through, if you want me to point you at particular parts, just let me know.

[bagder]

I wrote up this minimal example (4499.c) trying to reproduce this, but failed. This example adds two transfers, the second after the first transfer has completed, and the second one reuses the connection fine for me according to the verbose output.

So can you adjust this example so that it reproduces the claims of this report?

[kinnison]

Thank you for doing that so quickly. I shall try that against my test case here and see what adjustments might be needed to make it replicate my issue.

[kinnison]

I'm sorry it has taken so long to get back to you -- I'm having a lot of difficulty getting your example to replicate the issue. I'm attaching what log I do have from NetSurf running: netsurf-log.txt, a log from the program you provided (and I hacked): 4499-log.txt and my rewritten C file: 4499.c.txt. I tried to hack it into a shape similar to how NetSurf uses handles (including the multi handle, a base easy handle, and duping from there). I haven't added all the SSL stuff in though if you think that might help, I'd appreciate guidance.

I appreciate that it's incredibly hard to help diagnose this kind of thing -- if there's any suggestion as to extra debug I can turn on in libcurl to understand why it chooses not to reuse the handle under NetSurf then I can try that for you too.

[regregex]

It's failing to reuse if CURLOPT_HTTPAUTH is set to CURLAUTH_ANY, and the user/pass has not previously been tried on that connection, including going from authentication to no authentication. I've edited your 4499.c to demonstrate the bug, and so far it hasn't timed out.

The first response code doesn't matter, except to cause the UA to try a(nother) login in the first place. A server need only be keep-alive and single-threaded, and cURL will lock up.

[kinnison]

@regregex Thank you so much, I completely missed that bit. I do have a sample server which behaves like that (which came from a bug report to NetSurf) --
single-threaded-server.pl.txt

[bagder]

So this latest edit now reproduces the problem for you? Can you elaborate on how your test server on localhost should act authorization wise as you ask for auth now. Should the server require auth (if so, which?) or does it repro even if it doesn't do any auth at all? I just want to rerun the test case as good as possible!

[kinnison]

Sure, the test server was provided by a reporter of this issue to NetSurf -- you run it and it binds localhost:2345 -- any request which passes in any authentication token is let through (no specific user/pass needed), but any non-authenticated request is responded to with a 401. If you take the new 4499.c change the url to include the port number, and run it, then it reproduces the issue for me when running against that perl script. I'm sure you could write a smaller simpler test case but that's the one supplied to me. The critical thing is that while servicing a keepalive connection, the server doesn't get back to its accept() call and so subsequent attempts to connect to it by curl are unserviced, blocking the client.

[bagder]

Thanks with this setup I can reproduce the issue.

The problem for curl here is that with allowing CURLAUTH_ANY auth, that includes NTLM. With NTLM you authenticate connections and not individual requests. So in this case curl sees "wants NTLM" and then it sees that the credentials mismatch (this request vs the ones used that created the existing connection) and avoids reusing the existing connection.

Now, obviously the previous connection hasn't been authenticated with NTLM so this is a over-cautious check here and I'll see what I can do to fix this. What complicates the matter somewhat is that in negotiating an NTLM authenticated connection, there's always a 401 response first...

[kinnison]

Aha, that sounds quite sensible for curl to do. But perhaps the answer is that check needs to be against auth data types sent on the channel, rather than mechanisms requested? So a channel which has had a 401 already but not yet sent NTLM data could be reused by a non-NTLM request?

It also sounds like for now, a mitigation strategy for me will be to actually restrict that bitfield from CURLAUTH_ANY to the set of authentication types we actually want to see supported (which for now is probably only CURLAUTH_BASIC and CURLAUTH_BEARER.

If there's anything else I can do to help you with deciding the right fix, please yell. Otherwise I'll try that mitigation strategy and let you know if it doesn't work for us.

[bagder]

If you set only CURLAUTH_BASIC | CURLAUTH_BEARER you should see that you work around this issue nicely.

But yes, #4514 is a PR to address this issue by making the check a little smarter.