Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for javax.net.ssl.HostnameVerifier to HttpClient #3154

Closed
joakime opened this issue Nov 27, 2018 · 7 comments
Closed

Add support for javax.net.ssl.HostnameVerifier to HttpClient #3154

joakime opened this issue Nov 27, 2018 · 7 comments

Comments

@joakime
Copy link
Contributor

joakime commented Nov 27, 2018

Currently javax.net.ssl.HostnameVerifier is a way to perform Hostname verification within the following libraries (but not Jetty's HttpClient) ...

Java itself

  HostnameVerifier hv = new TrustAllHostnameVerifier();
  HttpsURLConnection.setDefaultSSLSocketFactory(sc.getSocketFactory());

OkHttp

  SslClient sslClient = SslClient.localhost();
  SSLSocketFactory socketFactory = sslClient.socketFactory;
  HostnameVerifier hostnameVerifier = new HostnameVerifier() {
    @Override public boolean verify(String s, SSLSession session) {
      return true;
    }
  };

  client = new OkHttpClient.Builder()
        .sslSocketFactory(socketFactory, sslClient.trustManager)
        .hostnameVerifier(hostnameVerifier)
        .build();

Apache HttpClient 3.x

  HostnameVerifier hostnameVerifier = SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER;
  SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(builder.build(), hostnameVerifier);
  CloseableHttpClient httpclient = HttpClients.custom().setSSLSocketFactory(sslsf).build();

Apache HttpClient 4.x

  HttpClientBuilder b = HttpClientBuilder.create();
  SSLContext sslContext = new SSLContextBuilder().loadTrustMaterial(null, new TrustStrategy() {
         public boolean isTrusted(X509Certificate[] arg0, String arg1) throws CertificateException {
            return true;
         }
      }).build();
  b.setSslcontext(sslContext);
  HostnameVerifier hostnameVerifier = SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER;
  SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContext, hostnameVerifier);
  PoolingHttpClientConnectionManager connMgr = new PoolingHttpClientConnectionManager(RegistryBuilder.<ConnectionSocketFactory> create()
    .register("http", PlainConnectionSocketFactory.getSocketFactory()).register("https", sslSocketFactory).build());
  b.setConnectionManager(connMgr);
  HttpClient client = b.build();

Investigate if we should offer this in Jetty's HttpClient as well.

@nikhilahuja
Copy link

I was looking for it. It would be really helpful if this could be added as an easy-to-use built-in feature just like Apache HttpClient. We have our own implementation of HostnameVerifier (it takes into consideration various options configured by the user) that we were using with Apache HttpClient for HTTP/1.1 support. Since the current implementation of Apache HttpClient with HTTP/2 support (version 5.0) is still in beta, we decided to go with Jetty.

A simple way I would have liked to use it would have been:

final SslContextFactory sslContextFactory = new SslContextFactory();
sslContextFactory.setHostnameVerifier(new HostnameVerifier() {
    @Override
     public boolean verify(String s, SSLSession sslSession) {
         // My code for verification
     }
});

final HttpClientTransport transport = new HttpClientTransportOverHTTP2(new HTTP2Client());
final HttpClient httpClient = new HttpClient(transport, sslContextFactory);

@sbordet
Copy link
Contributor

sbordet commented Feb 5, 2019

Can I ask what is the use case here?

You want to allow certain hosts that don't match the certificate and at the same time disallow other hosts that also don't match the certificate?

I ask because in the expression (allow some && disallow others) the first half alone (before the &&) and the second half alone (after the &&) can already be done.

Can you detail a case where you need both?
I have an implementation almost ready, but I want to verify that your particular case will be covered.

@sbordet
Copy link
Contributor

sbordet commented Feb 5, 2019

Would be great if you guys can test the code with this issue fixed and report if it works fine for you.

@nikhilahuja
Copy link

@sbordet thanks for working on this. My use case was that we have some properties exposed to the users of our application like whether to allow wildcard certificates or not and looking at SubjectAlternativeNames (SAN) to match hostname etc. All these global configurations are taken into account by out HostnameVerifier implementation.

We were able to accomplish this by 2 ways:

  1. Using SslHandshakeListener - the issue with this was that this Listener gets call only for new SSL connections. So if our global property value changes, those were not being applied for existing active connections.
  2. By overriding the acquire() method of MultiplexConnectionPool - using this solved the problem.

Also, throwing the exception in Listener or in the acquire() method if verification failed didn't solve the problem as it holds the Main thread until the request timed out. we have to abort the HttpExchange instead. Like:

if (!hostnameVerifier.verify(destination.getHost(), sslSession)) {
    sslSession.invalidate();
    sslEngine.closeOutbound();
    try {
        sslEngine.closeInbound();
    } catch (SSLException e) {
        LOGGER.log(Level.FINE, "Error closing SSL inbound connection", e);
    }
    final Throwable cause = new SSLKeyException("SSL hostname verification failed!");
    destination.getHttpExchanges().forEach(exchange -> exchange.getRequest().abort(cause));
}

@nikhilahuja
Copy link

@sbordet Sure, will test out my use case and let you know.

@sbordet
Copy link
Contributor

sbordet commented Feb 5, 2019

The current work on commit 8964608 calls the verifier only at the moment the connection is established.

For your case, when the connection is not good for your settings, have you tried to close the connection and return null from acquire()?
This would trigger the opening of a new connection that you can check again with the verifier, and should be much simpler.

@nikhilahuja
Copy link

If we return null then will it not go into the loop because new connection might not be a valid one as well and then the Main thread will just wait for response until it gets timed out.

Will try this out as well and get back to you on yow was the behaviour.

sbordet added a commit that referenced this issue Feb 6, 2019
…lient.

Added javadocs after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet closed this as completed in 8964608 Feb 7, 2019
sbordet added a commit that referenced this issue Feb 7, 2019
Fixes #3154 - Add support for javax.net.ssl.HostnameVerifier to HttpClient
sbordet added a commit that referenced this issue Feb 7, 2019
…lient.

Fixed compilation errors after merge.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants