[ 
https://issues.apache.org/jira/browse/HTTPCLIENT-1977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16811938#comment-16811938
 ] 

Dmitry Andrianov commented on HTTPCLIENT-1977:
----------------------------------------------

Oleg,

thank you very much for the explanation. I see your point and I guess it makes 
my report invalid.

Having said that, it was a bit of unexpected behaviour from the HttpClient 
because we are using the same client certificate on the second connection and 
this certificate is known before the request is even sent so one would expect 
this will be matched.

By using contexts I effectively have to manage connection identities manually 
by telling HttpClient that these two requests can go over the same connection 
while those - can not. And I kinda hoped it can be just automatic given all the 
information is available before the request.

 

I guess it just does not fit into the existing architecture. Not asking to do 
anything about it, just tried to explain my expectations.

 

Cheers

> HttpClient does not use Keep-Alive if client SSL auth is used
> -------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1977
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1977
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>            Reporter: Dmitry Andrianov
>            Priority: Major
>
> When client is configured with a certificate for SSL auth, the connection 
> pool stops working properly so each request uses a new connection.
> This happens because   when {{AbstractConnPool.getPoolEntryBlocking()}} 
> method is invoked, the state is null so this code
> {code:java}
> entry = pool.getFree(state);
> if (entry == null) {
>     break;
> }{code}
> gets null back. The {{getFree}} method only returns connection with the same 
> state. So when new connection is opened, it kinda gets null state too but 
> later after the request, {{MainClientExec}} does this:
> {code:java}
> if (userToken != null) {
>     connHolder.setState(userToken);
> }{code}
> and this eventually sets state on the pool entry to the subject of client 
> certificate.
> When another request is made - the story repeats, since 
> {{AbstractConnPool.getPoolEntryBlocking()}} uses null state, it cannot find 
> matching connection in the pool (because the only one connection there HAS 
> state now) and a new connection is created. In the end pooled HTTPS 
> connections are never reused.
>  
> This is the code to reproduce:
> {code:java}
> import com.alertme.zoo.hubregistration.util.PemUtils;
> import org.apache.http.client.config.RequestConfig;
> import org.apache.http.client.methods.CloseableHttpResponse;
> import org.apache.http.client.methods.HttpGet;
> import org.apache.http.config.Registry;
> import org.apache.http.config.RegistryBuilder;
> import org.apache.http.conn.socket.ConnectionSocketFactory;
> import org.apache.http.conn.ssl.NoopHostnameVerifier;
> import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
> import org.apache.http.impl.client.CloseableHttpClient;
> import org.apache.http.impl.client.HttpClients;
> import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
> import org.apache.http.ssl.SSLContextBuilder;
> import org.apache.http.ssl.SSLContexts;
> import org.apache.http.util.EntityUtils;
> import java.security.KeyStore;
> import java.security.PrivateKey;
> import java.security.cert.X509Certificate;
> public class Test {
>     // openssl req -newkey rsa:2048 -nodes -keyout test-client.key -x509 
> -days 3650 -out test-client.crt -subj '/CN=test-client/'
>     private final static String KEY_PEM = "" +
>             "-----BEGIN PRIVATE KEY-----\n" +
>             
> "MIIEwAIBADANBgkqhkiG9w0BAQEFAASCBKowggSmAgEAAoIBAQC+Kzo5Ehd1vrbe\n" +
>             
> "Za+rNIqnIzWAzKr/iWFySQnGQu5SpK1nsZ5JhBK83J4RQxRS6mULrifJgG/AqMlA\n" +
>             
> "0NmCDw0C6p/uQF+VKycntULjnuvwjU04lkTmrWk3twHnvDkd+j2SH+jfzjZePv+l\n" +
>             
> "XCCQLQS5xiMreGjMx4EILUsB00EQoUFbdGg1NgTt7g8L/WuEDC9YybC6FJHOBYP/\n" +
>             
> "02wmW4REYyMuVY3SfZAsOk5YRbEf25c7IAA3+tbaXz9SuRgUK3SKI0ReGBtLyvSj\n" +
>             
> "gyoyOTwT89QOdEsL6KM9TZhjJGpmeXGMHBNP7uEXuXgfB6wKay1ejD6hr+lcgnIC\n" +
>             
> "QqHthASrAgMBAAECggEBAJw6Rwq7oipJE1KBl1+/Omk0s6+sdI6Z/kQ1XKJUOhYK\n" +
>             
> "06pscO1UY1BkrjbgNMIpbfm6iVUw/p34C94DtazzUG0k81535A5X9ULZ1qnI1Ww5\n" +
>             
> "qUbjrJcVv2rWHeqS5xmJiyuQq2+xqVijyMHAfb/0O/2imSINOYuCGq7tBsHpG3rc\n" +
>             
> "k8fpB7Q43+aZRnQNN6F12GwEH/vTSkhbWABSUzLjGxNc1JzArGqhp8Fm2UxuZ+gG\n" +
>             
> "rtfnCQp3AaYrZa1SAxFnA7cfSWiLYv6gv2ejpekbN9Z/iff71in6a0wNOMofbvOv\n" +
>             
> "wLuIuorjHynpc21brJs09Rx4uxJv3kyC8keoWcM22oECgYEA7BwqquZbRAE4EmNM\n" +
>             
> "vWSeXeMnMfQy9u786WoXv3PW7J6ftniKf6f0ZaIdHJv4ilgFf6Xk5ap8e4S2x21W\n" +
>             
> "ciTH/u34O/UTmQ7t5UkGscZzcCldO4vPoU1531md4hXtMcY8W02lRfMtvV9SmlR0\n" +
>             
> "I39eJU0NLb70KgQZ060FUNnL8c0CgYEAzjBQ5IA51KGUOQA81cPrhuUat1/sExgQ\n" +
>             
> "lxsfn5ZW4S74G7fl8W5n6LfX1kZryj11XYEx0Vv3kPuFQ6pjjyv5YiSi7ZgCtD+8\n" +
>             
> "lZcHeadV9V7Mm77x4+pTlJ1ZuMb575OxKOfEX0LmN6qqvW0U5LdSCDa1WWkIqHaa\n" +
>             
> "/MVAM22QOFcCgYEA4PaZZMolTS9IKKT6Wj4DcntbPgppgMQGr7NJOz55GmysyiQh\n" +
>             
> "+i2h/DAxQrANaGsjmhMLfBQrlVjG+k7gHdOTxv8gFKiW6q/B1UP2H+5w0P5oebLl\n" +
>             
> "us/h/gAaIW8418MEgQ4DGhnwi83GG4u6OJRDtJCsrNiTNXFA1mG1fep2mkUCgYEA\n" +
>             
> "k92UdXn7fyBtIr+n4QlC5BdzJGSW8U6Fv0fFUvZG0fCUH5SvQ4gQ3pTRJaqU7JFM\n" +
>             
> "lMTtDB4vGXs3I8KS6X74tkhdy5QDBG7c+E46HyVBANl+VIcIA5HtZJu/V0LixMwe\n" +
>             
> "9Z3YdxSL8wnirjwHCsro+lj5juhDPETqezGeDAObtLsCgYEAqQQvzv/zc/HmzdhI\n" +
>             
> "wvFg8HubQWOhv7LwF16VBLA1I7ojZnTCvIQph9GKmp6NQRY1oHUC62giYDZzCjQ3\n" +
>             
> "2lAYmJ52/vIep2ktoa/tAxJdCLL0ULkpfze8ZJSNN3uSip4KadybAwz7JHuQ9J/5\n" +
>             "XKr8FjgTA/zAHLJ9Dft+jqlx9LA=\n" +
>             "-----END PRIVATE KEY-----\n";
>     private final static String CERT_PEM = "" +
>             "-----BEGIN CERTIFICATE-----\n" +
>             
> "MIIDDTCCAfWgAwIBAgIUPKaQbQfY5d36BzcHcJg5MmMaqBQwDQYJKoZIhvcNAQEL\n" +
>             
> "BQAwFjEUMBIGA1UEAwwLdGVzdC1jbGllbnQwHhcNMTkwMzI5MTg0NzI0WhcNMjkw\n" +
>             
> "MzI2MTg0NzI0WjAWMRQwEgYDVQQDDAt0ZXN0LWNsaWVudDCCASIwDQYJKoZIhvcN\n" +
>             
> "AQEBBQADggEPADCCAQoCggEBAL4rOjkSF3W+tt5lr6s0iqcjNYDMqv+JYXJJCcZC\n" +
>             
> "7lKkrWexnkmEErzcnhFDFFLqZQuuJ8mAb8CoyUDQ2YIPDQLqn+5AX5UrJye1QuOe\n" +
>             
> "6/CNTTiWROataTe3Aee8OR36PZIf6N/ONl4+/6VcIJAtBLnGIyt4aMzHgQgtSwHT\n" +
>             
> "QRChQVt0aDU2BO3uDwv9a4QML1jJsLoUkc4Fg//TbCZbhERjIy5VjdJ9kCw6TlhF\n" +
>             
> "sR/blzsgADf61tpfP1K5GBQrdIojRF4YG0vK9KODKjI5PBPz1A50Swvooz1NmGMk\n" +
>             
> "amZ5cYwcE0/u4Re5eB8HrAprLV6MPqGv6VyCcgJCoe2EBKsCAwEAAaNTMFEwHQYD\n" +
>             
> "VR0OBBYEFHVu5hu3Z7NYqZuxroKghvqtlItSMB8GA1UdIwQYMBaAFHVu5hu3Z7NY\n" +
>             
> "qZuxroKghvqtlItSMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEB\n" +
>             
> "AKPfRWU4W6Zr/HV3hGYIga/arXWpkO/2mW0dN9qdQI3Ok1rXezFhrtxiR5iOaN0f\n" +
>             
> "wYKAUAbfQmfN6Ai4Nl0ZSbuWRe7/Vut/+rwINaARZLoDz3HVeAmi52ZKsikZG9h0\n" +
>             
> "InpDa52Zs/8nnxPo7UiXP05mMTt2psyQGYeolac2FliKchZ7+SvX2FPxSUQ7LkNT\n" +
>             
> "1DGu7jlLkT3plf9rCW0PhIa6vpzyxVVijjqZl86fBU6vaFzJHH6pNoPpQbK4vOmb\n" +
>             
> "2278AHjrDTU6j8IL0uSk8g2klZ8hC9kerblLREp09Haw5l74kwBiypLHV366XbWR\n" +
>             "uDXRs4Y1a29pJ41jDhlqllk=\n" +
>             "-----END CERTIFICATE-----\n";
>     public static void main(String[] args) throws Exception {
>         final X509Certificate clientCertificate = 
> PemUtils.loadCertificate(CERT_PEM);
>         final PrivateKey clientKey = PemUtils.loadPrivateKey(KEY_PEM);
>         final SSLContextBuilder sslContextBuilder = SSLContexts.custom();
>         final KeyStore keystore = 
> KeyStore.getInstance(KeyStore.getDefaultType());
>         keystore.load(null, null);
>         // When this line is commented out, no client certificate is used and 
> Keep-Alive works as expected
>         // and connection gets reused for the second request.
>         // However when line is not commented, client SSL cert is used and 
> this breaks connection pool logic
>         keystore.setKeyEntry("client", clientKey, new char[]{}, new 
> X509Certificate[]{clientCertificate});
>         sslContextBuilder.loadKeyMaterial(keystore, new char[]{});
>         final SSLConnectionSocketFactory sslConnectionSocketFactory = new 
> SSLConnectionSocketFactory(
>                 // Do not verify hostname in case we want to use IP in 
> addition to name.
>                 // After all this is a test only
>                 sslContextBuilder.build(), new NoopHostnameVerifier());
>         final Registry<ConnectionSocketFactory> socketFactoryRegistry =
>                 RegistryBuilder.<ConnectionSocketFactory>create()
>                         .register("https", sslConnectionSocketFactory)
>                         .build();
>         PoolingHttpClientConnectionManager connManager = new 
> PoolingHttpClientConnectionManager(socketFactoryRegistry);
>         final CloseableHttpClient client = HttpClients.custom()
>                 .setConnectionManager(connManager)
>                 .setDefaultRequestConfig(
>                         RequestConfig.custom()
>                                 .setRedirectsEnabled(false)
>                                 .build())
>                 .build();
>         // Found the server here https://stackoverflow.com/a/42770043 - it 
> requires client certificate
>         // Any "normal" HTTPS server like google.com does not ask for 
> certificate
>         // and the issue is only reproducible when client cert is actually 
> used not just added to KeyStore
>         final String url = "https://server.cryptomix.com/secure/";;
>         // Actual responses are irrelevant. The important bit is what tcpdump 
> shows:
>         //   sudo tcpdump -pn host server.cryptomix.com
>         // there will be two separate connections for each
>         // And if client cert is not used, there will be only one
>         final CloseableHttpResponse response1 = client.execute(new 
> HttpGet(url));
>         System.out.println("response1 = " + 
> response1.getHeaders("Content-Type")[0]);
>         EntityUtils.consume(response1.getEntity());
>         final CloseableHttpResponse response2 = client.execute(new 
> HttpGet(url));
>         System.out.println("response2 = " + 
> response2.getHeaders("Content-Type")[0]);
>         EntityUtils.consume(response2.getEntity());
>         // The fact two connections were used is also visible in the pool 
> stats
>         // as it shows
>         //   [leased: 0; pending: 0; available: 2; max: 20]
>         // and when keystore.setKeyEntry() is commented out - just 1 
> available connection
>         System.out.println("Pool stats: " + connManager.getTotalStats());
>     }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to