ReferenceCountedOpenSslEngines SSLSession must provide local certific… (#8918)

Motivation:

The SSLSession that is returned by SSLEngine.getHandshakeSession() must be able to provide the local certificates when the TrustManager is invoked on the server-side.

Modifications:

- Correctly return the local certificates
- Add unit test

Result:

Be able to obtain local certificates from handshake SSLSession during verification on the server side.

handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java

@@ -2002,7 +2002,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
         // thread.
         private X509Certificate[] x509PeerCerts;
         private Certificate[] peerCerts;
-        private Certificate[] localCerts;
 
         private String protocol;
         private String cipher;
@@ -2149,7 +2148,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
                     id = SSL.getSessionId(ssl);
                     cipher = toJavaCipherSuite(SSL.getCipherForSSL(ssl));
                     protocol = SSL.getVersion(ssl);
-                    localCerts = localCertificateChain;
 
                     initPeerCerts();
                     selectApplicationProtocol();
@@ -2284,6 +2282,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
 
         @Override
         public Certificate[] getLocalCertificates() {
+            Certificate[] localCerts = ReferenceCountedOpenSslEngine.this.localCertificateChain;
             if (localCerts == null) {
                 return null;
             }
@@ -2310,7 +2309,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
 
         @Override
         public Principal getLocalPrincipal() {
-            Certificate[] local = localCerts;
+            Certificate[] local = ReferenceCountedOpenSslEngine.this.localCertificateChain;
             if (local == null || local.length == 0) {
                 return null;
             }

handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java

@@ -87,4 +87,11 @@ public class ConscryptJdkSslEngineInteropTest extends SSLEngineTest {
         // Ignore due bug in Conscrypt where the incorrect SSLSession object is used in the SSLSessionBindingEvent.
         // See https://github.com/google/conscrypt/issues/593
     }
+
+    @Ignore("Ignore due bug in Conscrypt")
+    @Override
+    public void testHandshakeSession() throws Exception {
+        // Ignore as Conscrypt does not correctly return the local certificates while the TrustManager is invoked.
+        // See https://github.com/google/conscrypt/issues/634
+    }
 }

handler/src/test/java/io/netty/handler/ssl/ConscryptSslEngineTest.java

@@ -85,4 +85,11 @@ public class ConscryptSslEngineTest extends SSLEngineTest {
         // Ignore due bug in Conscrypt where the incorrect SSLSession object is used in the SSLSessionBindingEvent.
         // See https://github.com/google/conscrypt/issues/593
     }
+
+    @Ignore("Ignore due bug in Conscrypt")
+    @Override
+    public void testHandshakeSession() throws Exception {
+        // Ignore as Conscrypt does not correctly return the local certificates while the TrustManager is invoked.
+        // See https://github.com/google/conscrypt/issues/634
+    }
 }

handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java

@@ -85,4 +85,11 @@ public class JdkConscryptSslEngineInteropTest extends SSLEngineTest {
         // TODO(scott): work around for a JDK issue. The exception should be SSLHandshakeException.
         return super.mySetupMutualAuthServerIsValidClientException(cause) || causedBySSLException(cause);
     }
+
+    @Ignore("Ignore due bug in Conscrypt")
+    @Override
+    public void testHandshakeSession() throws Exception {
+        // Ignore as Conscrypt does not correctly return the local certificates while the TrustManager is invoked.
+        // See https://github.com/google/conscrypt/issues/634
+    }
 }

handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java

@@ -128,6 +128,12 @@ public class JdkOpenSslEngineInteroptTest extends SSLEngineTest {
         return super.mySetupMutualAuthServerIsValidClientException(cause) || causedBySSLException(cause);
     }
 
