Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3.2
    • Component/s: HttpClient
    • Labels:

      Description

      Provide support for Server Name Indication (SNI) support as per RFC 3546 (section 3.1).

      Currently attempting to connect to SNI enabled host 'expectedhost' over SSL using http client results in an SSLException similar to:

      javax.net.ssl.SSLException: hostname in certificate didn't match: <expectedhost> != <defaulthost>
      at org.apache.http.conn.ssl.AbstractVerifier.verify(AbstractVerifier.java:220)

      We use SNI on some of our environments and were trying to use httpclient to automatically test host access and availability.

        Activity

        Gus Power created issue -
        Hide
        Oleg Kalnichevski added a comment -

        We happily take contributions.

        Oleg

        Show
        Oleg Kalnichevski added a comment - We happily take contributions. Oleg
        Oleg Kalnichevski made changes -
        Field Original Value New Value
        Fix Version/s Future [ 12312298 ]
        Hide
        Alin Vasile added a comment -

        Can httpclient support this, since Java 1.5 & 1.6 don't support SNI?

        Show
        Alin Vasile added a comment - Can httpclient support this, since Java 1.5 & 1.6 don't support SNI?
        Hide
        Michael Locher added a comment -

        Support SNI on JDK 7 via reflective call to #setHost on concrete the SSLSocket instance.

        sun.net.www.protocol.https.HttpsClient takes the same approach.

        https://github.com/cmbntr/httpclient/commit/1f9898495624dbd10d5c12ae4e0ea9ea50071c85

        Show
        Michael Locher added a comment - Support SNI on JDK 7 via reflective call to #setHost on concrete the SSLSocket instance. sun.net.www.protocol.https.HttpsClient takes the same approach. https://github.com/cmbntr/httpclient/commit/1f9898495624dbd10d5c12ae4e0ea9ea50071c85
        Michael Locher made changes -
        Hide
        Oleg Kalnichevski added a comment -

        Hi Michael

        It is a good quality patch, no doubt. But I am somewhat reluctant to include a fairly substantial piece of JRE specific code through reflection, given it is quite trivial to plug in that code through a custom hostname verifier. If the SNI support is critical enough we should consider writing proper RFC 3546 implementation in order to support all JRE platforms HttpClient can run on.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Hi Michael It is a good quality patch, no doubt. But I am somewhat reluctant to include a fairly substantial piece of JRE specific code through reflection, given it is quite trivial to plug in that code through a custom hostname verifier. If the SNI support is critical enough we should consider writing proper RFC 3546 implementation in order to support all JRE platforms HttpClient can run on. Oleg
        Hide
        Gary Gregory added a comment -

        I'm with Oleg, I'm not crazy about the reflection aspect.

        Show
        Gary Gregory added a comment - I'm with Oleg, I'm not crazy about the reflection aspect.
        Hide
        Josef Johansson added a comment -

        How much of a TLS implementation is done to this day? How much work does 'writing proper RFC 3546 implementation' imply would you say?

        Is it not possible to copy the code in JDK 7 more or less, or is it not license compatible? What is hindering us to do that?

        Is someone looking at this at all? If you see missing SNI as a problem, this is huge because every Android mobile today is lacking it because hc does not support it.

        • Josef
        Show
        Josef Johansson added a comment - How much of a TLS implementation is done to this day? How much work does 'writing proper RFC 3546 implementation' imply would you say? Is it not possible to copy the code in JDK 7 more or less, or is it not license compatible? What is hindering us to do that? Is someone looking at this at all? If you see missing SNI as a problem, this is huge because every Android mobile today is lacking it because hc does not support it. Josef
        Hide
        Oleg Kalnichevski added a comment -

        Josef,
        There is nothing that prevents you from using a custom HostnameVerifier that makes use of SNI extensions when running on Java 7. It probably takes just 1 line of code instead of several dozens when the same call is used reflectively.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Josef, There is nothing that prevents you from using a custom HostnameVerifier that makes use of SNI extensions when running on Java 7. It probably takes just 1 line of code instead of several dozens when the same call is used reflectively. Oleg
        Hide
        John Vasileff added a comment -

        Oleg, thanks, it's good to know that there is an easy work around for developers using the library.

        But that doesn't really help users of all of the apps that use the library. It is unrealistic, even for a developer, to patch or run custom builds of all apps that use HttpClient.

        I'm sure developers of the numerous apps using HttpClient are mostly not aware of SNI issues. Even for the ones that are, requiring Java 7 or including a bit of ugliness and the work involved is being pushed down to thousands of apps rather than being addressed in one place.

        If this affects a fair number of Android apps, I'd have to agree with Josef that this is a huge issue. In my quick research after trying to deploy some internal SNI https sites, it seems most major platforms newer than Windows XP support SNI today. IP addresses are harder and harder to come by, and it sure would be nice to be able to use SNI.

        John

        p.s. - I came across this issue when trying to deploy a maven repository behind SNI/https.

        Show
        John Vasileff added a comment - Oleg, thanks, it's good to know that there is an easy work around for developers using the library. But that doesn't really help users of all of the apps that use the library. It is unrealistic, even for a developer, to patch or run custom builds of all apps that use HttpClient. I'm sure developers of the numerous apps using HttpClient are mostly not aware of SNI issues. Even for the ones that are, requiring Java 7 or including a bit of ugliness and the work involved is being pushed down to thousands of apps rather than being addressed in one place. If this affects a fair number of Android apps, I'd have to agree with Josef that this is a huge issue. In my quick research after trying to deploy some internal SNI https sites, it seems most major platforms newer than Windows XP support SNI today. IP addresses are harder and harder to come by, and it sure would be nice to be able to use SNI. John p.s. - I came across this issue when trying to deploy a maven repository behind SNI/https.
        Hide
        Oleg Kalnichevski added a comment -

        John,
        (1) Security is an application specific aspect. One cannot expect HttpClient to be able to guess the correct settings for all various application types. This is a responsibility of the application developer(s). If you think it is reasonable for Maven to activate SNI per default feel free to raise this issue with Maven developers. I will happily assist them in writing SNI aware HostnameVerifier.
        (2) Whatever we do here will have no bearing on Android. Google refused to upgrade HttpClient and will continue shipping 4.0-alpha.

        Oleg

        Show
        Oleg Kalnichevski added a comment - John, (1) Security is an application specific aspect. One cannot expect HttpClient to be able to guess the correct settings for all various application types. This is a responsibility of the application developer(s). If you think it is reasonable for Maven to activate SNI per default feel free to raise this issue with Maven developers. I will happily assist them in writing SNI aware HostnameVerifier. (2) Whatever we do here will have no bearing on Android. Google refused to upgrade HttpClient and will continue shipping 4.0-alpha. Oleg
        Hide
        Josef Johansson added a comment -

        Hi Oleg,

        Thanks for the info, it is much appreciated.

        Would you like to enlighten me if Google have a reason to not upgrading or
        fixing this issue? It seems that this has been a long standing bug. I can't
        see much reason from what they have written about it.

        Cheers,
        Josef

        Show
        Josef Johansson added a comment - Hi Oleg, Thanks for the info, it is much appreciated. Would you like to enlighten me if Google have a reason to not upgrading or fixing this issue? It seems that this has been a long standing bug. I can't see much reason from what they have written about it. Cheers, Josef
        Hide
        John Vasileff added a comment -

        Oleg,

        My understanding is that SNI's sole purpose is to support multiple https sites on a single IP, and it is not to either increase or decrease the level of security. Sending the domain name over the wire in an SNI scenario is nearly equivalent information to the IP address of the web host in a single web site per IP scenario. If other platforms have standardized on supporting SNI, why shouldn't the Java universe? The world is stuck with one-site-per-IPv4-address until support for SNI is ubiquitous.

        Is the real issue the use of reflection in the offered patch, or a desire to not use SNI by default? If the former, any suggestions to work around this? I haven't looked at the code, but along the lines of what Josef asked, do you have a hunch as to the effort of implementing this without reflection or generally what must be done? Is the argument against reflection performance or aesthetics?

        John

        Show
        John Vasileff added a comment - Oleg, My understanding is that SNI's sole purpose is to support multiple https sites on a single IP, and it is not to either increase or decrease the level of security. Sending the domain name over the wire in an SNI scenario is nearly equivalent information to the IP address of the web host in a single web site per IP scenario. If other platforms have standardized on supporting SNI, why shouldn't the Java universe? The world is stuck with one-site-per-IPv4-address until support for SNI is ubiquitous. Is the real issue the use of reflection in the offered patch, or a desire to not use SNI by default? If the former, any suggestions to work around this? I haven't looked at the code, but along the lines of what Josef asked, do you have a hunch as to the effort of implementing this without reflection or generally what must be done? Is the argument against reflection performance or aesthetics? John
        Hide
        Will Norris added a comment -

        Adding support for SNI has no bearing on how that certificate is validated, with the exception that the lack of SNI support encourages developers to turn of host verification altogether in order to get things to work. Adding support for SNI will in fact increase the ability to have secure applications. And while i'm not intimately familiar with how HttpClient does cert validation, I suspect that simply switching out the hostname verifier is not sufficient, as SNI requires the desired hostname to be specified in the initial handshake.

        Regarding the mention of Android and HttpClient earlier in this thread, see http://android-developers.blogspot.com/2011/09/androids-http-clients.html. Most specifically, the very last line: "New applications should use HttpURLConnection; it is where we will be spending our energy going forward."

        Show
        Will Norris added a comment - Adding support for SNI has no bearing on how that certificate is validated, with the exception that the lack of SNI support encourages developers to turn of host verification altogether in order to get things to work. Adding support for SNI will in fact increase the ability to have secure applications. And while i'm not intimately familiar with how HttpClient does cert validation, I suspect that simply switching out the hostname verifier is not sufficient, as SNI requires the desired hostname to be specified in the initial handshake. Regarding the mention of Android and HttpClient earlier in this thread, see http://android-developers.blogspot.com/2011/09/androids-http-clients.html . Most specifically, the very last line: "New applications should use HttpURLConnection; it is where we will be spending our energy going forward."
        Hide
        John Vasileff added a comment - - edited

        Will - I think your comment about switching out the hostname verifier makes sense. Without SNI, by the time hostname verification is attempted we have already been sent the wrong certificate. So hacking a verifier to claim the wrong certificate is ok would be the wrong thing to do. I am not familiar with the codebase, but it looks like the patch above does the right thing by adding a "HostNameSetter". The web server may have several certificates, it just needs SNI to help it decide which one to send the client.

        Edit - looking at it again, I guess the verifier executes immediately after the HostNameSetter code in the patch anyway. So I guess the setter code could go in a custom verifier.

        Show
        John Vasileff added a comment - - edited Will - I think your comment about switching out the hostname verifier makes sense. Without SNI, by the time hostname verification is attempted we have already been sent the wrong certificate. So hacking a verifier to claim the wrong certificate is ok would be the wrong thing to do. I am not familiar with the codebase, but it looks like the patch above does the right thing by adding a "HostNameSetter". The web server may have several certificates, it just needs SNI to help it decide which one to send the client. Edit - looking at it again, I guess the verifier executes immediately after the HostNameSetter code in the patch anyway. So I guess the setter code could go in a custom verifier.
        Hide
        Oleg Kalnichevski added a comment -

        @Josef
        This is a long and and unpleasant story. Google is most likely going to add SNI to their HttpURLConnection. I have no idea what they are going to do about their fork of HttpClient.

        @John
        It is a combination of both. I do not want to end up having to maintain that particular bit of code when all it takes to enable SNI on Java 7 is one line of code.

        @Will
        You are right. SNI extensions are used during SSL handshake before the session is fully established. My main points still holds, though. SNI is a TLS level aspect. It is a responsibility of individual applications to ensure that they operate in an appropriate security context.

        Oleg

        Show
        Oleg Kalnichevski added a comment - @Josef This is a long and and unpleasant story. Google is most likely going to add SNI to their HttpURLConnection. I have no idea what they are going to do about their fork of HttpClient. @John It is a combination of both. I do not want to end up having to maintain that particular bit of code when all it takes to enable SNI on Java 7 is one line of code. @Will You are right. SNI extensions are used during SSL handshake before the session is fully established. My main points still holds, though. SNI is a TLS level aspect. It is a responsibility of individual applications to ensure that they operate in an appropriate security context. Oleg
        Hide
        John Vasileff added a comment -

        Oleg - the patch is only a few lines of code. This would be a maintenance problem? I agree that as a developer it would be quite easy for me to add SNI support to a Java 7 app of mine. But as a user duplicating this debate with the developers of every app that uses HttpClient is untenable.

        Show
        John Vasileff added a comment - Oleg - the patch is only a few lines of code. This would be a maintenance problem? I agree that as a developer it would be quite easy for me to add SNI support to a Java 7 app of mine. But as a user duplicating this debate with the developers of every app that uses HttpClient is untenable.
        Hide
        John Vasileff added a comment -

        For what it's worth, from the URL Will provided (from 9/2011):

        "We also made several improvements to HTTPS in Gingerbread. HttpsURLConnection attempts to connect with Server Name Indication (SNI) which allows multiple HTTPS hosts to share an IP address..."

        Show
        John Vasileff added a comment - For what it's worth, from the URL Will provided (from 9/2011): "We also made several improvements to HTTPS in Gingerbread. HttpsURLConnection attempts to connect with Server Name Indication (SNI) which allows multiple HTTPS hosts to share an IP address..."
        Hide
        tom chiverton added a comment -

        Oleg,

        What is this simple "1 line of code" that users of the library need to include in order for SNI to work ?

        I still think this is a case of SNI should 'just work', and users of the library shouldn't need to care about it.
        I can't see much of a maintenance burden being added. And it's certainly vastly less work then reimplementing the support Java 1.7 already has for it.

        All of which stops every single user of the library having to add a custom HostVerifier (or whatever) to work around the limitation. In open source projects this may be easy to get done, but in a commercial environment it could just be a total show stopper.

        Show
        tom chiverton added a comment - Oleg, What is this simple "1 line of code" that users of the library need to include in order for SNI to work ? I still think this is a case of SNI should 'just work', and users of the library shouldn't need to care about it. I can't see much of a maintenance burden being added. And it's certainly vastly less work then reimplementing the support Java 1.7 already has for it. All of which stops every single user of the library having to add a custom HostVerifier (or whatever) to work around the limitation. In open source projects this may be easy to get done, but in a commercial environment it could just be a total show stopper.
        Hide
        Oleg Kalnichevski added a comment -

        > What is this simple "1 line of code" that users of the library need to include in order for SNI to work ?

        See patch. In the process of doing so you might get an idea why I am reluctant to include it into HttpClient

        > I still think this is a case of SNI should 'just work', and users of the library shouldn't need to care about it.

        Indeed. So, direct your righteous indignation to Oracle. SNI is a TLS level aspect, not HTTP. It should work completely transparently to the protocol layer on top of TLS.

        Oleg

        Show
        Oleg Kalnichevski added a comment - > What is this simple "1 line of code" that users of the library need to include in order for SNI to work ? See patch. In the process of doing so you might get an idea why I am reluctant to include it into HttpClient > I still think this is a case of SNI should 'just work', and users of the library shouldn't need to care about it. Indeed. So, direct your righteous indignation to Oracle. SNI is a TLS level aspect, not HTTP. It should work completely transparently to the protocol layer on top of TLS. Oleg
        Hide
        tom chiverton added a comment -

        Well, it's not a single line I can ask projects to drop in then; they'd need that new private class too, right ?

        As Oracle are unlikely to fix this, why can't Apache make it easy to use - that's the job of a library, no ?

        Show
        tom chiverton added a comment - Well, it's not a single line I can ask projects to drop in then; they'd need that new private class too, right ? As Oracle are unlikely to fix this, why can't Apache make it easy to use - that's the job of a library, no ?
        Hide
        Oleg Kalnichevski added a comment -

        sun.security.ssl.SSLSocketImpl is public.

        Oleg

        Show
        Oleg Kalnichevski added a comment - sun.security.ssl.SSLSocketImpl is public. Oleg
        Hide
        Graham Leggett added a comment -

        SNI is a non backwards compatible Java 7 feature, and like other new features before it you can either a) use reflection to determine at runtime whether the feature exists or not before trying to use it, or b) produce dedicated binaries for Java 7.

        It is becoming more and more difficult to justify the use of multiple IP addresses on a single box, and in turn, it becomes more and more difficult to justify the continued use of code like httpclient that doesn't support SNI.

        Show
        Graham Leggett added a comment - SNI is a non backwards compatible Java 7 feature, and like other new features before it you can either a) use reflection to determine at runtime whether the feature exists or not before trying to use it, or b) produce dedicated binaries for Java 7. It is becoming more and more difficult to justify the use of multiple IP addresses on a single box, and in turn, it becomes more and more difficult to justify the continued use of code like httpclient that doesn't support SNI.
        Hide
        Oleg Kalnichevski added a comment -

        If that particular method was a part of public APIs this might have been a valid argument, but it is not. There are enough HTTP client libraries around these days. If you do not like HttpClient there are alternatives.

        Oleg

        Show
        Oleg Kalnichevski added a comment - If that particular method was a part of public APIs this might have been a valid argument, but it is not. There are enough HTTP client libraries around these days. If you do not like HttpClient there are alternatives. Oleg
        Hide
        John Vasileff added a comment -

        Let see if I can net this out:

        1) (I think) Anyone who wants can create a custom X509HostnameVerifier that includes

        if (socket instanceof sun.security.ssl.SSLSocketImpl)
        ((sun.security.ssl.SSLSocketImpl)socket).setHost(host);

        But, they would be using an unpublished api (may break later), and would be dropping support for JREs before 7. They would also be using X509HostnameVerifier in a way not supported by the API documentation (setting something in a "verifier" - seems dangerous, even if it would work today.)

        2) Reflection could added to #1 (as in the patch) to help with JRE compatibility. Any arguments against the suggested patch would apply here as well, but to a greater degree: users of HttpClient are a further step removed from the underlying TLS implementation, they are not necessarily socket programming experts, and the funny use of a "verifier" api.

        3) The patch (or similar) could be applied to HttpClient. As in #2, JRE compatibility would be maintained. Leveraging an unpublished sun.x API isn't great, but it shouldn't break anything (if it's not there at runtime, SNI simply won't be performed.) Some dislike reflection, but without it, we can never leverage new features.

        Through all of the prior discussion, I fail to see a concrete reason why the patch would cause problems. It may be aesthetically displeasing to some, but... what are the practical downsides? Pushing the burden of implementing this "TLS fix-up" to users of HttpClient has the practical downsides mentioned above and previously.

        Show
        John Vasileff added a comment - Let see if I can net this out: 1) (I think) Anyone who wants can create a custom X509HostnameVerifier that includes if (socket instanceof sun.security.ssl.SSLSocketImpl) ((sun.security.ssl.SSLSocketImpl)socket).setHost(host); But, they would be using an unpublished api (may break later), and would be dropping support for JREs before 7. They would also be using X509HostnameVerifier in a way not supported by the API documentation (setting something in a "verifier" - seems dangerous, even if it would work today.) 2) Reflection could added to #1 (as in the patch) to help with JRE compatibility. Any arguments against the suggested patch would apply here as well, but to a greater degree: users of HttpClient are a further step removed from the underlying TLS implementation, they are not necessarily socket programming experts, and the funny use of a "verifier" api. 3) The patch (or similar) could be applied to HttpClient. As in #2, JRE compatibility would be maintained. Leveraging an unpublished sun.x API isn't great, but it shouldn't break anything (if it's not there at runtime, SNI simply won't be performed.) Some dislike reflection, but without it, we can never leverage new features. Through all of the prior discussion, I fail to see a concrete reason why the patch would cause problems. It may be aesthetically displeasing to some, but... what are the practical downsides? Pushing the burden of implementing this "TLS fix-up" to users of HttpClient has the practical downsides mentioned above and previously.
        Hide
        Oleg Kalnichevski added a comment -

        I'll take a look at the Java 8 source code at some point and if there is no hope of having a better SNI support in a foreseeable future that does not involve Oracle specific APIs I'll review and commit the patch.

        Oleg

        Show
        Oleg Kalnichevski added a comment - I'll take a look at the Java 8 source code at some point and if there is no hope of having a better SNI support in a foreseeable future that does not involve Oracle specific APIs I'll review and commit the patch. Oleg
        Hide
        John Vasileff added a comment -

        That's great, thanks!

        Show
        John Vasileff added a comment - That's great, thanks!
        Hide
        Oleg Kalnichevski added a comment -

        SNI support in Java 7 and Java 8 looks like a complete bloody mess. The proposed hack would likely work with Java 7 only and in blocking mode only. I found no way of setting a virtual host name on non-blocking SSLEngine in Java 7. As a result with the proposed hack SNI would only work with HttpClient and not with HttpAsyncClient. Quite frankly this sucks. To make matters worse Java 8 has a completely different SNI API provided out of javax.net.ssl package. If we want to make SNI work on Java 8 we will have to resort to yet another reflection hack.

        Oleg

        Show
        Oleg Kalnichevski added a comment - SNI support in Java 7 and Java 8 looks like a complete bloody mess. The proposed hack would likely work with Java 7 only and in blocking mode only. I found no way of setting a virtual host name on non-blocking SSLEngine in Java 7. As a result with the proposed hack SNI would only work with HttpClient and not with HttpAsyncClient. Quite frankly this sucks. To make matters worse Java 8 has a completely different SNI API provided out of javax.net.ssl package. If we want to make SNI work on Java 8 we will have to resort to yet another reflection hack. Oleg
        Hide
        Oleg Kalnichevski added a comment -

        Sebastian, Gary
        Do you have a strong opinion regarding this issue? If do not hear from you I will not commit the patch and will proceed with the 4.2.4 release without it. The resolution of this issue will have to wait until the official release of Java 1.8.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Sebastian, Gary Do you have a strong opinion regarding this issue? If do not hear from you I will not commit the patch and will proceed with the 4.2.4 release without it. The resolution of this issue will have to wait until the official release of Java 1.8. Oleg
        Hide
        Gary Gregory added a comment -

        Release early, release often. Let's not include this now. Another problem is non Oracle JREs. Would this kind of hack work on an IBM JRE on AIX for example?

        We can add it in later or something like it, once it's in, we can't take it out for a long time.

        Show
        Gary Gregory added a comment - Release early, release often. Let's not include this now. Another problem is non Oracle JREs. Would this kind of hack work on an IBM JRE on AIX for example? We can add it in later or something like it, once it's in, we can't take it out for a long time.
        Hide
        Sebb added a comment -

        Don't add this now; seems risky to add the API now when it is likely to have to change.

        Show
        Sebb added a comment - Don't add this now; seems risky to add the API now when it is likely to have to change.
        Hide
        Oleg Kalnichevski added a comment - - edited

        For the record, this is how to enable SNI with HttpClient 4.3 using BeanUtils

        SSLContext sslcontext = SSLContexts.createSystemDefault();
        SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(sslcontext) {
        
            @Override
            public Socket connectSocket(
                    int connectTimeout,
                    Socket socket,
                    HttpHost host,
                    InetSocketAddress remoteAddress,
                    InetSocketAddress localAddress,
                    HttpContext context) throws IOException, ConnectTimeoutException {
                if (socket instanceof SSLSocket) {
                    try {
                        PropertyUtils.setProperty(socket, "host", host.getHostName());
                    } catch (NoSuchMethodException ex) {
                    } catch (IllegalAccessException ex) {
                    } catch (InvocationTargetException ex) {
                    }
                }
                return super.connectSocket(connectTimeout, socket, host, remoteAddress,
                        localAddress, context);
            }
        
        };
        CloseableHttpClient httpclient = HttpClients.custom()
                .setSSLSocketFactory(sslsf)
                .build();
        CloseableHttpResponse response = httpclient.execute(new HttpGet("https://verisign.com/"));
        try {
            System.out.println(response.getStatusLine());
            EntityUtils.consume(response.getEntity());
        } finally {
            response.close();
        }
        

        Oleg

        Show
        Oleg Kalnichevski added a comment - - edited For the record, this is how to enable SNI with HttpClient 4.3 using BeanUtils SSLContext sslcontext = SSLContexts.createSystemDefault(); SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(sslcontext) { @Override public Socket connectSocket( int connectTimeout, Socket socket, HttpHost host, InetSocketAddress remoteAddress, InetSocketAddress localAddress, HttpContext context) throws IOException, ConnectTimeoutException { if (socket instanceof SSLSocket) { try { PropertyUtils.setProperty(socket, "host" , host.getHostName()); } catch (NoSuchMethodException ex) { } catch (IllegalAccessException ex) { } catch (InvocationTargetException ex) { } } return super .connectSocket(connectTimeout, socket, host, remoteAddress, localAddress, context); } }; CloseableHttpClient httpclient = HttpClients.custom() .setSSLSocketFactory(sslsf) .build(); CloseableHttpResponse response = httpclient.execute( new HttpGet( "https: //verisign.com/" )); try { System .out.println(response.getStatusLine()); EntityUtils.consume(response.getEntity()); } finally { response.close(); } Oleg
        Hide
        Bruno Harbulot added a comment -

        (I've just seen the initial patch: it's quite clearly confusing SNI with host name verification. As it as already been said, SNI happens at the TLS level and has nothing to with host name verification: making the error disappear by tweaking the host name verifier is only a side-effect.)

        A simpler way to have SNI support would be to use the socket methods that use the name instead of the address.

        The following code (which doesn't rely on reflection tricks and would work on Java 6) would enable SNI with Java 7 (but not Java 6, of course):

        javax.net.ssl.SSLSocketFactory ssf = (SSLSocketFactory) SSLSocketFactory.getDefault();
        SSLSocket socket = (SSLSocket) ssf.createSocket("www.google.com", 443);
        

        The following code would not:

        SSLSocket socket = (SSLSocket) ssf.createSocket(InetAddress.getByName("www.google.com"), 443);
        

        When using an InetAddress, the host name information is lost. This is what prevents SNI from being used.

        Perhaps letting the socket API do the DNS resolution whenever possible would be a simpler way to fix this altogether.

        Show
        Bruno Harbulot added a comment - (I've just seen the initial patch: it's quite clearly confusing SNI with host name verification. As it as already been said, SNI happens at the TLS level and has nothing to with host name verification: making the error disappear by tweaking the host name verifier is only a side-effect.) A simpler way to have SNI support would be to use the socket methods that use the name instead of the address. The following code (which doesn't rely on reflection tricks and would work on Java 6) would enable SNI with Java 7 (but not Java 6, of course): javax.net.ssl.SSLSocketFactory ssf = (SSLSocketFactory) SSLSocketFactory.getDefault(); SSLSocket socket = (SSLSocket) ssf.createSocket( "www.google.com" , 443); The following code would not : SSLSocket socket = (SSLSocket) ssf.createSocket(InetAddress.getByName( "www.google.com" ), 443); When using an InetAddress , the host name information is lost. This is what prevents SNI from being used. Perhaps letting the socket API do the DNS resolution whenever possible would be a simpler way to fix this altogether.
        Hide
        Oleg Kalnichevski added a comment -

        @Bruno: InetAddress retains the original host name it was resolved with. I do not think that the way the socket is created should have any bearing on SNI.

        Oleg

        Show
        Oleg Kalnichevski added a comment - @Bruno: InetAddress retains the original host name it was resolved with. I do not think that the way the socket is created should have any bearing on SNI. Oleg
        Hide
        Bruno Harbulot added a comment -

        @Oleg:

        Indeed, in principle, it could work using the host from InetAddress, but it doesn't. I guess it's there to capture the intent. InetAddress.getHostName() does retain the host name, but it also does a reverse lookup if it came from an IP address, which may not have been the intent.

        Try the two snippets of code above and look at the Client Hello message with Wireshark, you'll see only the first one has the server_name extension.

        If you look at the SSLSocketImpl source code, only the methods that have String host parameters set the internal host name used for SNI.

        (Similarly, for SSLEngine, you need to create it with SSLContext.createSSLEngine(String peerHost, int peerPort) instead of SSLContext.createSSLEngine(): the host name and port are just used as an indication and are not necessarily linked to what's actually used for the connection by the channels.)

        Show
        Bruno Harbulot added a comment - @Oleg: Indeed, in principle, it could work using the host from InetAddress , but it doesn't. I guess it's there to capture the intent. InetAddress.getHostName() does retain the host name, but it also does a reverse lookup if it came from an IP address, which may not have been the intent. Try the two snippets of code above and look at the Client Hello message with Wireshark, you'll see only the first one has the server_name extension. If you look at the SSLSocketImpl source code , only the methods that have String host parameters set the internal host name used for SNI. (Similarly, for SSLEngine , you need to create it with SSLContext.createSSLEngine(String peerHost, int peerPort) instead of SSLContext.createSSLEngine() : the host name and port are just used as an indication and are not necessarily linked to what's actually used for the connection by the channels.)
        Hide
        Bruno Harbulot added a comment -

        Here is a patch to support SNI (when using Java 7) with Apache HttpClient 4.2.x. This doesn't require any changes from the user's point of view, doesn't use any reflection or any code specific to Java 7 (it will simply not use SNI with a JRE that doesn't support it).

        Here is a bit of background. To get client-side support with Java 7 (at least with JREs that are based on the OpenJDK), the SSLSocket must be created using one of the createSocket methods that take use the String host (not the InetAddress host) parameter.

        In particular, this causes problems, because of the way HttpClient first creates the (non-connected) socket, changes some of its settings, and only connects it later.

        This patch addresses this problem by creating a normal Socket in all cases, thereby allowing HttpClient to make any setting to the socket before connection (timeout, local address re-use, ...), and then make use of SSLSocketFactory.createSocket(Socket s, String host, int port, boolean autoClose), which will make use of SNI when available.

        I had made a first attempt to change this by re-ordering some of the content of the connectSocket method in HttpClient's SSLSocketFactory. This worked, but was unsatisfactory because this would prevent some parameters to be set before connect the connection (this would affect the timeout setting before connection as well as the ability to use sock.setReuseAddress when the local address needs to be re-used).

        Show
        Bruno Harbulot added a comment - Here is a patch to support SNI (when using Java 7) with Apache HttpClient 4.2.x. This doesn't require any changes from the user's point of view, doesn't use any reflection or any code specific to Java 7 (it will simply not use SNI with a JRE that doesn't support it). Here is a bit of background. To get client-side support with Java 7 (at least with JREs that are based on the OpenJDK), the SSLSocket must be created using one of the createSocket methods that take use the String host ( not the InetAddress host ) parameter. In particular, this causes problems, because of the way HttpClient first creates the (non-connected) socket, changes some of its settings, and only connects it later. This patch addresses this problem by creating a normal Socket in all cases, thereby allowing HttpClient to make any setting to the socket before connection (timeout, local address re-use, ...), and then make use of SSLSocketFactory.createSocket(Socket s, String host, int port, boolean autoClose) , which will make use of SNI when available. I had made a first attempt to change this by re-ordering some of the content of the connectSocket method in HttpClient's SSLSocketFactory . This worked, but was unsatisfactory because this would prevent some parameters to be set before connect the connection (this would affect the timeout setting before connection as well as the ability to use sock.setReuseAddress when the local address needs to be re-used).
        Bruno Harbulot made changes -
        Attachment apache_httpclient_4.2.x_sni.patch [ 12608703 ]
        Hide
        Oleg Kalnichevski added a comment -

        I don't know. This only makes SNI support in Java 7 look shoddier to me. Anyway I am glad the problem can be worked around this way though I do not quite understand why creating a plain socket and then laying SSL over it should behave differently than just creating a SSLSocket directly (which internally should be doing the same anyway). I would happily commit the patch if you could port and re-test it with HC 4.3.

        Oleg

        Show
        Oleg Kalnichevski added a comment - I don't know. This only makes SNI support in Java 7 look shoddier to me. Anyway I am glad the problem can be worked around this way though I do not quite understand why creating a plain socket and then laying SSL over it should behave differently than just creating a SSLSocket directly (which internally should be doing the same anyway). I would happily commit the patch if you could port and re-test it with HC 4.3. Oleg
        Hide
        Bruno Harbulot added a comment -

        I agree this is not very well documented, but I think this makes sense.

        I think they just require the host name to be passed as a String, to prevent problems when trying to get the host name from InetAddress. Indeed, InetAddress.getHostName() doesn't necessarily return the initial argument to InetAddress.getByName(...) (for example, if it's an IP address in text form, which is allowed according to the documentation).

        Since there is no direct way to set the host once the socket is created (in the public API of SSLSocket), this seemed to have been a way to implement SNI in a way that disrupted the public API the least.

        Unfortunately, all of the javax.net.ssl.SSLSocketFactory.createSocket(...) methods that create a socket using a String parameter to pass the host name also connect this socket immediately, except the one that overlays the SSLSocket on top of a plain Socket.

        Since Apache HttpClient uses its sockets in such a way that they cannot be created and connected in the same call to the JRE's socket factory, using a plain socket first makes this work, because it's the only way to make use of a suitable j.n.s.SSLSocketFactory.createSocket(...) method, after the socket creation.

        Another way to achieve this would be to change the Apache HttpClient API in such a way that it doesn't have to rely on separate calls to createSocket and connectSocket of its own socket factories, but this would certainly be a more disruptive change in Apache HttpClient.

        I'll try this patch with HC 4.3 shortly.

        Show
        Bruno Harbulot added a comment - I agree this is not very well documented, but I think this makes sense. I think they just require the host name to be passed as a String , to prevent problems when trying to get the host name from InetAddress . Indeed, InetAddress.getHostName() doesn't necessarily return the initial argument to InetAddress.getByName(...) (for example, if it's an IP address in text form, which is allowed according to the documentation). Since there is no direct way to set the host once the socket is created (in the public API of SSLSocket ), this seemed to have been a way to implement SNI in a way that disrupted the public API the least. Unfortunately, all of the javax.net.ssl.SSLSocketFactory.createSocket(...) methods that create a socket using a String parameter to pass the host name also connect this socket immediately, except the one that overlays the SSLSocket on top of a plain Socket . Since Apache HttpClient uses its sockets in such a way that they cannot be created and connected in the same call to the JRE's socket factory, using a plain socket first makes this work, because it's the only way to make use of a suitable j.n.s.SSLSocketFactory.createSocket(...) method, after the socket creation. Another way to achieve this would be to change the Apache HttpClient API in such a way that it doesn't have to rely on separate calls to createSocket and connectSocket of its own socket factories, but this would certainly be a more disruptive change in Apache HttpClient. I'll try this patch with HC 4.3 shortly.
        Hide
        Oleg Kalnichevski added a comment -

        InetAddress retains the original hostname it was created with. Oracle is not restricted to using public APIs. There is nothing that prevents them from obtaining the original hostname without triggering undesired reverse lookups (which I believe is the rationale given for the present bizarre behavior) .

        There is a reason for having #createSocket and #connectSocket as separate methods: it enables HttpClient to unblock #connectSocket operation by shutting down the underlying socket. There is also reason for using InetAddress to represent the target of an outgoing connection.

        Oleg

        Show
        Oleg Kalnichevski added a comment - InetAddress retains the original hostname it was created with. Oracle is not restricted to using public APIs. There is nothing that prevents them from obtaining the original hostname without triggering undesired reverse lookups (which I believe is the rationale given for the present bizarre behavior) . There is a reason for having #createSocket and #connectSocket as separate methods: it enables HttpClient to unblock #connectSocket operation by shutting down the underlying socket. There is also reason for using InetAddress to represent the target of an outgoing connection. Oleg
        Hide
        Oleg Kalnichevski added a comment -

        Patch committed to SVN trunk with some minor tweaks. Many thanks, Bruno, for contributing it.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Patch committed to SVN trunk with some minor tweaks. Many thanks, Bruno, for contributing it. Oleg
        Oleg Kalnichevski made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 4.3.2 [ 12325245 ]
        Fix Version/s Future [ 12312298 ]
        Resolution Fixed [ 1 ]
        Hide
        pavan added a comment -

        Hi,

        When is the targeted release date for this patch 4.3.2 ?

        --Thanks

        Show
        pavan added a comment - Hi, When is the targeted release date for this patch 4.3.2 ? --Thanks
        Hide
        Oleg Kalnichevski added a comment -

        Jan 2014.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Jan 2014. Oleg

          People

          • Assignee:
            Unassigned
            Reporter:
            Gus Power
          • Votes:
            11 Vote for this issue
            Watchers:
            17 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development