jetty.project icon indicating copy to clipboard operation
jetty.project copied to clipboard

Jetty http client SSL connectivity over CNTLM proxy fails

Open arykov opened this issue 3 years ago • 13 comments

Jetty version(s) 9.4.36.v20210114.

Java version/vendor (use: java -version) 1.8.0_281

OS type/version Windows 10

Description

This is taking place in an environment where internet access can only be performed over a forward proxy(blue coat from what I can tell) that has the following characteristics:

  1. MITM is done by the proxy - it generates certificates using its own CA cert
  2. Requires NTLM authentication In order to deal with the NTLM cntlm chained proxy is being run, so that http client does not need to deal with this. Now when cntlm proxy is used, wget, curl, java in built http client are able to access https and http endpoints Unfortunately Jetty http client does not appear to be able to access https endpoints this way due to SSLHandshakeException caused by ClosedChannelException. Cannot post the full stack trace at the moment Would be great to get troubleshooting

How to reproduce?

Used ubuntu, but any distro should work Setup:

  1. sudo apt install squid
  2. sudo apt install cntlm
  3. Copy contents of attached cntlm.txt into /etc/cntlm.conf(basically listen port is changed from 3128 to 3129 and upstream proxy is changed to be localhost:3128)
  4. sudo service cntlm stop
  5. sudo service cntlm start

Reproduce It works for curl: a) export http_proxy=http://localhost:3129 b)export https_proxy=http://localhost:3129 c) curl https://cnn.com

It fails if using the following code:

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
import org.eclipse.jetty.util.ssl.SslContextFactory;
public class Test{
    public static void main(String[] args) throws Exception {
        System.setProperty("org.eclipse.jetty.http.LEVEL", "DEBUG");
        HttpClient client = new HttpClient(new HttpClientTransportOverHTTP(), new SslContextFactory(true));
        client.getProxyConfiguration().getProxies().add(new HttpProxy(new Origin.Address("localhost", 3129), false));
        client.start();
        System.out.println(client.GET("https://cnn.com").getContentAsString());
    }
	
}

arykov avatar Jun 29 '21 21:06 arykov

The log attached log.txt

arykov avatar Jun 29 '21 21:06 arykov

The logs shows this request:

CONNECT cnn.com:443 HTTP/1.1
Accept-Encoding: gzip
User-Agent: Jetty/9.4.36.v20210114
Host: cnn.com:443

and this response:

HTTP/1.1 200 Connection established
Connection: close

So the server is telling the client that it's closing the connection.

It's a buggy proxy, ask them to not send the Connection: close header in the response -- that's just wrong.

sbordet avatar Jun 29 '21 22:06 sbordet

Does not this mean that the proxy requests close after the exchange is complete? Basically don't send other HTTP requests via the same pipe. Jetty client appears to close right away unlike other clients.

This header appears to be sent back by CNTLM unless upstream proxy sends keep-alive back, which seems incorrect if I read HTTP 1.1. spec right

arykov avatar Jun 30 '21 21:06 arykov

The exchange between client and proxy is completed: a CONNECT with a 200 response.

The client asked for a tunnel, the server is replying 200 with a Connection: close, which does not make sense because why would the tunnel be opened if then the client cannot even send a request through it because the proxy says "tunnel ok, but don't send anything. Bye"?

"Keep-alive" is a thing of 20+ years ago. HTTP/1.1 connections are persistent by default, unless Connection: close is present, which is the case here.

Ignoring the header won't be correct, as the connection cannot be reused ever by the client, so the only option is to close it -- keeping it around would just waste resources.

I assume cntlm could be configured to not behave in this broken way. Have you tried to configure cntlm properly?

sbordet avatar Jun 30 '21 22:06 sbordet

It is not configurable. Culprit code on the line 767 here is 11 years old. This code appears to replicate HTTP/1.0 spec, rather than HTTP/1.1. And removing it indeed allows jetty client to function properly. It is just curious that in 10+ years of using CNTLM jetty client is the first client that I noticed issue. So my instinct was to assume it is the client bug.

arykov avatar Jul 01 '21 01:07 arykov

@arykov the versat/cntlm project seems maintained, perhaps you can open an issue there.

If you have done this scavenging, do you know what for example curl does when it receives a Connection: close for a CONNECT?

sbordet avatar Jul 01 '21 06:07 sbordet

I tried curl, wget and chrome and captured tcpdump.zip (attached all but chrome and there are two curl dumps with -L and not) and they all seem to ignore close(note the "source" port that remains the same).

Further I captured curl log log.zip using curl -v -L https://cnn.com > log 2>&1 and glanced over curl source code: http_proxy.c http.c, connect.h, connect.c and it looks to me that "CONNECT" is handled by http_proxy.c and it does not appear to pay any attention to Connect header(unlike http.c, which does)

arykov avatar Jul 02 '21 02:07 arykov

@arykov I would open an issue anyway to cntlm.

@gregw what do you think?

That a proxy sends an explicit Connection: close in the 200 response to a CONNECT is IMHO a blatant proxy bug.

I would rather push back and have the issue fixed in the proxy, than to introduce a special handling in Jetty's HttpClient to work around some other library bugs. We have a single point in HttpClient where we handle Connection: close, and perhaps it's not too complicated to add an if method == "CONNECT", but it just feels wrong.

sbordet avatar Jul 02 '21 07:07 sbordet

Although I created an issue and pull request for CNTLM, I don't anticipate this fix trickling downstream any time soon. Ubuntu and Redhat/centos packages still reference its old home on the sourceforge which has not been showing too much life since 2012

arykov avatar Jul 02 '21 15:07 arykov

@arykov we will discuss if it's the case for Jetty's HttpClient to ignore Connection: close for CONNECTs -- as I said, I think technically the enhancement is simple, but if we start working around someone else's bugs, we will never end 😃

sbordet avatar Jul 02 '21 21:07 sbordet

@sbordet I've seen it argued that the entire conversation is the body of a CONNECT response, so that if you accept that, then the close applies after the conversation.

I'm not sure I agree, but then I'm not sure I don't. So if the work around is simple....

gregw avatar Jul 03 '21 19:07 gregw

Although I created an issue and pull request for CNTLM, I don't anticipate this fix trickling downstream any time soon. Ubuntu and Redhat/centos packages still reference its old home on the sourceforge which has not been showing too much life since 2012

Maybe you could help getting a newer version version of the fork packages for distributions you use? :) At least a PPA/COPR with newer cntlm packages would be nice. A next step would be seeing if something could be submitted to Debian/Fedora (to get it in turn into Ubuntu/RHEL).

jschwartzenberg avatar Sep 06 '21 06:09 jschwartzenberg

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 07 '22 00:09 github-actions[bot]

Duplicate of #9501, fixed by #9508.

sbordet avatar Mar 16 '23 09:03 sbordet