+    @Override
+    public void testHandshakeSession() throws Exception {
+        checkShouldUseKeyManagerFactory();
+        super.testHandshakeSession();
+    }
+
     @Override
     protected SSLEngine wrapEngine(SSLEngine engine) {
         return Java8SslTestUtils.wrapSSLEngineForTesting(engine);

handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java

@@ -161,6 +161,12 @@ public class OpenSslEngineTest extends SSLEngineTest {
         super.testClientHostnameValidationFail();
     }
 
+    @Override
+    public void testHandshakeSession() throws Exception {
+        checkShouldUseKeyManagerFactory();
+        super.testHandshakeSession();
+    }
+
     private static boolean isNpnSupported(String versionString) {
         String[] versionStringParts = versionString.split(" ", -1);
         if (versionStringParts.length == 2 && "LibreSSL".equals(versionStringParts[0])) {

handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java

@@ -127,6 +127,12 @@ public class OpenSslJdkSslEngineInteroptTest extends SSLEngineTest {
         return super.mySetupMutualAuthServerIsValidServerException(cause) || causedBySSLException(cause);
     }
 
+    @Override
+    public void testHandshakeSession() throws Exception {
+        checkShouldUseKeyManagerFactory();
+        super.testHandshakeSession();
+    }
+
     @Override
     protected SSLEngine wrapEngine(SSLEngine engine) {
         return Java8SslTestUtils.wrapSSLEngineForTesting(engine);

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

@@ -37,6 +37,7 @@ import io.netty.channel.socket.nio.NioSocketChannel;
 import io.netty.handler.ssl.ApplicationProtocolConfig.Protocol;
 import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
 import io.netty.handler.ssl.util.SelfSignedCertificate;
+import io.netty.handler.ssl.util.SimpleTrustManagerFactory;
 import io.netty.util.CharsetUtil;
 import io.netty.util.NetUtil;
 import io.netty.util.ReferenceCountUtil;
@@ -71,18 +72,24 @@ import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.TrustManagerFactorySpi;
+import javax.net.ssl.X509ExtendedTrustManager;
 import javax.net.ssl.X509TrustManager;
 import javax.security.cert.X509Certificate;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetSocketAddress;
+import java.net.Socket;
 import java.nio.ByteBuffer;
 import java.nio.channels.ClosedChannelException;
 import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 import java.security.Provider;
+import java.security.UnrecoverableKeyException;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.util.ArrayList;
@@ -107,6 +114,7 @@ import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_1;
 import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2;
 import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_3;
 import static io.netty.handler.ssl.SslUtils.SSL_RECORD_HEADER_LENGTH;
+
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -3060,6 +3068,149 @@ public abstract class SSLEngineTest {
         }
     }
 
+    @Test
+    public void testHandshakeSession() throws Exception {
+        final SelfSignedCertificate ssc = new SelfSignedCertificate();
+
+        final TestTrustManagerFactory clientTmf = new TestTrustManagerFactory(ssc.cert());
+        final TestTrustManagerFactory serverTmf = new TestTrustManagerFactory(ssc.cert());
+
+        clientSslCtx = SslContextBuilder.forClient()
+                .trustManager(new SimpleTrustManagerFactory() {
+                    @Override
+                    protected void engineInit(KeyStore keyStore) {
+                        // NOOP
+                    }
+
+                    @Override
+                    protected void engineInit(ManagerFactoryParameters managerFactoryParameters) {
+                        // NOOP
+                    }
+
+                    @Override
+                    protected TrustManager[] engineGetTrustManagers() {
+                        return new TrustManager[] { clientTmf };
+                    }
+                })
+                .keyManager(newKeyManagerFactory(ssc))
+                .sslProvider(sslClientProvider())
+                .sslContextProvider(clientSslContextProvider())
+                .protocols(protocols())
+                .ciphers(ciphers())
+                .build();
+        serverSslCtx = SslContextBuilder.forServer(newKeyManagerFactory(ssc))
+                .trustManager(new SimpleTrustManagerFactory() {
+                    @Override
+                    protected void engineInit(KeyStore keyStore) {
+                        // NOOP
+                    }
+
+                    @Override
+                    protected void engineInit(ManagerFactoryParameters managerFactoryParameters) {
+                        // NOOP
+                    }
+
+                    @Override
+                    protected TrustManager[] engineGetTrustManagers() {
+                        return new TrustManager[] { serverTmf };
+                    }
+                })
+                .sslProvider(sslServerProvider())
+                .sslContextProvider(serverSslContextProvider())
+                .protocols(protocols())
+                .ciphers(ciphers())
+                .clientAuth(ClientAuth.REQUIRE)
+                .build();
+        SSLEngine clientEngine = null;
+        SSLEngine serverEngine = null;
+        try {
+            clientEngine = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT));
+            serverEngine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT));
+            handshake(clientEngine, serverEngine);
+
+            assertTrue(clientTmf.isVerified());
+            assertTrue(serverTmf.isVerified());
+        } finally {
+            cleanupClientSslEngine(clientEngine);
+            cleanupServerSslEngine(serverEngine);
+            ssc.delete();
+        }
+    }
+
+    private KeyManagerFactory newKeyManagerFactory(SelfSignedCertificate ssc)
+            throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+            CertificateException, IOException {
+        return SslContext.buildKeyManagerFactory(
+                new java.security.cert.X509Certificate[] { ssc.cert() }, ssc.key(), null, null);
+    }
+
+    private final class TestTrustManagerFactory extends X509ExtendedTrustManager {
+        private final Certificate localCert;
+        private volatile boolean verified;
+
+        TestTrustManagerFactory(Certificate localCert) {
+            this.localCert = localCert;
+        }
+
+        boolean isVerified() {
+            return verified;
+        }
+
+        @Override
+        public void checkClientTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s, Socket socket) {
+            fail();
+        }
+
+        @Override
+        public void checkServerTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s, Socket socket) {
+            fail();
+        }
+
+        @Override
+        public void checkClientTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) {
+            verified = true;
+            assertFalse(sslEngine.getUseClientMode());
+            SSLSession session = sslEngine.getHandshakeSession();
+            assertNotNull(session);
+            Certificate[] localCertificates = session.getLocalCertificates();
+            assertNotNull(localCertificates);
+            assertEquals(1, localCertificates.length);
+            assertEquals(localCert, localCertificates[0]);
+            assertNotNull(session.getLocalPrincipal());
+        }
+
+        @Override
+        public void checkServerTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) {
+            verified = true;
+            assertTrue(sslEngine.getUseClientMode());
+            SSLSession session = sslEngine.getHandshakeSession();
+            assertNotNull(session);
+            assertNull(session.getLocalCertificates());
+            assertNull(session.getLocalPrincipal());
+        }
+
+        @Override
+        public void checkClientTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s) {
+            fail();
+        }
+
+        @Override
+        public void checkServerTrusted(
+                java.security.cert.X509Certificate[] x509Certificates, String s) {
+            fail();
+        }
+
+        @Override
+        public java.security.cert.X509Certificate[] getAcceptedIssuers() {
+            return EmptyArrays.EMPTY_X509_CERTIFICATES;
+        }
+    }
+
     protected SSLEngine wrapEngine(SSLEngine engine) {
         return engine;
     }