Bill commented on a change in pull request #7449: URL: https://github.com/apache/geode/pull/7449#discussion_r836882933
########## File path: geode-core/src/integrationTest/java/org/apache/geode/internal/net/NioSslEngineKeyUpdateTest.java ########## @@ -0,0 +1,459 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.internal.net; + +import static java.util.concurrent.CompletableFuture.supplyAsync; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import java.io.FileInputStream; +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; +import java.security.GeneralSecurityException; +import java.security.KeyManagementException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.Security; +import java.security.UnrecoverableKeyException; +import java.util.Arrays; +import java.util.Properties; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.TrustManagerFactory; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.geode.cache.ssl.CertStores; +import org.apache.geode.cache.ssl.CertificateBuilder; +import org.apache.geode.cache.ssl.CertificateMaterial; +import org.apache.geode.distributed.internal.DMStats; + +/** + * In TLSv1.3, when a GCM-based cipher is used, there is a limit on the number + * of bytes that may be encoded with a key. When that limit is reached, a TLS + * KeyUpdate message is generated (by the SSLEngine). That message causes a new + * key to be negotiated between the peer SSLEngines. + * + * Geode's {@link NioSslEngine} class (subclass of {@link NioFilter}) encapsulates + * Java's {@link SSLEngine}. Geode's MsgStreamer, Connection, and ConnectionTable + * classes interact with NioSslEngine to accomplish peer-to-peer (P2P) messaging. + * + * This test constructs a pair of SSLEngine's and wraps them in NioSslEngine. + * Rather than relying on MsgStreamer, Connection, or ConnectionTable classes + * (which themselves have a lot of dependencies on other classes), this test + * implements simplified logic for driving the sending and receiving of data, + * i.e. calling NioSslEngine.wrap() and NioSslEngine.unwrap(). See + * {@link #send(int, NioSslEngine, SocketChannel, int)} and + * {@link #receive(int, NioSslEngine, SocketChannel, ByteBuffer)}. + * + * The {@link #secureDataTransferTest()} arranges for the encrypted bytes limit + * to be reached very quickly (see {@link #ENCRYPTED_BYTES_LIMIT}). + * The test verifies data transfer continues correctly, after the limit is reached. + * This indirectly verifies that the KeyUpdate protocol initiated by the sending + * SSLEngine is correctly handled by all the components involved. + */ +public class NioSslEngineKeyUpdateTest { + + private static final String TLS_PROTOCOL = "TLSv1.3"; + private static final String TLS_CIPHER_SUITE = "TLS_AES_256_GCM_SHA384"; + + // number of bytes the GCM cipher can encrypt before initiating a KeyUpdate + private static final int ENCRYPTED_BYTES_LIMIT = 1; + + /* + * KeyUpdate messages don't happen in the handshake, they happen afterward. + * This is the number of bytes we'll transfer after the TLS handshake. + */ + private static final int BYTES_TO_TRANSFER_AFTER_HANDSHAKE = 2; + + { + assert IsPowerOfTwo(ENCRYPTED_BYTES_LIMIT) && ENCRYPTED_BYTES_LIMIT != 0; + + // The bytes will be sent using two calls to wrap so the number must be even + assert BYTES_TO_TRANSFER_AFTER_HANDSHAKE % 2 == 0; + + assert BYTES_TO_TRANSFER_AFTER_HANDSHAKE > ENCRYPTED_BYTES_LIMIT; + + Security.setProperty("jdk.tls.keyLimits", + "AES/GCM/NoPadding KeyUpdate " + ENCRYPTED_BYTES_LIMIT); + } + + private static BufferPool bufferPool; + private static SSLContext sslContext; + private static KeyStore keystore; + private static char[] keystorePassword; + private static KeyStore truststore; + + private SSLEngine clientEngine; + private SSLEngine serverEngine; + private int packetBufferSize; + + @BeforeClass + public static void beforeClass() throws GeneralSecurityException, IOException { + DMStats mockStats = mock(DMStats.class); + bufferPool = new BufferPool(mockStats); + + final Properties securityProperties = createKeystoreAndTruststore(); + + keystore = KeyStore.getInstance("JKS"); + keystorePassword = securityProperties.getProperty(SSL_KEYSTORE_PASSWORD).toCharArray(); + keystore.load(new FileInputStream(securityProperties.getProperty(SSL_KEYSTORE)), + keystorePassword); + + truststore = KeyStore.getInstance("JKS"); + final char[] truststorePassword = + securityProperties.getProperty(SSL_TRUSTSTORE_PASSWORD).toCharArray(); + truststore.load(new FileInputStream(securityProperties.getProperty(SSL_TRUSTSTORE)), + truststorePassword); + } + + @Before + public void before() throws NoSuchAlgorithmException, UnrecoverableKeyException, + KeyStoreException, KeyManagementException { + final KeyManagerFactory kmf = KeyManagerFactory.getInstance("PKIX"); + kmf.init(keystore, keystorePassword); + final TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX"); + tmf.init(truststore); + + sslContext = SSLContext.getInstance("TLS"); + final SecureRandom random = new SecureRandom(new byte[] {1, 2, 3}); + sslContext.init(kmf.getKeyManagers(), tmf.getTrustManagers(), random); + + final SSLParameters defaultParameters = sslContext.getDefaultSSLParameters(); + final String[] protocols = defaultParameters.getProtocols(); + final String[] cipherSuites = defaultParameters.getCipherSuites(); + System.out.println( + String.format("TLS settings (default) before handshake: Protocols: %s, Cipher Suites: %s", + Arrays.toString(protocols), Arrays.toString(cipherSuites))); + + clientEngine = createSSLEngine("server-host", true, sslContext); + packetBufferSize = clientEngine.getSession().getPacketBufferSize(); + + serverEngine = createSSLEngine("client-host", false, sslContext); + } + + @Test + public void handshakeTest() { + clientServerTest( + (channel, filter, peerNetData) -> { + handshakeTLS(channel, filter, peerNetData, "Client:"); + return true; + }, + (channel, filter, peerNetData1) -> { + handshakeTLS(channel, filter, peerNetData1, + "Server:"); + return true; + }); + } + + @Test + public void secureDataTransferTest() { + clientServerTest( + (final SocketChannel channel, + final NioSslEngine filter, + final ByteBuffer peerNetData) -> { + handshakeTLS(channel, filter, peerNetData, "Client:"); + /* + * if we call send() only once, it seems that jdk.tls.keyLimits + * is not evaluated by wrap(). Calling it twice seems to fix that. + */ + // send(BYTES_TO_TRANSFER_AFTER_HANDSHAKE, filter, channel); + send(BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2, filter, channel, 0); + send(BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2, filter, channel, + BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2); + return true; + }, + (final SocketChannel channel, + final NioSslEngine filter, + final ByteBuffer peerNetData) -> { + handshakeTLS(channel, filter, peerNetData, "Server:"); + /* + * Call receive() twice (like we did for send()) just to test that receive() is + * leaving buffers in the correct readable/writable state when it returns. + */ + for (int i = 0; i < 2; i++) { + final byte[] received = + receive(BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2, filter, channel, peerNetData); + assertThat(received).hasSize(BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2); + for (int j = i * BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2; j < (i + 1) + * BYTES_TO_TRANSFER_AFTER_HANDSHAKE / 2; j++) { + assertThat(received[j % received.length]).isEqualTo((byte) j); + } + } + return true; + }); + } + + private static SSLEngine createSSLEngine(final String peerHost, final boolean useClientMode, + final SSLContext sslContext) { + final SSLEngine engine = sslContext.createSSLEngine(peerHost, 10001); + engine.setEnabledProtocols(new String[] {TLS_PROTOCOL}); + engine.setEnabledCipherSuites(new String[] {TLS_CIPHER_SUITE}); + engine.setUseClientMode(useClientMode); + return engine; + } + + private void clientServerTest(final PeerAction clientAction, final PeerAction serverAction) { + final ExecutorService executorService = Executors.newFixedThreadPool(2); + + final CompletableFuture<SocketAddress> boundAddress = new CompletableFuture<>(); + + final CountDownLatch serversWaiting = new CountDownLatch(1); + + final CompletableFuture<Boolean> serverHandshakeFuture = + supplyAsync( + () -> server(boundAddress, packetBufferSize, + serverAction, serversWaiting, serverEngine), + executorService); + + final CompletableFuture<Boolean> clientHandshakeFuture = + supplyAsync( + () -> client(boundAddress, packetBufferSize, + clientAction, serversWaiting, clientEngine), + executorService); + + CompletableFuture.allOf(serverHandshakeFuture, clientHandshakeFuture) + .join(); + } + + /* + * An action taken on a client or server after the SocketChannel has been established. + */ + private interface PeerAction { + boolean apply(final SocketChannel acceptedChannel, + final NioSslEngine filter, + final ByteBuffer peerNetData) throws IOException; + } + + private static boolean client( + final CompletableFuture<SocketAddress> boundAddress, + final int packetBufferSize, + final PeerAction peerAction, + final CountDownLatch serversWaiting, + final SSLEngine engine) { + try { + try (final SocketChannel connectedChannel = SocketChannel.open()) { + connectedChannel.connect(boundAddress.get()); + final ByteBuffer peerNetData = + ByteBuffer.allocateDirect(packetBufferSize); + + final NioSslEngine filter = new NioSslEngine(engine, bufferPool); + + final boolean result = + peerAction.apply(connectedChannel, filter, peerNetData); + + serversWaiting.await(); // wait for last server to give up before closing our socket + + filter.close(connectedChannel); + + return result; + } + } catch (IOException | InterruptedException | ExecutionException e) { + printException("In client:", e); + throw new RuntimeException(e); + } + } + + private static boolean server( + final CompletableFuture<SocketAddress> boundAddress, + final int packetBufferSize, + final PeerAction peerAction, + final CountDownLatch serversWaiting, + final SSLEngine engine) { + try (final ServerSocketChannel boundChannel = ServerSocketChannel.open()) { + final InetSocketAddress bindAddress = + new InetSocketAddress(InetAddress.getLoopbackAddress(), 0); + boundChannel.bind(bindAddress); + boundAddress.complete(boundChannel.getLocalAddress()); + try (final SocketChannel acceptedChannel = boundChannel.accept()) { + final ByteBuffer peerNetData = + ByteBuffer.allocateDirect(packetBufferSize); + + final NioSslEngine filter = new NioSslEngine(engine, bufferPool); + + final boolean result = + peerAction.apply(acceptedChannel, filter, peerNetData); + + filter.close(acceptedChannel); + + return result; + } + } catch (IOException e) { + printException("In server:", e); + throw new RuntimeException(e); + } finally { + serversWaiting.countDown(); + } + } + + private static void printException(final String context, final Exception e) { + System.out.println(context + "\n"); + e.printStackTrace(); + } + + private static Properties createKeystoreAndTruststore() + throws GeneralSecurityException, IOException { + final CertificateMaterial ca = new CertificateBuilder() + .commonName("Test CA") + .isCA() + .generate(); + + CertificateMaterial serverCertificate = new CertificateBuilder() + .commonName("server") + .issuedBy(ca) + .generate(); + + final CertStores serverStore = CertStores.serverStore(); + serverStore.withCertificate("server", serverCertificate); + serverStore.trust("ca", ca); + return serverStore.propertiesWith("all", true, false); Review comment: The call to `CertStores.propertiesWith()` here creates only `TempFile`s which furthermore are flagged `deleteOnExit()`. Is that really not sufficient? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
