MockWebServer incompatible with HttpClient closing on TLSv1.3 #38646

jaymode · 2019-02-08T19:35:09Z

The MockWebServer used by certain tests is incompatible with the way that HttpClient closes connections, when TLSv1.3 is in use AND the entire response has not been consumed. The HttpClient connection close code does some manual manipulation of the socket prior to calling close(). Ultimately a close_notify is sent by the client and then the server attempts to respond to it, but the TLSv1.3 SSLEngine never produces any bytes and doesn't transition back to the closed state, so the MockWebServer ends up in an endless loop.

This behavior is ultimately all within JDK code on the server side; fortunately there are workarounds that we can use for tests.

Consume the whole response
Use Socket#close instead of the HttpClient closing code by creating our own connection, connection factory and connection manager

I plan to send this issue to the OpenJDK mailing list with the following as a minimal reproduction.

import com.sun.net.httpserver.HttpsConfigurator;
import com.sun.net.httpserver.HttpsParameters;
import com.sun.net.httpserver.HttpsServer;

import javax.net.ssl.HandshakeCompletedEvent;
import javax.net.ssl.HandshakeCompletedListener;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManagerFactory;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.security.KeyStore;

public class HttpsServerEndlessLoop {

    public static void main(String[] args) throws Exception {
        final SSLContext sslContext = getSSLContext();
        InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress().getHostAddress(), 0);
        HttpsServer httpsServer = HttpsServer.create(address, 0);
        httpsServer.setHttpsConfigurator(new HttpsConfigurator(sslContext) {
            @Override
            public void configure (HttpsParameters params) {
            }
        });
        httpsServer.createContext("/", s -> {
            try {
                final byte[] body = "<body>".getBytes(StandardCharsets.UTF_8);
                s.sendResponseHeaders(200, body.length);
                s.getResponseBody().write(body);
            } finally {
                s.close();
            }
        });
        httpsServer.start();

        SSLSocketFactory socketFactory = sslContext.getSocketFactory();
        SSLSocket sslSocket = (SSLSocket)
            socketFactory.createSocket(httpsServer.getAddress().getHostString(), httpsServer.getAddress().getPort());
        sslSocket.addHandshakeCompletedListener(new HandshakeCompletedListener() {
            @Override
            public void handshakeCompleted(HandshakeCompletedEvent event) {
                System.out.println("handshake of initial socket completed with session: " + event.getSession());
            }
        });

        sslSocket.startHandshake();
        final String httpGetLine = "GET / HTTP/1.1\r\n\r\n";
        OutputStream os = sslSocket.getOutputStream();
        os.write(httpGetLine.getBytes(StandardCharsets.UTF_8));
        BufferedReader reader = new BufferedReader(new InputStreamReader(sslSocket.getInputStream(), StandardCharsets.UTF_8));
        // only read the first line before closing, but if all lines are read the issue doesn't happen
        System.out.println(reader.readLine());

        try {
            try {
                try {
                    sslSocket.shutdownOutput();
                } catch (final IOException e) {
                    System.out.println("exception while shutting down output: ");
                    e.printStackTrace();
                }
                try {
                    sslSocket.shutdownInput();
                } catch (final IOException e) {
                    System.out.println("exception while shutting down input: ");
                    e.printStackTrace();
                }
            } catch (final UnsupportedOperationException ignore) {
                // if one isn't supported, the other one isn't either
            }
        } finally {
            sslSocket.close();
        }

        sslSocket = (SSLSocket)
            socketFactory.createSocket(httpsServer.getAddress().getHostString(), httpsServer.getAddress().getPort());
        sslSocket.addHandshakeCompletedListener(new HandshakeCompletedListener() {
            @Override
            public void handshakeCompleted(HandshakeCompletedEvent event) {
                System.out.println("handshake of second socket completed with session: " + event.getSession());
            }
        });

        // never get past here!
        sslSocket.startHandshake();

        sslSocket.close();
        httpsServer.stop(0);
    }

    private static SSLContext getSSLContext() throws Exception {
        KeyStore keyStore = KeyStore.getInstance("JKS");
        try (InputStream is = HttpsServerEndlessLoop.class.getResourceAsStream("/test.jks")) {
            keyStore.load(is, "test".toCharArray());
        }
        KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
        keyManagerFactory.init(keyStore, "test".toCharArray());
        TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
        trustManagerFactory.init(keyStore);

        SSLContext sslContext = SSLContext.getInstance("TLSv1.3");
        sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null);
        return sslContext;
    }
}

The text was updated successfully, but these errors were encountered:

This change removes the pinning of TLSv1.2 in the SSLConfigurationReloaderTests that had been added to workaround an issue with the MockWebServer and Apache HttpClient when using TLSv1.3. The way HttpClient closes the socket causes issues with the TLSv1.3 SSLEngine implementation that causes the MockWebServer to loop endlessly trying to send the close message back to the client. This change wraps the created http connection in a way that allows us to override the closing behavior of HttpClient. An upstream request with HttpClient has been opened at https://issues.apache.org/jira/browse/HTTPCORE-571 to see if the method of closing can be special cased for SSLSocket instances. Relates elastic#38646

This change removes the pinning of TLSv1.2 in the SSLConfigurationReloaderTests that had been added to workaround an issue with the MockWebServer and Apache HttpClient when using TLSv1.3. The way HttpClient closes the socket causes issues with the TLSv1.3 SSLEngine implementation that causes the MockWebServer to loop endlessly trying to send the close message back to the client. This change wraps the created http connection in a way that allows us to override the closing behavior of HttpClient. An upstream request with HttpClient has been opened at https://issues.apache.org/jira/browse/HTTPCORE-571 to see if the method of closing can be special cased for SSLSocket instances. This is caused by a JDK bug, JDK-8214418 which is fixed by https://hg.openjdk.java.net/jdk/jdk12/rev/5022a4915fe9. Relates #38646

jaymode · 2019-02-12T16:29:56Z

On the security-dev mailing list it has been suggested that this matches a JDK bug, JDK-8214418, which is currently not visible in the openjdk bug tracker. The bug will be fixed in JDK 12. No changes will be made in the HttpClient code.

This change removes the pinning of TLSv1.2 in the SSLConfigurationReloaderTests that had been added to workaround an issue with the MockWebServer and Apache HttpClient when using TLSv1.3. The way HttpClient closes the socket causes issues with the TLSv1.3 SSLEngine implementation that causes the MockWebServer to loop endlessly trying to send the close message back to the client. This change wraps the created http connection in a way that allows us to override the closing behavior of HttpClient. An upstream request with HttpClient has been opened at https://issues.apache.org/jira/browse/HTTPCORE-571 to see if the method of closing can be special cased for SSLSocket instances. This is caused by a JDK bug, JDK-8214418 which is fixed by https://hg.openjdk.java.net/jdk/jdk12/rev/5022a4915fe9. Relates #38646

jaymode · 2019-05-02T19:11:47Z

JDK 12.0.1 has been released but this bug is still present or a version of it is as I have encountered the endless loop with this JDK. Additionally when the runtime JDK is less than 12.0.1, tests can fail.

This commit updates the default ciphers and TLS protocols that are used when the runtime JDK supports them. New cipher support has been introduced in JDK 11 and 12 along with performance fixes for AES GCM. The ciphers are ordered with PFS ciphers being most preferred, then AEAD ciphers, and finally those with mainstream hardware support. When available stronger encryption is preferred for a given cipher. This is a backport of elastic#41385 and elastic#41808. There are known JDK bugs with TLSv1.3 that have been fixed in various versions. These are: 1. The JDK's bundled HttpsServer will endless loop under JDK11 and JDK 12.0 (Fixed in 12.0.1) based on the way the Apache HttpClient performs a close (half close). 2. In all versions of JDK 11 and 12, the HttpsServer will endless loop when certificates are not trusted or another handshake error occurs. An email has been sent to the openjdk security-dev list and elastic#38646 is open to track this. 3. In JDK 11.0.2 and prior there is a race condition with session resumption that leads to handshake errors when multiple concurrent handshakes are going on between the same client and server. This bug does not appear when client authentication is in use. This is JDK-8213202, which was fixed in 11.0.3 and 12.0. 4. In JDK 11.0.2 and prior there is a bug where resumed TLS sessions do not retain peer certificate information. This is JDK-8212885. The way these issues are addressed is that the current java version is checked and used to determine the supported protocols for tests that provoke these issues.

