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

Jetty http client SSL connectivity over CNTLM proxy fails #6483

Closed
arykov opened this issue Jun 29, 2021 · 14 comments
Closed

Jetty http client SSL connectivity over CNTLM proxy fails #6483

arykov opened this issue Jun 29, 2021 · 14 comments
Labels
Bug For general bugs on Jetty side

Comments

@arykov
Copy link

arykov commented Jun 29, 2021

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 arykov added the Bug For general bugs on Jetty side label Jun 29, 2021
@arykov
Copy link
Author

arykov commented Jun 29, 2021

The log attached
log.txt

@arykov arykov changed the title Jetty http client SSL connectivity over blue coat proxy fails Jetty http client SSL connectivity over specific corporate proxy fails Jun 29, 2021
@sbordet
Copy link
Contributor

sbordet commented Jun 29, 2021

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.

@arykov
Copy link
Author

arykov commented Jun 30, 2021

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

@sbordet
Copy link
Contributor

sbordet commented Jun 30, 2021

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?

@arykov
Copy link
Author

arykov commented Jul 1, 2021

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.

@sbordet
Copy link
Contributor

sbordet commented Jul 1, 2021

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

@arykov arykov changed the title Jetty http client SSL connectivity over specific corporate proxy fails Jetty http client SSL connectivity over CNTLM proxy fails Jul 2, 2021
@arykov
Copy link
Author

arykov commented Jul 2, 2021

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)

@sbordet
Copy link
Contributor

sbordet commented Jul 2, 2021

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

@arykov
Copy link
Author

arykov commented Jul 2, 2021

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

@sbordet
Copy link
Contributor

sbordet commented Jul 2, 2021

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

@gregw
Copy link
Contributor

gregw commented Jul 3, 2021

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

@jschwartzenberg
Copy link

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

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

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 github-actions bot added the Stale For auto-closed stale issues and pull requests label Sep 7, 2022
@sbordet sbordet removed the Stale For auto-closed stale issues and pull requests label Sep 24, 2022
sbordet added a commit that referenced this issue Mar 16, 2023
Now Connection: close is ignored for 2xx responses to a CONNECT method.
In this way the tunnel is kept open, and bad proxies that were sending
Connection: close are now supported as apparently they are still out there.

Fixes also #6483.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor

sbordet commented Mar 16, 2023

Duplicate of #9501, fixed by #9508.

@sbordet sbordet closed this as completed Mar 16, 2023
sbordet added a commit that referenced this issue Mar 20, 2023
Now Connection: close is ignored for 2xx responses to a CONNECT method.
In this way the tunnel is kept open, and bad proxies that were sending
Connection: close are now supported as apparently they are still out there.

Fixes also #6483.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

4 participants