Bill commented on a change in pull request #7449: URL: https://github.com/apache/geode/pull/7449#discussion_r836875596
########## 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); Review comment: I fixed this comment. ########## 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; Review comment: fixed. -- 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