This commit updates the default ciphers and TLS protocols that are used when the runtime JDK supports them. New cipher support has been introduced in JDK 11 and 12 along with performance fixes for AES GCM. The ciphers are ordered with PFS ciphers being most preferred, then AEAD ciphers, and finally those with mainstream hardware support. When available stronger encryption is preferred for a given cipher. This is a backport of #41385 and #41808. There are known JDK bugs with TLSv1.3 that have been fixed in various versions. These are: 1. The JDK's bundled HttpsServer will endless loop under JDK11 and JDK 12.0 (Fixed in 12.0.1) based on the way the Apache HttpClient performs a close (half close). 2. In all versions of JDK 11 and 12, the HttpsServer will endless loop when certificates are not trusted or another handshake error occurs. An email has been sent to the openjdk security-dev list and #38646 is open to track this. 3. In JDK 11.0.2 and prior there is a race condition with session resumption that leads to handshake errors when multiple concurrent handshakes are going on between the same client and server. This bug does not appear when client authentication is in use. This is JDK-8213202, which was fixed in 11.0.3 and 12.0. 4. In JDK 11.0.2 and prior there is a bug where resumed TLS sessions do not retain peer certificate information. This is JDK-8212885. The way these issues are addressed is that the current java version is checked and used to determine the supported protocols for tests that provoke these issues.

jaymode · 2020-10-25T19:28:21Z

This might finally be fixed in JDK 16 openjdk/jdk@953e472

This commit changes code that previously pinned to TLSv1.2 when running on JDK 12+ to allow the use of TLSv1.3 if on JDK 16 or newer. There was a bug in the HttpsServer code that has finally been fixed, which prevented the use of TLSv1.3 as the HttpsServer would endlessly loop. The JDK bug is JDK-8254967. Closes elastic#38646

This commit changes code that previously pinned to TLSv1.2 when running on JDK 12+ to allow the use of TLSv1.3 if on JDK 16 or newer. There was a bug in the HttpsServer code that has finally been fixed, which prevented the use of TLSv1.3 as the HttpsServer would endlessly loop. The JDK bug is JDK-8254967. Closes #38646

This commit changes code that previously pinned to TLSv1.2 when running on JDK 12+ to allow the use of TLSv1.3 if on JDK 16 or newer. There was a bug in the HttpsServer code that has finally been fixed, which prevented the use of TLSv1.3 as the HttpsServer would endlessly loop. The JDK bug is JDK-8254967. Closes #38646 Backport of #64496

jaymode added >test Issues or PRs that are addressing/adding tests :Security/TLS SSL/TLS, Certificates labels Feb 8, 2019

jaymode mentioned this issue Feb 8, 2019

Remove TLSv1.2 pinning in ssl reload tests #38651

Merged

jaymode mentioned this issue May 3, 2019

Update TLS ciphers and protocols for JDK 11 #41808

Merged

jaymode mentioned this issue May 10, 2019

Update ciphers for TLSv1.3 and JDK11 if available #42082

Merged

rjernst added the Team:Security Meta label for security team label May 4, 2020

jaymode mentioned this issue Nov 2, 2020

HttpsServer can use TLSv1.3 on JDK16+ #64496

Merged

jaymode closed this as completed in #64496 Nov 19, 2020

jaymode mentioned this issue Nov 19, 2020

HttpsServer can use TLSv1.3 on JDK16+ #65274

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MockWebServer incompatible with HttpClient closing on TLSv1.3 #38646

MockWebServer incompatible with HttpClient closing on TLSv1.3 #38646

jaymode commented Feb 8, 2019

jaymode commented Feb 12, 2019

jaymode commented May 2, 2019

jaymode commented Oct 25, 2020

MockWebServer incompatible with HttpClient closing on TLSv1.3 #38646

MockWebServer incompatible with HttpClient closing on TLSv1.3 #38646

Comments

jaymode commented Feb 8, 2019

jaymode commented Feb 12, 2019

jaymode commented May 2, 2019

jaymode commented Oct 25, 2020