From 18a1bca385d921292369650f3299b5ccb2e2f78d Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 19 May 2020 12:36:57 +0300 Subject: [PATCH 01/31] KAFKA-9320: Initial commit. --- .../kafka/common/config/SslConfigs.java | 15 ++++++++-- .../kafka/common/network/KafkaChannel.java | 1 + .../common/network/SslTransportLayerTest.java | 28 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java index f114e66f8ffbf..816b2079ee857 100644 --- a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java +++ b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java @@ -17,6 +17,7 @@ package org.apache.kafka.common.config; import org.apache.kafka.common.config.internals.BrokerSecurityConfigs; +import org.apache.kafka.common.utils.Java; import org.apache.kafka.common.utils.Utils; import javax.net.ssl.KeyManagerFactory; @@ -53,7 +54,7 @@ public class SslConfigs { + "Allowed values in recent JVMs are TLSv1.2 and TLSv1.3. TLS, TLSv1.1, SSL, SSLv2 and SSLv3 " + "may be supported in older JVMs, but their usage is discouraged due to known security vulnerabilities."; - public static final String DEFAULT_SSL_PROTOCOL = "TLSv1.2"; + public static final String DEFAULT_SSL_PROTOCOL; public static final String SSL_PROVIDER_CONFIG = "ssl.provider"; public static final String SSL_PROVIDER_DOC = "The name of the security provider used for SSL connections. Default value is the default security provider of the JVM."; @@ -64,7 +65,17 @@ public class SslConfigs { public static final String SSL_ENABLED_PROTOCOLS_CONFIG = "ssl.enabled.protocols"; public static final String SSL_ENABLED_PROTOCOLS_DOC = "The list of protocols enabled for SSL connections."; - public static final String DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2"; + public static final String DEFAULT_SSL_ENABLED_PROTOCOLS; + + static { + if (Java.IS_JAVA11_COMPATIBLE) { + DEFAULT_SSL_PROTOCOL = "TLSv1.3"; + DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2,TLSv1.3"; + } else { + DEFAULT_SSL_PROTOCOL = "TLSv1.2"; + DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2"; + } + } public static final String SSL_KEYSTORE_TYPE_CONFIG = "ssl.keystore.type"; public static final String SSL_KEYSTORE_TYPE_DOC = "The file format of the key store file. " diff --git a/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java b/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java index 4e4edd47adb3c..586d1b9ad0be2 100644 --- a/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java +++ b/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java @@ -177,6 +177,7 @@ public void prepare() throws AuthenticationException, IOException { authenticator.authenticate(); } } catch (AuthenticationException e) { + e.printStackTrace(); // Clients are notified of authentication exceptions to enable operations to be terminated // without retries. Other errors are handled as network exceptions in Selector. String remoteDesc = remoteAddress != null ? remoteAddress.toString() : null; diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 0e793b0771384..f891a6f88c4a4 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -622,6 +622,34 @@ public void testUnsupportedTLSVersion() throws Exception { server.verifyAuthenticationMetrics(0, 1); } + /** + * Tests that connections can be made with TLSv1.2 and custom cipher suite. + */ + @Test + public void testCiphersSuiteForTLSv1_2() throws Exception { + String node = "0"; + SSLContext context = SSLContext.getInstance(tlsProtocol); + context.init(null, null, null); + + //Note, that only some ciphers works out of the box. Others requires additional configuration. + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + server = createEchoServer(SecurityProtocol.SSL); + + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + createSelector(sslClientConfigs); + InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); + selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); + + NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.READY); + server.verifyAuthenticationMetrics(1, 0); + } + /** * Tests that connections cannot be made with unsupported TLS cipher suites */ From 1076e5187fa045ec8198842e109b3afc14b6c059 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 19 May 2020 22:17:19 +0300 Subject: [PATCH 02/31] KAFKA-9320: Initial commit. --- .../main/java/org/apache/kafka/common/config/SslConfigs.java | 4 +--- .../java/org/apache/kafka/common/network/KafkaChannel.java | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java index 816b2079ee857..f6f006f1d1c53 100644 --- a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java +++ b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java @@ -54,7 +54,7 @@ public class SslConfigs { + "Allowed values in recent JVMs are TLSv1.2 and TLSv1.3. TLS, TLSv1.1, SSL, SSLv2 and SSLv3 " + "may be supported in older JVMs, but their usage is discouraged due to known security vulnerabilities."; - public static final String DEFAULT_SSL_PROTOCOL; + public static final String DEFAULT_SSL_PROTOCOL = "TLSv1.2"; public static final String SSL_PROVIDER_CONFIG = "ssl.provider"; public static final String SSL_PROVIDER_DOC = "The name of the security provider used for SSL connections. Default value is the default security provider of the JVM."; @@ -69,10 +69,8 @@ public class SslConfigs { static { if (Java.IS_JAVA11_COMPATIBLE) { - DEFAULT_SSL_PROTOCOL = "TLSv1.3"; DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2,TLSv1.3"; } else { - DEFAULT_SSL_PROTOCOL = "TLSv1.2"; DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2"; } } diff --git a/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java b/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java index 586d1b9ad0be2..4e4edd47adb3c 100644 --- a/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java +++ b/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java @@ -177,7 +177,6 @@ public void prepare() throws AuthenticationException, IOException { authenticator.authenticate(); } } catch (AuthenticationException e) { - e.printStackTrace(); // Clients are notified of authentication exceptions to enable operations to be terminated // without retries. Other errors are handled as network exceptions in Selector. String remoteDesc = remoteAddress != null ? remoteAddress.toString() : null; From 7dec0d658a1972fff7d3cf37ab293c6b0460cdbc Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 20 May 2020 19:56:35 +0300 Subject: [PATCH 03/31] KAFKA-9320: Test added --- .../common/network/SslTransportLayerTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index f891a6f88c4a4..9d934c13d5887 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -622,6 +622,37 @@ public void testUnsupportedTLSVersion() throws Exception { server.verifyAuthenticationMetrics(0, 1); } + /** + * Tests that connections fails if TLSv1.3 enabled but cipher suite suitable only for TLSv1.2 used. + */ + @Test + public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { + if (!Java.IS_JAVA11_COMPATIBLE) + return; + + String node = "0"; + SSLContext context = SSLContext.getInstance(tlsProtocol); + context.init(null, null, null); + + //Note, that only some ciphers works out of the box. Others requires additional configuration. + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + server = createEchoServer(SecurityProtocol.SSL); + + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + createSelector(sslClientConfigs); + InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); + selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); + + NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.AUTHENTICATION_FAILED); + server.verifyAuthenticationMetrics(0, 1); + } + /** * Tests that connections can be made with TLSv1.2 and custom cipher suite. */ From f6afbb92ff2b5255e18ce76af2c1c20f6cef6eee Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 20 May 2020 20:08:11 +0300 Subject: [PATCH 04/31] KAFKA-9320: Test added --- .../common/network/SslTransportLayerTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 9d934c13d5887..03c2fca335be7 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -653,6 +653,24 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { server.verifyAuthenticationMetrics(0, 1); } + /** + * Tests that connections can be made with TLSv1.2 and custom cipher suite. + */ + @Test + public void testCiphersSuiteFailForServerTLSv1_2_ClientTLSv1_3() throws Exception { + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.2")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + server = createEchoServer(SecurityProtocol.SSL); + + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + + checkAuthentiationFailed("0", "TLSv1.3"); + } + /** * Tests that connections can be made with TLSv1.2 and custom cipher suite. */ From 142e487e9eea558d145a9487fcd054753ea50615 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 20 May 2020 20:10:10 +0300 Subject: [PATCH 05/31] KAFKA-9320: Test added --- .../kafka/common/network/SslTransportLayerTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 03c2fca335be7..35602d0aa94ec 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -630,7 +630,6 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { if (!Java.IS_JAVA11_COMPATIBLE) return; - String node = "0"; SSLContext context = SSLContext.getInstance(tlsProtocol); context.init(null, null, null); @@ -642,14 +641,10 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); server = createEchoServer(SecurityProtocol.SSL); - sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); - createSelector(sslClientConfigs); - InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); - selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.AUTHENTICATION_FAILED); + checkAuthentiationFailed("0", "TLSv1.3"); server.verifyAuthenticationMetrics(0, 1); } @@ -658,6 +653,9 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { */ @Test public void testCiphersSuiteFailForServerTLSv1_2_ClientTLSv1_3() throws Exception { + if (!Java.IS_JAVA11_COMPATIBLE) + return; + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); From ac448d173f5ce727d3aee02ef4dbac15a38f28ff Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 20 May 2020 20:21:44 +0300 Subject: [PATCH 06/31] KAFKA-9320: Test added --- .../main/java/org/apache/kafka/common/config/SslConfigs.java | 4 +++- .../apache/kafka/common/network/SslTransportLayerTest.java | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java index f6f006f1d1c53..816b2079ee857 100644 --- a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java +++ b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java @@ -54,7 +54,7 @@ public class SslConfigs { + "Allowed values in recent JVMs are TLSv1.2 and TLSv1.3. TLS, TLSv1.1, SSL, SSLv2 and SSLv3 " + "may be supported in older JVMs, but their usage is discouraged due to known security vulnerabilities."; - public static final String DEFAULT_SSL_PROTOCOL = "TLSv1.2"; + public static final String DEFAULT_SSL_PROTOCOL; public static final String SSL_PROVIDER_CONFIG = "ssl.provider"; public static final String SSL_PROVIDER_DOC = "The name of the security provider used for SSL connections. Default value is the default security provider of the JVM."; @@ -69,8 +69,10 @@ public class SslConfigs { static { if (Java.IS_JAVA11_COMPATIBLE) { + DEFAULT_SSL_PROTOCOL = "TLSv1.3"; DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2,TLSv1.3"; } else { + DEFAULT_SSL_PROTOCOL = "TLSv1.2"; DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.2"; } } diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 35602d0aa94ec..a91bc8f9a5db6 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -591,10 +591,7 @@ public void testUnsupportedCipher() throws Exception { createSelector(sslClientConfigs); checkAuthentiationFailed("1", "TLSv1.1"); - server.verifyAuthenticationMetrics(0, 1); - checkAuthentiationFailed("2", "TLSv1"); - server.verifyAuthenticationMetrics(0, 2); } } From e1287c610f938bf0c379fa892cb0a13dee27a444 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 25 May 2020 17:58:50 +0300 Subject: [PATCH 07/31] KAFKA-9320: SslVersionsTransportLayerTest added. --- .../common/network/SslTransportLayerTest.java | 6 +- .../SslVersionsTransportLayerTest.java | 130 ++++++++++++++++++ 2 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index a91bc8f9a5db6..b019e8b9ebe7c 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -1322,7 +1322,7 @@ private interface FailureAction { void run() throws IOException; } - private static class TestSslChannelBuilder extends SslChannelBuilder { + public static class TestSslChannelBuilder extends SslChannelBuilder { private Integer netReadBufSizeOverride; private Integer netWriteBufSizeOverride; @@ -1365,7 +1365,7 @@ protected TestSslTransportLayer newTransportLayer(String id, SelectionKey key, S *
  • Delayed writes to test handshake failure notifications to peer
    • * */ - class TestSslTransportLayer extends SslTransportLayer { + public class TestSslTransportLayer extends SslTransportLayer { private final ResizeableBufferSize netReadBufSize; private final ResizeableBufferSize netWriteBufSize; @@ -1433,7 +1433,7 @@ private void resetDelayedFlush() { } } - private static class ResizeableBufferSize { + public static class ResizeableBufferSize { private Integer bufSizeOverride; ResizeableBufferSize(Integer bufSizeOverride) { this.bufSizeOverride = bufSizeOverride; diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java new file mode 100644 index 0000000000000..4481405b96cf2 --- /dev/null +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -0,0 +1,130 @@ +/* + * 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.kafka.common.network; + +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.kafka.common.config.SslConfigs; +import org.apache.kafka.common.metrics.Metrics; +import org.apache.kafka.common.security.TestSecurityConfig; +import org.apache.kafka.common.security.auth.SecurityProtocol; +import org.apache.kafka.common.utils.Java; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Time; +import org.apache.kafka.test.TestUtils; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +/** + * Tests for the SSL transport layer. + * Checks different versions of the protocol usage on the server and client. + */ +@RunWith(value = Parameterized.class) +public class SslVersionsTransportLayerTest { + private static final int BUFFER_SIZE = 4 * 1024; + private static final Time TIME = Time.SYSTEM; + + private final String tlsServerProtocol; + private final String tlsClientProtocol; + + @Parameterized.Parameters(name = "tlsServerProtocol={0},tlsClientProtocol={1}") + public static Collection data() { + List values = new ArrayList<>(); + values.add(new Object[] {"TLSv1.2", "TLSv1.2"}); + if (Java.IS_JAVA11_COMPATIBLE) { + values.add(new Object[] {"TLSv1.2", "TLSv1.3"}); + values.add(new Object[] {"TLSv1.3", "TLSv1.2"}); + values.add(new Object[] {"TLSv1.3", "TLSv1.3"}); + } + return values; + } + + public SslVersionsTransportLayerTest(String tlsServerProtocol, String tlsClientProtocol) { + this.tlsServerProtocol = tlsServerProtocol; + this.tlsClientProtocol = tlsClientProtocol; + } + + /** + * Tests that connection success with the default TLS version. + */ + @Test + public void testTLSDefaults() throws Exception { + // Create certificates for use by client and server. Add server cert to client truststore and vice versa. + CertStores serverCertStores = new CertStores(true, "server", "localhost"); + CertStores clientCertStores = new CertStores(false, "client", "localhost"); + + Map sslClientConfigs = getTrustingConfig(clientCertStores, serverCertStores, tlsClientProtocol); + Map sslServerConfigs = getTrustingConfig(serverCertStores, clientCertStores, tlsServerProtocol); + + NioEchoServer server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), + SecurityProtocol.SSL, + new TestSecurityConfig(sslServerConfigs), + null, + TIME); + Selector selector = createSelector(sslClientConfigs); + + String node = "0"; + selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); + + if (tlsServerProtocol.equals(tlsClientProtocol)) { + NetworkTestUtils.waitForChannelReady(selector, node); + + int msgSz = 1024 * 1024; + String message = TestUtils.randomString(msgSz); + selector.send(new NetworkSend(node, ByteBuffer.wrap(message.getBytes()))); + while (selector.completedReceives().isEmpty()) { + selector.poll(100L); + } + int totalBytes = msgSz + 4; // including 4-byte size + server.waitForMetric("incoming-byte", totalBytes); + server.waitForMetric("outgoing-byte", totalBytes); + server.waitForMetric("request", 1); + server.waitForMetric("response", 1); + } else { + NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.AUTHENTICATION_FAILED); + } + } + + public static Map sslConfig(String tlsServerProtocol) { + Map sslConfig = new HashMap<>(); + + sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsServerProtocol); + sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList(tlsServerProtocol)); + + return sslConfig; + } + + public static Map getTrustingConfig(CertStores certStores, CertStores peerCertStores, String tlsProtocol) { + Map configs = certStores.getTrustingConfig(peerCertStores); + configs.putAll(sslConfig(tlsProtocol)); + return configs; + } + + private Selector createSelector(Map sslClientConfigs) { + SslTransportLayerTest.TestSslChannelBuilder channelBuilder = new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); + channelBuilder.configureBufferSizes(null, null, null); + channelBuilder.configure(sslClientConfigs); + return new Selector(100 * 5000, new Metrics(), TIME, "MetricGroup", channelBuilder, new LogContext()); + } +} From b310e6052600fde7db7187dfc781e5f650d56bf0 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 26 May 2020 11:54:36 +0300 Subject: [PATCH 08/31] KAFKA-9320: Tests fix. --- core/src/test/scala/unit/kafka/network/SocketServerTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/scala/unit/kafka/network/SocketServerTest.scala b/core/src/test/scala/unit/kafka/network/SocketServerTest.scala index 6743dc8bab6bd..6fed2db3602d9 100644 --- a/core/src/test/scala/unit/kafka/network/SocketServerTest.scala +++ b/core/src/test/scala/unit/kafka/network/SocketServerTest.scala @@ -170,7 +170,7 @@ class SocketServerTest { } private def sslClientSocket(port: Int): Socket = { - val sslContext = SSLContext.getInstance("TLSv1.2") + val sslContext = SSLContext.getInstance(TestSslUtils.DEFAULT_TLS_PROTOCOL_FOR_TESTS) sslContext.init(null, Array(TestUtils.trustAllCerts), new java.security.SecureRandom()) val socketFactory = sslContext.getSocketFactory val socket = socketFactory.createSocket("localhost", port) From 518eb77b5bf4075db449bc4cf0aff89981987eae Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 26 May 2020 19:49:20 +0300 Subject: [PATCH 09/31] KAFKA-9320: system tests updated. --- tests/docker/run_tests.sh | 2 +- .../benchmarks/core/benchmark_test.py | 44 ++++++++++--------- .../sanity_checks/test_console_consumer.py | 9 ++-- tests/kafkatest/services/kafka/kafka.py | 21 +++++---- .../services/kafka/templates/kafka.properties | 5 ++- .../services/kafka_log4j_appender.py | 4 +- .../services/log_compaction_tester.py | 5 ++- .../services/replica_verification_tool.py | 5 ++- .../services/security/security_config.py | 16 ++++++- .../tests/core/consumer_group_command_test.py | 23 +++++----- .../kafkatest/tests/core/mirror_maker_test.py | 18 +++++--- .../tests/tools/log4j_appender_test.py | 19 ++++---- 12 files changed, 103 insertions(+), 68 deletions(-) diff --git a/tests/docker/run_tests.sh b/tests/docker/run_tests.sh index 063e24d178765..6278f9ac64582 100755 --- a/tests/docker/run_tests.sh +++ b/tests/docker/run_tests.sh @@ -30,6 +30,6 @@ if [ "$REBUILD" == "t" ]; then fi if ${SCRIPT_DIR}/ducker-ak ssh | grep -q '(none)'; then - ${SCRIPT_DIR}/ducker-ak up -n "${KAFKA_NUM_CONTAINERS}" || die "ducker-ak up failed" + ${SCRIPT_DIR}/ducker-ak up -j 'openjdk:11' -n "${KAFKA_NUM_CONTAINERS}" || die "ducker-ak up failed" fi ${SCRIPT_DIR}/ducker-ak test ${TC_PATHS} ${_DUCKTAPE_OPTIONS} || die "ducker-ak test failed" diff --git a/tests/kafkatest/benchmarks/core/benchmark_test.py b/tests/kafkatest/benchmarks/core/benchmark_test.py index 2b4ff87bb35db..6bab304f3037b 100644 --- a/tests/kafkatest/benchmarks/core/benchmark_test.py +++ b/tests/kafkatest/benchmarks/core/benchmark_test.py @@ -55,12 +55,12 @@ def __init__(self, test_context): def setUp(self): self.zk.start() - def start_kafka(self, security_protocol, interbroker_security_protocol, version): + def start_kafka(self, security_protocol, interbroker_security_protocol, version, tls_version=None): self.kafka = KafkaService( self.test_context, self.num_brokers, self.zk, security_protocol=security_protocol, interbroker_security_protocol=interbroker_security_protocol, topics=self.topics, - version=version) + version=version, tls_version=tls_version) self.kafka.log_level = "INFO" # We don't DEBUG logging here self.kafka.start() @@ -68,11 +68,12 @@ def start_kafka(self, security_protocol, interbroker_security_protocol, version) @parametrize(acks=1, topic=TOPIC_REP_ONE) @parametrize(acks=1, topic=TOPIC_REP_THREE) @parametrize(acks=-1, topic=TOPIC_REP_THREE) - @matrix(acks=[1], topic=[TOPIC_REP_THREE], message_size=[10, 100, 1000, 10000, 100000], compression_type=["none", "snappy"], security_protocol=['PLAINTEXT', 'SSL']) + @matrix(acks=[1], topic=[TOPIC_REP_THREE], message_size=[10, 100, 1000, 10000, 100000], compression_type=["none", "snappy"], security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @matrix(acks=[1], topic=[TOPIC_REP_THREE], message_size=[10, 100, 1000, 10000, 100000], compression_type=["none", "snappy"], security_protocol=['PLAINTEXT']) @cluster(num_nodes=7) @parametrize(acks=1, topic=TOPIC_REP_THREE, num_producers=3) def test_producer_throughput(self, acks, topic, num_producers=1, message_size=DEFAULT_RECORD_SIZE, - compression_type="none", security_protocol='PLAINTEXT', client_version=str(DEV_BRANCH), + compression_type="none", security_protocol='PLAINTEXT', tls_version=None, client_version=str(DEV_BRANCH), broker_version=str(DEV_BRANCH)): """ Setup: 1 node zk + 3 node kafka cluster @@ -85,7 +86,7 @@ def test_producer_throughput(self, acks, topic, num_producers=1, message_size=DE client_version = KafkaVersion(client_version) broker_version = KafkaVersion(broker_version) self.validate_versions(client_version, broker_version) - self.start_kafka(security_protocol, security_protocol, broker_version) + self.start_kafka(security_protocol, security_protocol, broker_version, tls_version) # Always generate the same total amount of data nrecords = int(self.target_data_size / message_size) @@ -101,9 +102,10 @@ def test_producer_throughput(self, acks, topic, num_producers=1, message_size=DE return compute_aggregate_throughput(self.producer) @cluster(num_nodes=5) - @parametrize(security_protocol='SSL', interbroker_security_protocol='PLAINTEXT') - @matrix(security_protocol=['PLAINTEXT', 'SSL'], compression_type=["none", "snappy"]) - def test_long_term_producer_throughput(self, compression_type="none", security_protocol='PLAINTEXT', + @matrix(security_protocol=['SSL'], interbroker_security_protocol=['PLAINTEXT'], tls_version=['TLSv1.2', 'TLSv1.3'], compression_type=["none", "snappy"]) + @matrix(security_protocol=['PLAINTEXT'], compression_type=["none", "snappy"]) + def test_long_term_producer_throughput(self, compression_type="none", + security_protocol='PLAINTEXT', tls_version=None, interbroker_security_protocol=None, client_version=str(DEV_BRANCH), broker_version=str(DEV_BRANCH)): """ @@ -119,7 +121,7 @@ def test_long_term_producer_throughput(self, compression_type="none", security_p self.validate_versions(client_version, broker_version) if interbroker_security_protocol is None: interbroker_security_protocol = security_protocol - self.start_kafka(security_protocol, interbroker_security_protocol, broker_version) + self.start_kafka(security_protocol, interbroker_security_protocol, broker_version, tls_version) self.producer = ProducerPerformanceService( self.test_context, 1, self.kafka, topic=TOPIC_REP_THREE, num_records=self.msgs_large, record_size=DEFAULT_RECORD_SIZE, @@ -157,11 +159,11 @@ def test_long_term_producer_throughput(self, compression_type="none", security_p return data @cluster(num_nodes=5) - @parametrize(security_protocol='SSL', interbroker_security_protocol='PLAINTEXT') - @matrix(security_protocol=['PLAINTEXT', 'SSL'], compression_type=["none", "snappy"]) + @matrix(security_protocol=['SSL'], interbroker_security_protocol=['PLAINTEXT'], tls_version=['TLSv1.2', 'TLSv1.3'], compression_type=["none", "snappy"]) + @matrix(security_protocol=['PLAINTEXT'], compression_type=["none", "snappy"]) @cluster(num_nodes=6) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL'], compression_type=["none", "snappy"]) - def test_end_to_end_latency(self, compression_type="none", security_protocol="PLAINTEXT", + def test_end_to_end_latency(self, compression_type="none", security_protocol="PLAINTEXT", tls_version=None, interbroker_security_protocol=None, client_version=str(DEV_BRANCH), broker_version=str(DEV_BRANCH)): """ @@ -178,7 +180,7 @@ def test_end_to_end_latency(self, compression_type="none", security_protocol="PL self.validate_versions(client_version, broker_version) if interbroker_security_protocol is None: interbroker_security_protocol = security_protocol - self.start_kafka(security_protocol, interbroker_security_protocol, broker_version) + self.start_kafka(security_protocol, interbroker_security_protocol, broker_version, tls_version) self.logger.info("BENCHMARK: End to end latency") self.perf = EndToEndLatencyService( self.test_context, 1, self.kafka, @@ -189,9 +191,9 @@ def test_end_to_end_latency(self, compression_type="none", security_protocol="PL return latency(self.perf.results[0]['latency_50th_ms'], self.perf.results[0]['latency_99th_ms'], self.perf.results[0]['latency_999th_ms']) @cluster(num_nodes=6) - @parametrize(security_protocol='SSL', interbroker_security_protocol='PLAINTEXT') - @matrix(security_protocol=['PLAINTEXT', 'SSL'], compression_type=["none", "snappy"]) - def test_producer_and_consumer(self, compression_type="none", security_protocol="PLAINTEXT", + @matrix(security_protocol=['SSL'], interbroker_security_protocol=['PLAINTEXT'], tls_version=['TLSv1.2', 'TLSv1.3'], compression_type=["none", "snappy"]) + @matrix(security_protocol=['PLAINTEXT'], compression_type=["none", "snappy"]) + def test_producer_and_consumer(self, compression_type="none", security_protocol="PLAINTEXT", tls_version=None, interbroker_security_protocol=None, client_version=str(DEV_BRANCH), broker_version=str(DEV_BRANCH)): """ @@ -207,7 +209,7 @@ def test_producer_and_consumer(self, compression_type="none", security_protocol= self.validate_versions(client_version, broker_version) if interbroker_security_protocol is None: interbroker_security_protocol = security_protocol - self.start_kafka(security_protocol, interbroker_security_protocol, broker_version) + self.start_kafka(security_protocol, interbroker_security_protocol, broker_version, tls_version) num_records = 10 * 1000 * 1000 # 10e6 self.producer = ProducerPerformanceService( @@ -236,9 +238,9 @@ def test_producer_and_consumer(self, compression_type="none", security_protocol= return data @cluster(num_nodes=6) - @parametrize(security_protocol='SSL', interbroker_security_protocol='PLAINTEXT') - @matrix(security_protocol=['PLAINTEXT', 'SSL'], compression_type=["none", "snappy"]) - def test_consumer_throughput(self, compression_type="none", security_protocol="PLAINTEXT", + @matrix(security_protocol=['SSL'], interbroker_security_protocol=['PLAINTEXT'], tls_version=['TLSv1.2', 'TLSv1.3'], compression_type=["none", "snappy"]) + @matrix(security_protocol=['PLAINTEXT'], compression_type=["none", "snappy"]) + def test_consumer_throughput(self, compression_type="none", security_protocol="PLAINTEXT", tls_version=None, interbroker_security_protocol=None, num_consumers=1, client_version=str(DEV_BRANCH), broker_version=str(DEV_BRANCH)): """ @@ -250,7 +252,7 @@ def test_consumer_throughput(self, compression_type="none", security_protocol="P self.validate_versions(client_version, broker_version) if interbroker_security_protocol is None: interbroker_security_protocol = security_protocol - self.start_kafka(security_protocol, interbroker_security_protocol, broker_version) + self.start_kafka(security_protocol, interbroker_security_protocol, broker_version, tls_version) num_records = 10 * 1000 * 1000 # 10e6 # seed kafka w/messages diff --git a/tests/kafkatest/sanity_checks/test_console_consumer.py b/tests/kafkatest/sanity_checks/test_console_consumer.py index acf1184e0595d..152cabbb88fab 100644 --- a/tests/kafkatest/sanity_checks/test_console_consumer.py +++ b/tests/kafkatest/sanity_checks/test_console_consumer.py @@ -15,7 +15,7 @@ import time -from ducktape.mark import matrix +from ducktape.mark import matrix, defaults from ducktape.mark import parametrize from ducktape.mark.resource import cluster from ducktape.tests.test import Test @@ -44,19 +44,22 @@ def setUp(self): self.zk.start() @cluster(num_nodes=3) - @matrix(security_protocol=['PLAINTEXT', 'SSL']) + @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @parametrize(security_protocol='PLAINTEXT') @cluster(num_nodes=4) @matrix(security_protocol=['SASL_SSL'], sasl_mechanism=['PLAIN', 'SCRAM-SHA-256', 'SCRAM-SHA-512']) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_lifecycle(self, security_protocol, sasl_mechanism='GSSAPI'): + def test_lifecycle(self, security_protocol, tls_version=None, sasl_mechanism='GSSAPI'): """Check that console consumer starts/stops properly, and that we are capturing log output.""" self.kafka.security_protocol = security_protocol + self.kafka.tls_version = tls_version self.kafka.client_sasl_mechanism = sasl_mechanism self.kafka.interbroker_sasl_mechanism = sasl_mechanism self.kafka.start() self.consumer.security_protocol = security_protocol + self.consumer.tls_version = tls_version t0 = time.time() self.consumer.start() diff --git a/tests/kafkatest/services/kafka/kafka.py b/tests/kafkatest/services/kafka/kafka.py index ec44fc9906d7f..58bd47a7bf839 100644 --- a/tests/kafkatest/services/kafka/kafka.py +++ b/tests/kafkatest/services/kafka/kafka.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import collections import json import os.path import re @@ -31,7 +30,7 @@ from kafkatest.services.security.minikdc import MiniKdc from kafkatest.services.security.listener_security_config import ListenerSecurityConfig from kafkatest.services.security.security_config import SecurityConfig -from kafkatest.version import DEV_BRANCH, LATEST_0_10_0, LATEST_0_9, LATEST_0_8_2 +from kafkatest.version import DEV_BRANCH from kafkatest.services.kafka.util import fix_opts_for_new_jvm @@ -95,17 +94,20 @@ class KafkaService(KafkaPathResolverMixin, JmxMixin, Service): "collect_default": True} } - def __init__(self, context, num_nodes, zk, security_protocol=SecurityConfig.PLAINTEXT, interbroker_security_protocol=SecurityConfig.PLAINTEXT, + def __init__(self, context, num_nodes, zk, security_protocol=SecurityConfig.PLAINTEXT, + interbroker_security_protocol=SecurityConfig.PLAINTEXT, client_sasl_mechanism=SecurityConfig.SASL_MECHANISM_GSSAPI, interbroker_sasl_mechanism=SecurityConfig.SASL_MECHANISM_GSSAPI, authorizer_class_name=None, topics=None, version=DEV_BRANCH, jmx_object_names=None, jmx_attributes=None, zk_connect_timeout=5000, zk_session_timeout=6000, server_prop_overides=None, zk_chroot=None, zk_client_secure=False, - listener_security_config=ListenerSecurityConfig(), per_node_server_prop_overrides=None, extra_kafka_opts=""): + listener_security_config=ListenerSecurityConfig(), per_node_server_prop_overrides=None, + extra_kafka_opts="", tls_version=None): """ :param context: test context :param ZookeeperService zk: :param dict topics: which topics to create automatically :param str security_protocol: security protocol for clients to use + :param str tls_version: version of the TLS protocol. :param str interbroker_security_protocol: security protocol to use for broker-to-broker communication :param str client_sasl_mechanism: sasl mechanism for clients to use :param str interbroker_sasl_mechanism: sasl mechanism to use for broker-to-broker communication @@ -129,6 +131,7 @@ def __init__(self, context, num_nodes, zk, security_protocol=SecurityConfig.PLAI self.zk = zk self.security_protocol = security_protocol + self.tls_version = tls_version self.client_sasl_mechanism = client_sasl_mechanism self.topics = topics self.minikdc = None @@ -215,7 +218,8 @@ def security_config(self): zk_sasl=self.zk.zk_sasl, zk_tls=self.zk_client_secure, client_sasl_mechanism=self.client_sasl_mechanism, interbroker_sasl_mechanism=self.interbroker_sasl_mechanism, - listener_security_config=self.listener_security_config) + listener_security_config=self.listener_security_config, + tls_version=self.tls_version) for port in self.port_mappings.values(): if port.open: config.enable_security_protocol(port.security_protocol) @@ -354,15 +358,16 @@ def start_cmd(self, node): def start_node(self, node, timeout_sec=60): node.account.mkdirs(KafkaService.PERSISTENT_ROOT) + + self.security_config.setup_node(node) + self.security_config.setup_credentials(node, self.path, self.zk_connect_setting(), broker=True) + prop_file = self.prop_file(node) self.logger.info("kafka.properties:") self.logger.info(prop_file) node.account.create_file(KafkaService.CONFIG_FILE, prop_file) node.account.create_file(self.LOG4J_CONFIG, self.render('log4j.properties', log_dir=KafkaService.OPERATIONAL_LOG_DIR)) - self.security_config.setup_node(node) - self.security_config.setup_credentials(node, self.path, self.zk_connect_setting(), broker=True) - cmd = self.start_cmd(node) self.logger.debug("Attempting to start KafkaService on %s with command: %s" % (str(node.account), cmd)) with node.account.monitor_log(KafkaService.STDOUT_STDERR_CAPTURE) as monitor: diff --git a/tests/kafkatest/services/kafka/templates/kafka.properties b/tests/kafkatest/services/kafka/templates/kafka.properties index 9795eacbc3908..0c028aac7073c 100644 --- a/tests/kafkatest/services/kafka/templates/kafka.properties +++ b/tests/kafkatest/services/kafka/templates/kafka.properties @@ -44,7 +44,10 @@ listener.name.{{ interbroker_listener.name.lower() }}.{{ k }}={{ v }} {% endif %} {% endfor %} {% endif %} - +{% if security_config.tls_version is not none %} +ssl.enabled.protocols={{ security_config.tls_version }} +ssl.protocol={{ security_config.tls_version }} +{% endif %} ssl.keystore.location=/mnt/security/test.keystore.jks ssl.keystore.password=test-ks-passwd ssl.key.password=test-ks-passwd diff --git a/tests/kafkatest/services/kafka_log4j_appender.py b/tests/kafkatest/services/kafka_log4j_appender.py index d6908aa93d9dc..d47914e738de5 100644 --- a/tests/kafkatest/services/kafka_log4j_appender.py +++ b/tests/kafkatest/services/kafka_log4j_appender.py @@ -28,14 +28,14 @@ class KafkaLog4jAppender(KafkaPathResolverMixin, BackgroundThreadService): "collect_default": False} } - def __init__(self, context, num_nodes, kafka, topic, max_messages=-1, security_protocol="PLAINTEXT"): + def __init__(self, context, num_nodes, kafka, topic, max_messages=-1, security_protocol="PLAINTEXT", tls_version=None): super(KafkaLog4jAppender, self).__init__(context, num_nodes) self.kafka = kafka self.topic = topic self.max_messages = max_messages self.security_protocol = security_protocol - self.security_config = SecurityConfig(self.context, security_protocol) + self.security_config = SecurityConfig(self.context, security_protocol, tls_version=tls_version) self.stop_timeout_sec = 30 def _worker(self, idx, node): diff --git a/tests/kafkatest/services/log_compaction_tester.py b/tests/kafkatest/services/log_compaction_tester.py index 4a19650ff2e1d..f927066557a46 100644 --- a/tests/kafkatest/services/log_compaction_tester.py +++ b/tests/kafkatest/services/log_compaction_tester.py @@ -33,12 +33,13 @@ class LogCompactionTester(KafkaPathResolverMixin, BackgroundThreadService): "collect_default": True} } - def __init__(self, context, kafka, security_protocol="PLAINTEXT", stop_timeout_sec=30): + def __init__(self, context, kafka, security_protocol="PLAINTEXT", tls_version=None, stop_timeout_sec=30): super(LogCompactionTester, self).__init__(context, 1) self.kafka = kafka self.security_protocol = security_protocol - self.security_config = SecurityConfig(self.context, security_protocol) + self.tls_version = tls_version + self.security_config = SecurityConfig(self.context, security_protocol, tls_version=tls_version) self.stop_timeout_sec = stop_timeout_sec self.log_compaction_completed = False diff --git a/tests/kafkatest/services/replica_verification_tool.py b/tests/kafkatest/services/replica_verification_tool.py index 8751797d6cf29..a233caee416b9 100644 --- a/tests/kafkatest/services/replica_verification_tool.py +++ b/tests/kafkatest/services/replica_verification_tool.py @@ -29,14 +29,15 @@ class ReplicaVerificationTool(KafkaPathResolverMixin, BackgroundThreadService): "collect_default": False} } - def __init__(self, context, num_nodes, kafka, topic, report_interval_ms, security_protocol="PLAINTEXT", stop_timeout_sec=30): + def __init__(self, context, num_nodes, kafka, topic, report_interval_ms, security_protocol="PLAINTEXT", tls_version=None, stop_timeout_sec=30): super(ReplicaVerificationTool, self).__init__(context, num_nodes) self.kafka = kafka self.topic = topic self.report_interval_ms = report_interval_ms self.security_protocol = security_protocol - self.security_config = SecurityConfig(self.context, security_protocol) + self.tls_version = tls_version + self.security_config = SecurityConfig(self.context, security_protocol, tls_version=tls_version) self.partition_lag = {} self.stop_timeout_sec = stop_timeout_sec diff --git a/tests/kafkatest/services/security/security_config.py b/tests/kafkatest/services/security/security_config.py index f70d7d282ef47..cff9884d94d6d 100644 --- a/tests/kafkatest/services/security/security_config.py +++ b/tests/kafkatest/services/security/security_config.py @@ -19,6 +19,8 @@ from tempfile import mkdtemp from shutil import rmtree from ducktape.template import TemplateRenderer + +from kafkatest.services.kafka.util import java_version from kafkatest.services.security.minikdc import MiniKdc from kafkatest.services.security.listener_security_config import ListenerSecurityConfig import itertools @@ -140,7 +142,7 @@ class SecurityConfig(TemplateRenderer): def __init__(self, context, security_protocol=None, interbroker_security_protocol=None, client_sasl_mechanism=SASL_MECHANISM_GSSAPI, interbroker_sasl_mechanism=SASL_MECHANISM_GSSAPI, zk_sasl=False, zk_tls=False, template_props="", static_jaas_conf=True, jaas_override_variables=None, - listener_security_config=ListenerSecurityConfig()): + listener_security_config=ListenerSecurityConfig(), tls_version=None): """ Initialize the security properties for the node and copy keystore and truststore to the remote node if the transport protocol @@ -176,6 +178,7 @@ def __init__(self, context, security_protocol=None, interbroker_security_protoco self.listener_security_config = listener_security_config self.properties = { 'security.protocol' : security_protocol, + 'tls.version' : tls_version, 'ssl.keystore.location' : SecurityConfig.KEYSTORE_PATH, 'ssl.keystore.password' : SecurityConfig.ssl_stores.keystore_passwd, 'ssl.key.password' : SecurityConfig.ssl_stores.key_passwd, @@ -201,13 +204,15 @@ def client_config(self, template_props="", node=None, jaas_override_variables=No template_props=template_props, static_jaas_conf=static_jaas_conf, jaas_override_variables=jaas_override_variables, - listener_security_config=self.listener_security_config) + listener_security_config=self.listener_security_config, + tls_version=self.tls_version) def enable_security_protocol(self, security_protocol): self.has_sasl = self.has_sasl or self.is_sasl(security_protocol) self.has_ssl = self.has_ssl or self.is_ssl(security_protocol) def setup_ssl(self, node): + node.account.ssh("mkdir -p %s" % SecurityConfig.CONFIG_DIR, allow_fail=False) node.account.copy_to(SecurityConfig.ssl_stores.truststore_path, SecurityConfig.TRUSTSTORE_PATH) SecurityConfig.ssl_stores.generate_and_copy_keystore(node) @@ -259,6 +264,9 @@ def setup_node(self, node): if self.has_sasl: self.setup_sasl(node) + if java_version(node) <= 9 and self.properties['tls.version'] == 'TLSv1.3': + self.properties.update({'tls.version': 'TLSv1.2'}) + def setup_credentials(self, node, path, zk_connect, broker): if broker: self.maybe_create_scram_credentials(node, zk_connect, path, self.interbroker_sasl_mechanism, @@ -303,6 +311,10 @@ def is_sasl_scram(self, sasl_mechanism): def security_protocol(self): return self.properties['security.protocol'] + @property + def tls_version(self): + return self.properties['tls.version'] + @property def client_sasl_mechanism(self): return self.properties['sasl.mechanism'] diff --git a/tests/kafkatest/tests/core/consumer_group_command_test.py b/tests/kafkatest/tests/core/consumer_group_command_test.py index 871e2761ade25..3b714f21a4f40 100644 --- a/tests/kafkatest/tests/core/consumer_group_command_test.py +++ b/tests/kafkatest/tests/core/consumer_group_command_test.py @@ -16,7 +16,7 @@ from ducktape.utils.util import wait_until from ducktape.tests.test import Test -from ducktape.mark import matrix +from ducktape.mark import matrix, parametrize from ducktape.mark.resource import cluster from kafkatest.services.zookeeper import ZookeeperService @@ -50,10 +50,11 @@ def __init__(self, test_context): def setUp(self): self.zk.start() - def start_kafka(self, security_protocol, interbroker_security_protocol): + def start_kafka(self, security_protocol, interbroker_security_protocol, tls_version=None): self.kafka = KafkaService( self.test_context, self.num_brokers, self.zk, security_protocol=security_protocol, + tls_version=tls_version, interbroker_security_protocol=interbroker_security_protocol, topics=self.topics) self.kafka.start() @@ -62,8 +63,8 @@ def start_consumer(self): consumer_timeout_ms=None) self.consumer.start() - def setup_and_verify(self, security_protocol, group=None): - self.start_kafka(security_protocol, security_protocol) + def setup_and_verify(self, security_protocol, tls_version=None, group=None): + self.start_kafka(security_protocol, security_protocol, tls_version) self.start_consumer() consumer_node = self.consumer.nodes[0] wait_until(lambda: self.consumer.alive(consumer_node), @@ -88,19 +89,21 @@ def setup_and_verify(self, security_protocol, group=None): self.consumer.stop() @cluster(num_nodes=3) - @matrix(security_protocol=['PLAINTEXT', 'SSL']) - def test_list_consumer_groups(self, security_protocol='PLAINTEXT'): + @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @parametrize(security_protocol='PLAINTEXT') + def test_list_consumer_groups(self, security_protocol='PLAINTEXT', tls_version=None): """ Tests if ConsumerGroupCommand is listing correct consumer groups :return: None """ - self.setup_and_verify(security_protocol) + self.setup_and_verify(security_protocol, tls_version) @cluster(num_nodes=3) - @matrix(security_protocol=['PLAINTEXT', 'SSL']) - def test_describe_consumer_group(self, security_protocol='PLAINTEXT'): + @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @parametrize(security_protocol='PLAINTEXT') + def test_describe_consumer_group(self, security_protocol='PLAINTEXT', tls_version=None): """ Tests if ConsumerGroupCommand is describing a consumer group correctly :return: None """ - self.setup_and_verify(security_protocol, group="test-consumer-group") + self.setup_and_verify(security_protocol, tls_version, group="test-consumer-group") diff --git a/tests/kafkatest/tests/core/mirror_maker_test.py b/tests/kafkatest/tests/core/mirror_maker_test.py index c33f103e50e9e..8f4c4f5e41563 100644 --- a/tests/kafkatest/tests/core/mirror_maker_test.py +++ b/tests/kafkatest/tests/core/mirror_maker_test.py @@ -58,10 +58,12 @@ def setUp(self): # Target cluster self.target_zk.start() - def start_kafka(self, security_protocol): + def start_kafka(self, security_protocol, tls_version=None): self.source_kafka.security_protocol = security_protocol + self.source_kafka.tls_version = tls_version self.source_kafka.interbroker_security_protocol = security_protocol self.target_kafka.security_protocol = security_protocol + self.target_kafka.tls_version = tls_version self.target_kafka.interbroker_security_protocol = security_protocol if self.source_kafka.security_config.has_sasl_kerberos: minikdc = MiniKdc(self.source_kafka.context, self.source_kafka.nodes + self.target_kafka.nodes) @@ -111,10 +113,11 @@ def wait_for_n_messages(self, n_messages=100): err_msg="Producer failed to produce %d messages in a reasonable amount of time." % n_messages) @cluster(num_nodes=7) - @matrix(security_protocol=['PLAINTEXT', 'SSL']) + @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @parametrize(security_protocol='PLAINTEXT') @cluster(num_nodes=8) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_simple_end_to_end(self, security_protocol): + def test_simple_end_to_end(self, security_protocol, tls_version=None): """ Test end-to-end behavior under non-failure conditions. @@ -126,7 +129,7 @@ def test_simple_end_to_end(self, security_protocol): - Consume messages from target. - Verify that number of consumed messages matches the number produced. """ - self.start_kafka(security_protocol) + self.start_kafka(security_protocol, tls_version) self.mirror_maker.start() mm_node = self.mirror_maker.nodes[0] @@ -136,10 +139,11 @@ def test_simple_end_to_end(self, security_protocol): self.mirror_maker.stop() @cluster(num_nodes=7) - @matrix(clean_shutdown=[True, False], security_protocol=['PLAINTEXT', 'SSL']) + @matrix(clean_shutdown=[True, False], security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @matrix(clean_shutdown=[True, False], security_protocol='PLAINTEXT') @cluster(num_nodes=8) @matrix(clean_shutdown=[True, False], security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_protocol='PLAINTEXT'): + def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_protocol='PLAINTEXT', tls_version=None): """ Test end-to-end behavior under failure conditions. @@ -157,7 +161,7 @@ def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_pro # the group until the previous session times out self.consumer.consumer_timeout_ms = 60000 - self.start_kafka(security_protocol) + self.start_kafka(security_protocol, tls_version) self.mirror_maker.offsets_storage = offsets_storage self.mirror_maker.start() diff --git a/tests/kafkatest/tests/tools/log4j_appender_test.py b/tests/kafkatest/tests/tools/log4j_appender_test.py index 03879e15d488c..af3fd87da8a0f 100644 --- a/tests/kafkatest/tests/tools/log4j_appender_test.py +++ b/tests/kafkatest/tests/tools/log4j_appender_test.py @@ -16,7 +16,7 @@ from ducktape.utils.util import wait_until from ducktape.tests.test import Test -from ducktape.mark import matrix +from ducktape.mark import matrix, parametrize from ducktape.mark.resource import cluster from kafkatest.services.zookeeper import ZookeeperService @@ -47,16 +47,16 @@ def __init__(self, test_context): def setUp(self): self.zk.start() - def start_kafka(self, security_protocol, interbroker_security_protocol): + def start_kafka(self, security_protocol, interbroker_security_protocol, tls_version=None): self.kafka = KafkaService( self.test_context, self.num_brokers, - self.zk, security_protocol=security_protocol, + self.zk, security_protocol=security_protocol, tls_version=tls_version, interbroker_security_protocol=interbroker_security_protocol, topics=self.topics) self.kafka.start() - def start_appender(self, security_protocol): + def start_appender(self, security_protocol, tls_version=None): self.appender = KafkaLog4jAppender(self.test_context, self.num_brokers, self.kafka, TOPIC, MAX_MESSAGES, - security_protocol=security_protocol) + security_protocol=security_protocol, tls_version=tls_version) self.appender.start() def custom_message_validator(self, msg): @@ -71,16 +71,17 @@ def start_consumer(self): self.consumer.start() @cluster(num_nodes=4) - @matrix(security_protocol=['PLAINTEXT', 'SSL']) + @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) + @parametrize(security_protocol='PLAINTEXT') @cluster(num_nodes=5) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_log4j_appender(self, security_protocol='PLAINTEXT'): + def test_log4j_appender(self, security_protocol='PLAINTEXT', tls_version=None): """ Tests if KafkaLog4jAppender is producing to Kafka topic :return: None """ - self.start_kafka(security_protocol, security_protocol) - self.start_appender(security_protocol) + self.start_kafka(security_protocol, security_protocol, tls_version) + self.start_appender(security_protocol, tls_version) self.appender.wait() self.start_consumer() From 5b5f37e458e0bfb0a017cabfa8ac8b677a6efbc5 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 27 May 2020 10:08:20 +0300 Subject: [PATCH 10/31] KAFKA-9320: system tests updated. --- tests/kafkatest/services/console_consumer.py | 1 - tests/kafkatest/services/kafka/kafka.py | 1 + tests/kafkatest/services/log_compaction_tester.py | 2 +- tests/kafkatest/services/replica_verification_tool.py | 3 ++- tests/kafkatest/services/zookeeper.py | 1 - tests/kafkatest/tests/core/consumer_group_command_test.py | 6 +++--- tests/kafkatest/tests/tools/log4j_appender_test.py | 1 - 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/kafkatest/services/console_consumer.py b/tests/kafkatest/services/console_consumer.py index 47c2456ebd1cf..14b450c0871e2 100644 --- a/tests/kafkatest/services/console_consumer.py +++ b/tests/kafkatest/services/console_consumer.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import itertools import os from ducktape.cluster.remoteaccount import RemoteCommandError diff --git a/tests/kafkatest/services/kafka/kafka.py b/tests/kafkatest/services/kafka/kafka.py index 58bd47a7bf839..9a4225e16bff6 100644 --- a/tests/kafkatest/services/kafka/kafka.py +++ b/tests/kafkatest/services/kafka/kafka.py @@ -51,6 +51,7 @@ def advertised_listener(self, node): def listener_security_protocol(self): return "%s:%s" % (self.name, self.security_protocol) + class KafkaService(KafkaPathResolverMixin, JmxMixin, Service): PERSISTENT_ROOT = "/mnt/kafka" STDOUT_STDERR_CAPTURE = os.path.join(PERSISTENT_ROOT, "server-start-stdout-stderr.log") diff --git a/tests/kafkatest/services/log_compaction_tester.py b/tests/kafkatest/services/log_compaction_tester.py index f927066557a46..cc6bf4fc2967e 100644 --- a/tests/kafkatest/services/log_compaction_tester.py +++ b/tests/kafkatest/services/log_compaction_tester.py @@ -33,7 +33,7 @@ class LogCompactionTester(KafkaPathResolverMixin, BackgroundThreadService): "collect_default": True} } - def __init__(self, context, kafka, security_protocol="PLAINTEXT", tls_version=None, stop_timeout_sec=30): + def __init__(self, context, kafka, security_protocol="PLAINTEXT", stop_timeout_sec=30, tls_version=None): super(LogCompactionTester, self).__init__(context, 1) self.kafka = kafka diff --git a/tests/kafkatest/services/replica_verification_tool.py b/tests/kafkatest/services/replica_verification_tool.py index a233caee416b9..13a1288001fd8 100644 --- a/tests/kafkatest/services/replica_verification_tool.py +++ b/tests/kafkatest/services/replica_verification_tool.py @@ -29,7 +29,8 @@ class ReplicaVerificationTool(KafkaPathResolverMixin, BackgroundThreadService): "collect_default": False} } - def __init__(self, context, num_nodes, kafka, topic, report_interval_ms, security_protocol="PLAINTEXT", tls_version=None, stop_timeout_sec=30): + def __init__(self, context, num_nodes, kafka, topic, report_interval_ms, security_protocol="PLAINTEXT", + stop_timeout_sec=30, tls_version=None): super(ReplicaVerificationTool, self).__init__(context, num_nodes) self.kafka = kafka diff --git a/tests/kafkatest/services/zookeeper.py b/tests/kafkatest/services/zookeeper.py index b8cb9b69cbd82..51f2b31de2dcf 100644 --- a/tests/kafkatest/services/zookeeper.py +++ b/tests/kafkatest/services/zookeeper.py @@ -16,7 +16,6 @@ import os import re -import time from ducktape.services.service import Service from ducktape.utils.util import wait_until diff --git a/tests/kafkatest/tests/core/consumer_group_command_test.py b/tests/kafkatest/tests/core/consumer_group_command_test.py index 3b714f21a4f40..b4f4df5b5b01d 100644 --- a/tests/kafkatest/tests/core/consumer_group_command_test.py +++ b/tests/kafkatest/tests/core/consumer_group_command_test.py @@ -63,7 +63,7 @@ def start_consumer(self): consumer_timeout_ms=None) self.consumer.start() - def setup_and_verify(self, security_protocol, tls_version=None, group=None): + def setup_and_verify(self, security_protocol, group=None, tls_version=None): self.start_kafka(security_protocol, security_protocol, tls_version) self.start_consumer() consumer_node = self.consumer.nodes[0] @@ -96,7 +96,7 @@ def test_list_consumer_groups(self, security_protocol='PLAINTEXT', tls_version=N Tests if ConsumerGroupCommand is listing correct consumer groups :return: None """ - self.setup_and_verify(security_protocol, tls_version) + self.setup_and_verify(security_protocol, tls_version=tls_version) @cluster(num_nodes=3) @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) @@ -106,4 +106,4 @@ def test_describe_consumer_group(self, security_protocol='PLAINTEXT', tls_versio Tests if ConsumerGroupCommand is describing a consumer group correctly :return: None """ - self.setup_and_verify(security_protocol, tls_version, group="test-consumer-group") + self.setup_and_verify(security_protocol, tls_version=tls_version, group="test-consumer-group") diff --git a/tests/kafkatest/tests/tools/log4j_appender_test.py b/tests/kafkatest/tests/tools/log4j_appender_test.py index af3fd87da8a0f..3a7dbb008ed53 100644 --- a/tests/kafkatest/tests/tools/log4j_appender_test.py +++ b/tests/kafkatest/tests/tools/log4j_appender_test.py @@ -23,7 +23,6 @@ from kafkatest.services.kafka import KafkaService from kafkatest.services.console_consumer import ConsoleConsumer from kafkatest.services.kafka_log4j_appender import KafkaLog4jAppender -from kafkatest.services.security.security_config import SecurityConfig TOPIC = "topic-log4j-appender" MAX_MESSAGES = 100 From c7000d9e7372824762db02cf4a5dd6688726ba32 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Wed, 27 May 2020 12:41:30 +0300 Subject: [PATCH 11/31] KAFKA-9320: system tests updated. --- tests/kafkatest/services/kafka/kafka.py | 2 +- tests/kafkatest/services/kafka/util.py | 27 ++++--------------- .../services/kafka_log4j_appender.py | 3 +++ .../services/security/security_config.py | 11 +++++--- tests/kafkatest/tests/core/upgrade_test.py | 3 ++- .../tests/tools/log4j_appender_test.py | 2 +- tests/kafkatest/utils/remote_account.py | 19 +++++++++++++ 7 files changed, 38 insertions(+), 29 deletions(-) diff --git a/tests/kafkatest/services/kafka/kafka.py b/tests/kafkatest/services/kafka/kafka.py index 9a4225e16bff6..cafe620f5912b 100644 --- a/tests/kafkatest/services/kafka/kafka.py +++ b/tests/kafkatest/services/kafka/kafka.py @@ -357,7 +357,7 @@ def start_cmd(self, node): KafkaService.STDOUT_STDERR_CAPTURE) return cmd - def start_node(self, node, timeout_sec=60): + def start_node(self, node, timeout_sec=180): node.account.mkdirs(KafkaService.PERSISTENT_ROOT) self.security_config.setup_node(node) diff --git a/tests/kafkatest/services/kafka/util.py b/tests/kafkatest/services/kafka/util.py index 92d59c74d1634..8782ebe7b4221 100644 --- a/tests/kafkatest/services/kafka/util.py +++ b/tests/kafkatest/services/kafka/util.py @@ -13,16 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os.path - from collections import namedtuple -from kafkatest.version import DEV_BRANCH, LATEST_0_8_2, LATEST_0_9, LATEST_0_10_0, LATEST_0_10_1, LATEST_0_10_2, LATEST_0_11_0, LATEST_1_0 -from kafkatest.directory_layout.kafka_path import create_path_resolver + +from kafkatest.utils.remote_account import java_version +from kafkatest.version import LATEST_0_8_2, LATEST_0_9, LATEST_0_10_0, LATEST_0_10_1, LATEST_0_10_2, LATEST_0_11_0, LATEST_1_0 TopicPartition = namedtuple('TopicPartition', ['topic', 'partition']) new_jdk_not_supported = frozenset([str(LATEST_0_8_2), str(LATEST_0_9), str(LATEST_0_10_0), str(LATEST_0_10_1), str(LATEST_0_10_2), str(LATEST_0_11_0), str(LATEST_1_0)]) + def fix_opts_for_new_jvm(node): # Startup scripts for early versions of Kafka contains options # that not supported on latest versions of JVM like -XX:+PrintGCDateStamps or -XX:UseParNewGC. @@ -38,21 +38,4 @@ def fix_opts_for_new_jvm(node): cmd += "export KAFKA_JVM_PERFORMANCE_OPTS=\"-server -XX:+UseG1GC -XX:MaxGCPauseMillis=20 -XX:InitiatingHeapOccupancyPercent=35 -XX:+ExplicitGCInvokesConcurrent -XX:MaxInlineLevel=15 -Djava.awt.headless=true\"; " return cmd -def java_version(node): - # Determine java version on the node - version = -1 - for line in node.account.ssh_capture("java -version"): - if line.find("version") != -1: - version = parse_version_str(line) - return version - -def parse_version_str(line): - # Parse java version string. Examples: - #`openjdk version "11.0.5" 2019-10-15` will return 11. - #`java version "1.5.0"` will return 5. - line = line[line.find('version \"') + 9:] - dot_pos = line.find(".") - if line[:dot_pos] == "1": - return int(line[dot_pos+1:line.find(".", dot_pos+1)]) - else: - return int(line[:dot_pos]) + diff --git a/tests/kafkatest/services/kafka_log4j_appender.py b/tests/kafkatest/services/kafka_log4j_appender.py index d47914e738de5..1212a7d5454da 100644 --- a/tests/kafkatest/services/kafka_log4j_appender.py +++ b/tests/kafkatest/services/kafka_log4j_appender.py @@ -38,6 +38,9 @@ def __init__(self, context, num_nodes, kafka, topic, max_messages=-1, security_p self.security_config = SecurityConfig(self.context, security_protocol, tls_version=tls_version) self.stop_timeout_sec = 30 + for node in self.nodes: + node.version = kafka.nodes[0].version + def _worker(self, idx, node): cmd = self.start_cmd(node) self.logger.debug("VerifiableLog4jAppender %d command: %s" % (idx, cmd)) diff --git a/tests/kafkatest/services/security/security_config.py b/tests/kafkatest/services/security/security_config.py index cff9884d94d6d..d18a265d5c156 100644 --- a/tests/kafkatest/services/security/security_config.py +++ b/tests/kafkatest/services/security/security_config.py @@ -20,11 +20,12 @@ from shutil import rmtree from ducktape.template import TemplateRenderer -from kafkatest.services.kafka.util import java_version from kafkatest.services.security.minikdc import MiniKdc from kafkatest.services.security.listener_security_config import ListenerSecurityConfig import itertools +from kafkatest.utils.remote_account import java_version + class SslStores(object): def __init__(self, local_scratch_dir, logger=None): @@ -178,7 +179,6 @@ def __init__(self, context, security_protocol=None, interbroker_security_protoco self.listener_security_config = listener_security_config self.properties = { 'security.protocol' : security_protocol, - 'tls.version' : tls_version, 'ssl.keystore.location' : SecurityConfig.KEYSTORE_PATH, 'ssl.keystore.password' : SecurityConfig.ssl_stores.keystore_passwd, 'ssl.key.password' : SecurityConfig.ssl_stores.key_passwd, @@ -189,6 +189,10 @@ def __init__(self, context, security_protocol=None, interbroker_security_protoco 'sasl.mechanism.inter.broker.protocol' : interbroker_sasl_mechanism, 'sasl.kerberos.service.name' : 'kafka' } + + if tls_version is not None: + self.properties.update({'tls.version' : tls_version}) + self.properties.update(self.listener_security_config.client_listener_overrides) self.jaas_override_variables = jaas_override_variables or {} @@ -212,7 +216,6 @@ def enable_security_protocol(self, security_protocol): self.has_ssl = self.has_ssl or self.is_ssl(security_protocol) def setup_ssl(self, node): - node.account.ssh("mkdir -p %s" % SecurityConfig.CONFIG_DIR, allow_fail=False) node.account.copy_to(SecurityConfig.ssl_stores.truststore_path, SecurityConfig.TRUSTSTORE_PATH) SecurityConfig.ssl_stores.generate_and_copy_keystore(node) @@ -313,7 +316,7 @@ def security_protocol(self): @property def tls_version(self): - return self.properties['tls.version'] + return self.properties.get('tls.version') @property def client_sasl_mechanism(self): diff --git a/tests/kafkatest/tests/core/upgrade_test.py b/tests/kafkatest/tests/core/upgrade_test.py index f5ec08c67f9c2..6c0e447a8b37d 100644 --- a/tests/kafkatest/tests/core/upgrade_test.py +++ b/tests/kafkatest/tests/core/upgrade_test.py @@ -23,8 +23,9 @@ from kafkatest.services.zookeeper import ZookeeperService from kafkatest.tests.produce_consume_validate import ProduceConsumeValidateTest from kafkatest.utils import is_int +from kafkatest.utils.remote_account import java_version from kafkatest.version import LATEST_0_8_2, LATEST_0_9, LATEST_0_10, LATEST_0_10_0, LATEST_0_10_1, LATEST_0_10_2, LATEST_0_11_0, LATEST_1_0, LATEST_1_1, LATEST_2_0, LATEST_2_1, LATEST_2_2, LATEST_2_3, LATEST_2_4, V_0_9_0_0, V_0_11_0_0, DEV_BRANCH, KafkaVersion -from kafkatest.services.kafka.util import java_version, new_jdk_not_supported +from kafkatest.services.kafka.util import new_jdk_not_supported class TestUpgrade(ProduceConsumeValidateTest): diff --git a/tests/kafkatest/tests/tools/log4j_appender_test.py b/tests/kafkatest/tests/tools/log4j_appender_test.py index 3a7dbb008ed53..4bb2bae96ece8 100644 --- a/tests/kafkatest/tests/tools/log4j_appender_test.py +++ b/tests/kafkatest/tests/tools/log4j_appender_test.py @@ -87,7 +87,7 @@ def test_log4j_appender(self, security_protocol='PLAINTEXT', tls_version=None): node = self.consumer.nodes[0] wait_until(lambda: self.consumer.alive(node), - timeout_sec=20, backoff_sec=.2, err_msg="Consumer was too slow to start") + timeout_sec=120, backoff_sec=.2, err_msg="Consumer was too slow to start") # Verify consumed messages count wait_until(lambda: self.messages_received_count == MAX_MESSAGES, timeout_sec=10, diff --git a/tests/kafkatest/utils/remote_account.py b/tests/kafkatest/utils/remote_account.py index d6ea72f664611..b572e06fe3039 100644 --- a/tests/kafkatest/utils/remote_account.py +++ b/tests/kafkatest/utils/remote_account.py @@ -37,3 +37,22 @@ def line_count(node, file): raise Exception("Expected single line of output from wc -l") return int(out[0].strip().split(" ")[0]) + +def java_version(node): + # Determine java version on the node + version = -1 + for line in node.account.ssh_capture("java -version"): + if line.find("version") != -1: + version = parse_version_str(line) + return version + +def parse_version_str(line): + # Parse java version string. Examples: + #`openjdk version "11.0.5" 2019-10-15` will return 11. + #`java version "1.5.0"` will return 5. + line = line[line.find('version \"') + 9:] + dot_pos = line.find(".") + if line[:dot_pos] == "1": + return int(line[dot_pos+1:line.find(".", dot_pos+1)]) + else: + return int(line[:dot_pos]) From 862f7ae4eceeb43135409da0762c3519260739a4 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Thu, 28 May 2020 11:48:24 +0300 Subject: [PATCH 12/31] KAFKA-9320: code review fixes --- .../common/network/SslTransportLayerTest.java | 16 +++++++++------- .../network/SslVersionsTransportLayerTest.java | 4 +--- tests/kafkatest/services/kafka/kafka.py | 3 +-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index b019e8b9ebe7c..750b82afa9ab6 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -64,6 +64,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; /** * Tests for the SSL transport layer. These use a test harness that runs a simple socket server that echos back responses. @@ -591,7 +592,10 @@ public void testUnsupportedCipher() throws Exception { createSelector(sslClientConfigs); checkAuthentiationFailed("1", "TLSv1.1"); + server.verifyAuthenticationMetrics(0, 1); + checkAuthentiationFailed("2", "TLSv1"); + server.verifyAuthenticationMetrics(0, 2); } } @@ -624,8 +628,7 @@ public void testUnsupportedTLSVersion() throws Exception { */ @Test public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { - if (!Java.IS_JAVA11_COMPATIBLE) - return; + assumeTrue(Java.IS_JAVA11_COMPATIBLE); SSLContext context = SSLContext.getInstance(tlsProtocol); context.init(null, null, null); @@ -650,8 +653,7 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { */ @Test public void testCiphersSuiteFailForServerTLSv1_2_ClientTLSv1_3() throws Exception { - if (!Java.IS_JAVA11_COMPATIBLE) - return; + assumeTrue(Java.IS_JAVA11_COMPATIBLE); String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; @@ -1322,7 +1324,7 @@ private interface FailureAction { void run() throws IOException; } - public static class TestSslChannelBuilder extends SslChannelBuilder { + static class TestSslChannelBuilder extends SslChannelBuilder { private Integer netReadBufSizeOverride; private Integer netWriteBufSizeOverride; @@ -1365,7 +1367,7 @@ protected TestSslTransportLayer newTransportLayer(String id, SelectionKey key, S *
  • Delayed writes to test handshake failure notifications to peer
    • * */ - public class TestSslTransportLayer extends SslTransportLayer { + class TestSslTransportLayer extends SslTransportLayer { private final ResizeableBufferSize netReadBufSize; private final ResizeableBufferSize netWriteBufSize; @@ -1433,7 +1435,7 @@ private void resetDelayedFlush() { } } - public static class ResizeableBufferSize { + static class ResizeableBufferSize { private Integer bufSizeOverride; ResizeableBufferSize(Integer bufSizeOverride) { this.bufSizeOverride = bufSizeOverride; diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index 4481405b96cf2..c073a2e552773 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -69,7 +69,7 @@ public SslVersionsTransportLayerTest(String tlsServerProtocol, String tlsClientP * Tests that connection success with the default TLS version. */ @Test - public void testTLSDefaults() throws Exception { + public void testTlsDefaults() throws Exception { // Create certificates for use by client and server. Add server cert to client truststore and vice versa. CertStores serverCertStores = new CertStores(true, "server", "localhost"); CertStores clientCertStores = new CertStores(false, "client", "localhost"); @@ -108,10 +108,8 @@ public void testTLSDefaults() throws Exception { public static Map sslConfig(String tlsServerProtocol) { Map sslConfig = new HashMap<>(); - sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsServerProtocol); sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList(tlsServerProtocol)); - return sslConfig; } diff --git a/tests/kafkatest/services/kafka/kafka.py b/tests/kafkatest/services/kafka/kafka.py index cafe620f5912b..58bd47a7bf839 100644 --- a/tests/kafkatest/services/kafka/kafka.py +++ b/tests/kafkatest/services/kafka/kafka.py @@ -51,7 +51,6 @@ def advertised_listener(self, node): def listener_security_protocol(self): return "%s:%s" % (self.name, self.security_protocol) - class KafkaService(KafkaPathResolverMixin, JmxMixin, Service): PERSISTENT_ROOT = "/mnt/kafka" STDOUT_STDERR_CAPTURE = os.path.join(PERSISTENT_ROOT, "server-start-stdout-stderr.log") @@ -357,7 +356,7 @@ def start_cmd(self, node): KafkaService.STDOUT_STDERR_CAPTURE) return cmd - def start_node(self, node, timeout_sec=180): + def start_node(self, node, timeout_sec=60): node.account.mkdirs(KafkaService.PERSISTENT_ROOT) self.security_config.setup_node(node) From 5578192735c999fc070b1c9559c7a3d3f3cbe2df Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Thu, 28 May 2020 12:09:28 +0300 Subject: [PATCH 13/31] KAFKA-9320: code review fixes --- tests/kafkatest/services/console_consumer.py | 1 + tests/kafkatest/services/zookeeper.py | 1 + .../tests/core/consumer_group_command_test.py | 23 ++++++++----------- .../kafkatest/tests/core/mirror_maker_test.py | 18 ++++++--------- .../tests/tools/log4j_appender_test.py | 22 +++++++++--------- 5 files changed, 30 insertions(+), 35 deletions(-) diff --git a/tests/kafkatest/services/console_consumer.py b/tests/kafkatest/services/console_consumer.py index 14b450c0871e2..47c2456ebd1cf 100644 --- a/tests/kafkatest/services/console_consumer.py +++ b/tests/kafkatest/services/console_consumer.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import itertools import os from ducktape.cluster.remoteaccount import RemoteCommandError diff --git a/tests/kafkatest/services/zookeeper.py b/tests/kafkatest/services/zookeeper.py index 51f2b31de2dcf..b8cb9b69cbd82 100644 --- a/tests/kafkatest/services/zookeeper.py +++ b/tests/kafkatest/services/zookeeper.py @@ -16,6 +16,7 @@ import os import re +import time from ducktape.services.service import Service from ducktape.utils.util import wait_until diff --git a/tests/kafkatest/tests/core/consumer_group_command_test.py b/tests/kafkatest/tests/core/consumer_group_command_test.py index b4f4df5b5b01d..871e2761ade25 100644 --- a/tests/kafkatest/tests/core/consumer_group_command_test.py +++ b/tests/kafkatest/tests/core/consumer_group_command_test.py @@ -16,7 +16,7 @@ from ducktape.utils.util import wait_until from ducktape.tests.test import Test -from ducktape.mark import matrix, parametrize +from ducktape.mark import matrix from ducktape.mark.resource import cluster from kafkatest.services.zookeeper import ZookeeperService @@ -50,11 +50,10 @@ def __init__(self, test_context): def setUp(self): self.zk.start() - def start_kafka(self, security_protocol, interbroker_security_protocol, tls_version=None): + def start_kafka(self, security_protocol, interbroker_security_protocol): self.kafka = KafkaService( self.test_context, self.num_brokers, self.zk, security_protocol=security_protocol, - tls_version=tls_version, interbroker_security_protocol=interbroker_security_protocol, topics=self.topics) self.kafka.start() @@ -63,8 +62,8 @@ def start_consumer(self): consumer_timeout_ms=None) self.consumer.start() - def setup_and_verify(self, security_protocol, group=None, tls_version=None): - self.start_kafka(security_protocol, security_protocol, tls_version) + def setup_and_verify(self, security_protocol, group=None): + self.start_kafka(security_protocol, security_protocol) self.start_consumer() consumer_node = self.consumer.nodes[0] wait_until(lambda: self.consumer.alive(consumer_node), @@ -89,21 +88,19 @@ def setup_and_verify(self, security_protocol, group=None, tls_version=None): self.consumer.stop() @cluster(num_nodes=3) - @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @parametrize(security_protocol='PLAINTEXT') - def test_list_consumer_groups(self, security_protocol='PLAINTEXT', tls_version=None): + @matrix(security_protocol=['PLAINTEXT', 'SSL']) + def test_list_consumer_groups(self, security_protocol='PLAINTEXT'): """ Tests if ConsumerGroupCommand is listing correct consumer groups :return: None """ - self.setup_and_verify(security_protocol, tls_version=tls_version) + self.setup_and_verify(security_protocol) @cluster(num_nodes=3) - @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @parametrize(security_protocol='PLAINTEXT') - def test_describe_consumer_group(self, security_protocol='PLAINTEXT', tls_version=None): + @matrix(security_protocol=['PLAINTEXT', 'SSL']) + def test_describe_consumer_group(self, security_protocol='PLAINTEXT'): """ Tests if ConsumerGroupCommand is describing a consumer group correctly :return: None """ - self.setup_and_verify(security_protocol, tls_version=tls_version, group="test-consumer-group") + self.setup_and_verify(security_protocol, group="test-consumer-group") diff --git a/tests/kafkatest/tests/core/mirror_maker_test.py b/tests/kafkatest/tests/core/mirror_maker_test.py index 8f4c4f5e41563..c33f103e50e9e 100644 --- a/tests/kafkatest/tests/core/mirror_maker_test.py +++ b/tests/kafkatest/tests/core/mirror_maker_test.py @@ -58,12 +58,10 @@ def setUp(self): # Target cluster self.target_zk.start() - def start_kafka(self, security_protocol, tls_version=None): + def start_kafka(self, security_protocol): self.source_kafka.security_protocol = security_protocol - self.source_kafka.tls_version = tls_version self.source_kafka.interbroker_security_protocol = security_protocol self.target_kafka.security_protocol = security_protocol - self.target_kafka.tls_version = tls_version self.target_kafka.interbroker_security_protocol = security_protocol if self.source_kafka.security_config.has_sasl_kerberos: minikdc = MiniKdc(self.source_kafka.context, self.source_kafka.nodes + self.target_kafka.nodes) @@ -113,11 +111,10 @@ def wait_for_n_messages(self, n_messages=100): err_msg="Producer failed to produce %d messages in a reasonable amount of time." % n_messages) @cluster(num_nodes=7) - @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @parametrize(security_protocol='PLAINTEXT') + @matrix(security_protocol=['PLAINTEXT', 'SSL']) @cluster(num_nodes=8) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_simple_end_to_end(self, security_protocol, tls_version=None): + def test_simple_end_to_end(self, security_protocol): """ Test end-to-end behavior under non-failure conditions. @@ -129,7 +126,7 @@ def test_simple_end_to_end(self, security_protocol, tls_version=None): - Consume messages from target. - Verify that number of consumed messages matches the number produced. """ - self.start_kafka(security_protocol, tls_version) + self.start_kafka(security_protocol) self.mirror_maker.start() mm_node = self.mirror_maker.nodes[0] @@ -139,11 +136,10 @@ def test_simple_end_to_end(self, security_protocol, tls_version=None): self.mirror_maker.stop() @cluster(num_nodes=7) - @matrix(clean_shutdown=[True, False], security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @matrix(clean_shutdown=[True, False], security_protocol='PLAINTEXT') + @matrix(clean_shutdown=[True, False], security_protocol=['PLAINTEXT', 'SSL']) @cluster(num_nodes=8) @matrix(clean_shutdown=[True, False], security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_protocol='PLAINTEXT', tls_version=None): + def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_protocol='PLAINTEXT'): """ Test end-to-end behavior under failure conditions. @@ -161,7 +157,7 @@ def test_bounce(self, offsets_storage="kafka", clean_shutdown=True, security_pro # the group until the previous session times out self.consumer.consumer_timeout_ms = 60000 - self.start_kafka(security_protocol, tls_version) + self.start_kafka(security_protocol) self.mirror_maker.offsets_storage = offsets_storage self.mirror_maker.start() diff --git a/tests/kafkatest/tests/tools/log4j_appender_test.py b/tests/kafkatest/tests/tools/log4j_appender_test.py index 4bb2bae96ece8..03879e15d488c 100644 --- a/tests/kafkatest/tests/tools/log4j_appender_test.py +++ b/tests/kafkatest/tests/tools/log4j_appender_test.py @@ -16,13 +16,14 @@ from ducktape.utils.util import wait_until from ducktape.tests.test import Test -from ducktape.mark import matrix, parametrize +from ducktape.mark import matrix from ducktape.mark.resource import cluster from kafkatest.services.zookeeper import ZookeeperService from kafkatest.services.kafka import KafkaService from kafkatest.services.console_consumer import ConsoleConsumer from kafkatest.services.kafka_log4j_appender import KafkaLog4jAppender +from kafkatest.services.security.security_config import SecurityConfig TOPIC = "topic-log4j-appender" MAX_MESSAGES = 100 @@ -46,16 +47,16 @@ def __init__(self, test_context): def setUp(self): self.zk.start() - def start_kafka(self, security_protocol, interbroker_security_protocol, tls_version=None): + def start_kafka(self, security_protocol, interbroker_security_protocol): self.kafka = KafkaService( self.test_context, self.num_brokers, - self.zk, security_protocol=security_protocol, tls_version=tls_version, + self.zk, security_protocol=security_protocol, interbroker_security_protocol=interbroker_security_protocol, topics=self.topics) self.kafka.start() - def start_appender(self, security_protocol, tls_version=None): + def start_appender(self, security_protocol): self.appender = KafkaLog4jAppender(self.test_context, self.num_brokers, self.kafka, TOPIC, MAX_MESSAGES, - security_protocol=security_protocol, tls_version=tls_version) + security_protocol=security_protocol) self.appender.start() def custom_message_validator(self, msg): @@ -70,24 +71,23 @@ def start_consumer(self): self.consumer.start() @cluster(num_nodes=4) - @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @parametrize(security_protocol='PLAINTEXT') + @matrix(security_protocol=['PLAINTEXT', 'SSL']) @cluster(num_nodes=5) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_log4j_appender(self, security_protocol='PLAINTEXT', tls_version=None): + def test_log4j_appender(self, security_protocol='PLAINTEXT'): """ Tests if KafkaLog4jAppender is producing to Kafka topic :return: None """ - self.start_kafka(security_protocol, security_protocol, tls_version) - self.start_appender(security_protocol, tls_version) + self.start_kafka(security_protocol, security_protocol) + self.start_appender(security_protocol) self.appender.wait() self.start_consumer() node = self.consumer.nodes[0] wait_until(lambda: self.consumer.alive(node), - timeout_sec=120, backoff_sec=.2, err_msg="Consumer was too slow to start") + timeout_sec=20, backoff_sec=.2, err_msg="Consumer was too slow to start") # Verify consumed messages count wait_until(lambda: self.messages_received_count == MAX_MESSAGES, timeout_sec=10, From c1847e7e30e61633e6616e5c2e8bb93d8cfb4eb6 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Thu, 28 May 2020 12:13:39 +0300 Subject: [PATCH 14/31] KAFKA-9320: code review fixes --- tests/kafkatest/sanity_checks/test_console_consumer.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/kafkatest/sanity_checks/test_console_consumer.py b/tests/kafkatest/sanity_checks/test_console_consumer.py index 152cabbb88fab..acf1184e0595d 100644 --- a/tests/kafkatest/sanity_checks/test_console_consumer.py +++ b/tests/kafkatest/sanity_checks/test_console_consumer.py @@ -15,7 +15,7 @@ import time -from ducktape.mark import matrix, defaults +from ducktape.mark import matrix from ducktape.mark import parametrize from ducktape.mark.resource import cluster from ducktape.tests.test import Test @@ -44,22 +44,19 @@ def setUp(self): self.zk.start() @cluster(num_nodes=3) - @matrix(security_protocol=['SSL'], tls_version=['TLSv1.2', 'TLSv1.3']) - @parametrize(security_protocol='PLAINTEXT') + @matrix(security_protocol=['PLAINTEXT', 'SSL']) @cluster(num_nodes=4) @matrix(security_protocol=['SASL_SSL'], sasl_mechanism=['PLAIN', 'SCRAM-SHA-256', 'SCRAM-SHA-512']) @matrix(security_protocol=['SASL_PLAINTEXT', 'SASL_SSL']) - def test_lifecycle(self, security_protocol, tls_version=None, sasl_mechanism='GSSAPI'): + def test_lifecycle(self, security_protocol, sasl_mechanism='GSSAPI'): """Check that console consumer starts/stops properly, and that we are capturing log output.""" self.kafka.security_protocol = security_protocol - self.kafka.tls_version = tls_version self.kafka.client_sasl_mechanism = sasl_mechanism self.kafka.interbroker_sasl_mechanism = sasl_mechanism self.kafka.start() self.consumer.security_protocol = security_protocol - self.consumer.tls_version = tls_version t0 = time.time() self.consumer.start() From c9012549a724719e641e2ab8c3883c7aaf4c4fa1 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Thu, 28 May 2020 12:14:27 +0300 Subject: [PATCH 15/31] KAFKA-9320: code review fixes --- tests/docker/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/docker/run_tests.sh b/tests/docker/run_tests.sh index 6278f9ac64582..063e24d178765 100755 --- a/tests/docker/run_tests.sh +++ b/tests/docker/run_tests.sh @@ -30,6 +30,6 @@ if [ "$REBUILD" == "t" ]; then fi if ${SCRIPT_DIR}/ducker-ak ssh | grep -q '(none)'; then - ${SCRIPT_DIR}/ducker-ak up -j 'openjdk:11' -n "${KAFKA_NUM_CONTAINERS}" || die "ducker-ak up failed" + ${SCRIPT_DIR}/ducker-ak up -n "${KAFKA_NUM_CONTAINERS}" || die "ducker-ak up failed" fi ${SCRIPT_DIR}/ducker-ak test ${TC_PATHS} ${_DUCKTAPE_OPTIONS} || die "ducker-ak test failed" From 61cd6c5e667a9ca1f1b7b94dd0590c4b8c1d0899 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Thu, 28 May 2020 14:48:27 +0300 Subject: [PATCH 16/31] KAFKA-9320: code review fixes --- .../SslVersionsTransportLayerTest.java | 49 ++++++++++--------- .../kafkatest/tests/core/replication_test.py | 7 ++- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index c073a2e552773..57dd6cbc9bfec 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -19,8 +19,8 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -45,24 +45,29 @@ public class SslVersionsTransportLayerTest { private static final int BUFFER_SIZE = 4 * 1024; private static final Time TIME = Time.SYSTEM; - private final String tlsServerProtocol; - private final String tlsClientProtocol; + private final List tlsServerProtocols; + private final List tlsClientProtocols; @Parameterized.Parameters(name = "tlsServerProtocol={0},tlsClientProtocol={1}") public static Collection data() { List values = new ArrayList<>(); - values.add(new Object[] {"TLSv1.2", "TLSv1.2"}); + values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.2")}); if (Java.IS_JAVA11_COMPATIBLE) { - values.add(new Object[] {"TLSv1.2", "TLSv1.3"}); - values.add(new Object[] {"TLSv1.3", "TLSv1.2"}); - values.add(new Object[] {"TLSv1.3", "TLSv1.3"}); + values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.2")}); + values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2")}); + values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); } return values; } - public SslVersionsTransportLayerTest(String tlsServerProtocol, String tlsClientProtocol) { - this.tlsServerProtocol = tlsServerProtocol; - this.tlsClientProtocol = tlsClientProtocol; + public SslVersionsTransportLayerTest(List tlsServerProtocols, List tlsClientProtocols) { + this.tlsServerProtocols = tlsServerProtocols; + this.tlsClientProtocols = tlsClientProtocols; } /** @@ -74,8 +79,8 @@ public void testTlsDefaults() throws Exception { CertStores serverCertStores = new CertStores(true, "server", "localhost"); CertStores clientCertStores = new CertStores(false, "client", "localhost"); - Map sslClientConfigs = getTrustingConfig(clientCertStores, serverCertStores, tlsClientProtocol); - Map sslServerConfigs = getTrustingConfig(serverCertStores, clientCertStores, tlsServerProtocol); + Map sslClientConfigs = getTrustingConfig(clientCertStores, serverCertStores, tlsClientProtocols); + Map sslServerConfigs = getTrustingConfig(serverCertStores, clientCertStores, tlsServerProtocols); NioEchoServer server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), SecurityProtocol.SSL, @@ -87,7 +92,7 @@ public void testTlsDefaults() throws Exception { String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); - if (tlsServerProtocol.equals(tlsClientProtocol)) { + if (tlsServerProtocols.contains(tlsClientProtocols.get(0))) { NetworkTestUtils.waitForChannelReady(selector, node); int msgSz = 1024 * 1024; @@ -106,19 +111,19 @@ public void testTlsDefaults() throws Exception { } } - public static Map sslConfig(String tlsServerProtocol) { - Map sslConfig = new HashMap<>(); - sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsServerProtocol); - sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList(tlsServerProtocol)); - return sslConfig; - } - - public static Map getTrustingConfig(CertStores certStores, CertStores peerCertStores, String tlsProtocol) { + private static Map getTrustingConfig(CertStores certStores, CertStores peerCertStores, List tlsProtocols) { Map configs = certStores.getTrustingConfig(peerCertStores); - configs.putAll(sslConfig(tlsProtocol)); + configs.putAll(sslConfig(tlsProtocols)); return configs; } + private static Map sslConfig(List tlsServerProtocols) { + Map sslConfig = new HashMap<>(); + sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsServerProtocols.get(0)); + sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, tlsServerProtocols); + return sslConfig; + } + private Selector createSelector(Map sslClientConfigs) { SslTransportLayerTest.TestSslChannelBuilder channelBuilder = new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); channelBuilder.configureBufferSizes(null, null, null); diff --git a/tests/kafkatest/tests/core/replication_test.py b/tests/kafkatest/tests/core/replication_test.py index f5c64222bdb65..603e99a9b1631 100644 --- a/tests/kafkatest/tests/core/replication_test.py +++ b/tests/kafkatest/tests/core/replication_test.py @@ -126,9 +126,11 @@ def min_cluster_size(self): security_protocol="SASL_SSL", client_sasl_mechanism="SCRAM-SHA-256", interbroker_sasl_mechanism="SCRAM-SHA-512") @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"], security_protocol=["PLAINTEXT"], broker_type=["leader"], compression_type=["gzip"]) + @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"], + security_protocol=["SSL"], broker_type=["leader"], compression_type=["gzip"], tls_version=["TLSv1.2", "TLSv1.3"]) def test_replication_with_broker_failure(self, failure_mode, security_protocol, broker_type, client_sasl_mechanism="GSSAPI", interbroker_sasl_mechanism="GSSAPI", - compression_type=None, enable_idempotence=False): + compression_type=None, enable_idempotence=False, tls_version=None): """Replication tests. These tests verify that replication provides simple durability guarantees by checking that data acked by brokers is still available for consumption in the face of various failure scenarios. @@ -149,7 +151,8 @@ def test_replication_with_broker_failure(self, failure_mode, security_protocol, security_protocol=security_protocol, interbroker_security_protocol=security_protocol, client_sasl_mechanism=client_sasl_mechanism, - interbroker_sasl_mechanism=interbroker_sasl_mechanism) + interbroker_sasl_mechanism=interbroker_sasl_mechanism, + tls_version=tls_version) self.kafka.start() compression_types = None if not compression_type else [compression_type] From fd1f48b04519a65cdb91f5a28b6a2bd6886adc61 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Fri, 29 May 2020 11:47:11 +0300 Subject: [PATCH 17/31] KAFKA-9320: code review fixes --- .../common/network/SslTransportLayerTest.java | 62 ++++++++++++++----- .../SslVersionsTransportLayerTest.java | 17 ++--- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 750b82afa9ab6..fe7b3a3720908 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -627,7 +627,7 @@ public void testUnsupportedTLSVersion() throws Exception { * Tests that connections fails if TLSv1.3 enabled but cipher suite suitable only for TLSv1.2 used. */ @Test - public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { + public void testCiphersSuiteForTls12_FailsForTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); SSLContext context = SSLContext.getInstance(tlsProtocol); @@ -637,57 +637,85 @@ public void testCiphersSuiteForTLSv1_2_FailsForTLSv1_3() throws Exception { String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); server = createEchoServer(SecurityProtocol.SSL); - sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); checkAuthentiationFailed("0", "TLSv1.3"); server.verifyAuthenticationMetrics(0, 1); } /** - * Tests that connections can be made with TLSv1.2 and custom cipher suite. + * Tests that connections can't be made if server uses TLSv1.2 with custom cipher suite and client uses TLSv1.3. */ @Test - public void testCiphersSuiteFailForServerTLSv1_2_ClientTLSv1_3() throws Exception { + public void testCiphersSuiteFailForServerTls12_ClientTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.2")); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.2")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); server = createEchoServer(SecurityProtocol.SSL); sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); checkAuthentiationFailed("0", "TLSv1.3"); } /** - * Tests that connections can be made with TLSv1.2 and custom cipher suite. + * Tests that connections can be made with TLSv1.3 cipher suite. */ @Test - public void testCiphersSuiteForTLSv1_2() throws Exception { + public void testCiphersSuiteForTls13() throws Exception { + assumeTrue(Java.IS_JAVA11_COMPATIBLE); + + String node = "0"; + SSLContext context = SSLContext.getInstance(tlsProtocol); + context.init(null, null, null); + + String cipherSuite = "TLS_AES_128_GCM_SHA256"; + + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + server = createEchoServer(SecurityProtocol.SSL); + + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + createSelector(sslClientConfigs); + InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); + selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); + + NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.READY); + server.verifyAuthenticationMetrics(1, 0); + } + + /** + * Tests that connections can be made with TLSv1.2 cipher suite. + */ + @Test + public void testCiphersSuiteForTls12() throws Exception { String node = "0"; SSLContext context = SSLContext.getInstance(tlsProtocol); context.init(null, null, null); - //Note, that only some ciphers works out of the box. Others requires additional configuration. String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; - sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); server = createEchoServer(SecurityProtocol.SSL); - sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuite)); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); createSelector(sslClientConfigs); InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index 57dd6cbc9bfec..bfc71c2fabc72 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -51,15 +52,15 @@ public class SslVersionsTransportLayerTest { @Parameterized.Parameters(name = "tlsServerProtocol={0},tlsClientProtocol={1}") public static Collection data() { List values = new ArrayList<>(); - values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.2")}); if (Java.IS_JAVA11_COMPATIBLE) { - values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.2")}); - values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2")}); - values.add(new Object[] {Arrays.asList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); } return values; From 4e7eaec600e094062cdd312fadb1177119aabdfa Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Fri, 29 May 2020 16:56:42 +0300 Subject: [PATCH 18/31] KAFKA-9320: test fix. --- .../kafka/common/network/SslTransportLayerTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index fe7b3a3720908..bf067ceb57dca 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -581,7 +581,16 @@ public void testTLSDefaults() throws Exception { @Test public void testUnsupportedCipher() throws Exception { - String[] cipherSuites = ((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites(); + String[] cipherSuites; + if (Java.IS_JAVA11_COMPATIBLE) { + cipherSuites = new String[] { + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + ((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites()[1] + }; + } else { + cipherSuites = ((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites(); + } + if (cipherSuites != null && cipherSuites.length > 1) { sslServerConfigs = serverCertStores.getTrustingConfig(clientCertStores); sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[0])); @@ -633,7 +642,6 @@ public void testCiphersSuiteForTls12_FailsForTls13() throws Exception { SSLContext context = SSLContext.getInstance(tlsProtocol); context.init(null, null, null); - //Note, that only some ciphers works out of the box. Others requires additional configuration. String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); From c756720b15c35e22f09a1ee7613dae6e2a190a29 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Fri, 29 May 2020 20:33:53 +0300 Subject: [PATCH 19/31] KAFKA-9320: code review fixes. --- .../kafka/common/network/SslTransportLayerTest.java | 8 +++++--- .../common/network/SslVersionsTransportLayerTest.java | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index bf067ceb57dca..72cc44bb69f21 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -663,17 +663,19 @@ public void testCiphersSuiteForTls12_FailsForTls13() throws Exception { public void testCiphersSuiteFailForServerTls12_ClientTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); - String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + String tls12CipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + String tls13CipherSuite = "TLS_AES_128_GCM_SHA256"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.2")); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls12CipherSuite)); server = createEchoServer(SecurityProtocol.SSL); sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls13CipherSuite)); checkAuthentiationFailed("0", "TLSv1.3"); + server.verifyAuthenticationMetrics(0, 1); } /** diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index bfc71c2fabc72..5240b4b4edf83 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -93,7 +93,7 @@ public void testTlsDefaults() throws Exception { String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); - if (tlsServerProtocols.contains(tlsClientProtocols.get(0))) { + if (!Collections.disjoint(tlsServerProtocols, tlsClientProtocols)) { NetworkTestUtils.waitForChannelReady(selector, node); int msgSz = 1024 * 1024; From a231e2f3a556be7b0cbca51b46d290e3b70ef47e Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Fri, 29 May 2020 20:51:28 +0300 Subject: [PATCH 20/31] KAFKA-9320: code review fixes. --- .../org/apache/kafka/common/config/SslConfigs.java | 2 +- .../common/network/SslVersionsTransportLayerTest.java | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java index 816b2079ee857..0d3c41ba7655e 100644 --- a/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java +++ b/clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java @@ -50,7 +50,7 @@ public class SslConfigs { public static final String SSL_PROTOCOL_CONFIG = "ssl.protocol"; public static final String SSL_PROTOCOL_DOC = "The SSL protocol used to generate the SSLContext. " - + "Default setting is TLSv1.2, which is fine for most cases. " + + "Default setting is TLSv1.2(TLSv1.3 for modern JVM), which is fine for most cases. " + "Allowed values in recent JVMs are TLSv1.2 and TLSv1.3. TLS, TLSv1.1, SSL, SSLv2 and SSLv3 " + "may be supported in older JVMs, but their usage is discouraged due to known security vulnerabilities."; diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index 5240b4b4edf83..f61199be3a7f1 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -57,11 +57,18 @@ public static Collection data() { values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.3")}); values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); - values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); - values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); } return values; } From 7ab2f390cfda9601f69dc0000d3ec8ccba1ec2fb Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 12:54:31 +0300 Subject: [PATCH 21/31] KAFKA-9320: code review fixes. --- .../apache/kafka/common/network/SslTransportLayerTest.java | 4 ++-- .../kafka/common/network/SslVersionsTransportLayerTest.java | 2 +- tests/kafkatest/services/security/security_config.py | 2 +- tests/kafkatest/tests/core/replication_test.py | 4 +--- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 72cc44bb69f21..c7c2a8f7b3c33 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -636,7 +636,7 @@ public void testUnsupportedTLSVersion() throws Exception { * Tests that connections fails if TLSv1.3 enabled but cipher suite suitable only for TLSv1.2 used. */ @Test - public void testCiphersSuiteForTls12_FailsForTls13() throws Exception { + public void testCiphersSuiteForTls12FailsForTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); SSLContext context = SSLContext.getInstance(tlsProtocol); @@ -660,7 +660,7 @@ public void testCiphersSuiteForTls12_FailsForTls13() throws Exception { * Tests that connections can't be made if server uses TLSv1.2 with custom cipher suite and client uses TLSv1.3. */ @Test - public void testCiphersSuiteFailForServerTls12_ClientTls13() throws Exception { + public void testCiphersSuiteFailForServerTls12ClientTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); String tls12CipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index f61199be3a7f1..6d4c652bd5f6e 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -100,7 +100,7 @@ public void testTlsDefaults() throws Exception { String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); - if (!Collections.disjoint(tlsServerProtocols, tlsClientProtocols)) { + if (tlsServerProtocols.contains(tlsClientProtocols.get(0))) { NetworkTestUtils.waitForChannelReady(selector, node); int msgSz = 1024 * 1024; diff --git a/tests/kafkatest/services/security/security_config.py b/tests/kafkatest/services/security/security_config.py index d18a265d5c156..2fb4f47bf0224 100644 --- a/tests/kafkatest/services/security/security_config.py +++ b/tests/kafkatest/services/security/security_config.py @@ -267,7 +267,7 @@ def setup_node(self, node): if self.has_sasl: self.setup_sasl(node) - if java_version(node) <= 9 and self.properties['tls.version'] == 'TLSv1.3': + if java_version(node) <= 11 and self.properties.get('tls.version') == 'TLSv1.3': self.properties.update({'tls.version': 'TLSv1.2'}) def setup_credentials(self, node, path, zk_connect, broker): diff --git a/tests/kafkatest/tests/core/replication_test.py b/tests/kafkatest/tests/core/replication_test.py index 603e99a9b1631..01ef34f318390 100644 --- a/tests/kafkatest/tests/core/replication_test.py +++ b/tests/kafkatest/tests/core/replication_test.py @@ -125,9 +125,7 @@ def min_cluster_size(self): broker_type="leader", security_protocol="SASL_SSL", client_sasl_mechanism="SCRAM-SHA-256", interbroker_sasl_mechanism="SCRAM-SHA-512") @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"], - security_protocol=["PLAINTEXT"], broker_type=["leader"], compression_type=["gzip"]) - @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"], - security_protocol=["SSL"], broker_type=["leader"], compression_type=["gzip"], tls_version=["TLSv1.2", "TLSv1.3"]) + security_protocol=["PLAINTEXT"], broker_type=["leader"], compression_type=["gzip"], tls_version=["TLSv1.2", "TLSv1.3"]) def test_replication_with_broker_failure(self, failure_mode, security_protocol, broker_type, client_sasl_mechanism="GSSAPI", interbroker_sasl_mechanism="GSSAPI", compression_type=None, enable_idempotence=False, tls_version=None): From 17612acc5acd40f33f133df18c679ad4fb07d8f4 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 16:24:53 +0300 Subject: [PATCH 22/31] KAFKA-9320: code review fixes. --- .../network/SslVersionsTransportLayerTest.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index 6d4c652bd5f6e..e2762f5444879 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -54,21 +54,16 @@ public static Collection data() { List values = new ArrayList<>(); values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.2")}); if (Java.IS_JAVA11_COMPATIBLE) { - values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.3")}); - values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.2")}); - values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.3")}); - values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); - values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); - values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); - values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); - values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + } return values; } @@ -100,7 +95,7 @@ public void testTlsDefaults() throws Exception { String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); - if (tlsServerProtocols.contains(tlsClientProtocols.get(0))) { + if (!Collections.disjoint(tlsServerProtocols, tlsClientProtocols)) { NetworkTestUtils.waitForChannelReady(selector, node); int msgSz = 1024 * 1024; From ebb20e1b1bf8d067f61e615eadcf5b760db56917 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 17:20:16 +0300 Subject: [PATCH 23/31] KAFKA-9320: revert test changes. --- .../network/SslVersionsTransportLayerTest.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index e2762f5444879..844ab01bf4452 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -54,15 +54,21 @@ public static Collection data() { List values = new ArrayList<>(); values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.2")}); if (Java.IS_JAVA11_COMPATIBLE) { - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); - values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); - + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Collections.singletonList("TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.2", "TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.3", "TLSv1.2"), Arrays.asList("TLSv1.3", "TLSv1.2")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); + values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); } return values; From 1b5558745b59da61594b2b728d99593bc6c620f3 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 17:35:09 +0300 Subject: [PATCH 24/31] KAFKA-9320: fix test duration. --- .../apache/kafka/common/network/SslTransportLayerTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index c7c2a8f7b3c33..6166f6abca959 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -702,8 +702,7 @@ public void testCiphersSuiteForTls13() throws Exception { createSelector(sslClientConfigs); InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - - NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.READY); + NetworkTestUtils.waitForChannelReady(selector, node); server.verifyAuthenticationMetrics(1, 0); } @@ -730,7 +729,7 @@ public void testCiphersSuiteForTls12() throws Exception { InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.READY); + NetworkTestUtils.waitForChannelReady(selector, node); server.verifyAuthenticationMetrics(1, 0); } From 9da1c210fcd6db5bc9f8c37c3fd13c1be88ae6f7 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 18:12:05 +0300 Subject: [PATCH 25/31] KAFKA-9320: unused code removed. --- .../common/network/SslTransportLayerTest.java | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 6166f6abca959..82262a2597adc 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -41,7 +41,6 @@ import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; -import javax.net.ssl.SSLServerSocketFactory; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.InetAddress; @@ -583,29 +582,24 @@ public void testTLSDefaults() throws Exception { public void testUnsupportedCipher() throws Exception { String[] cipherSuites; if (Java.IS_JAVA11_COMPATIBLE) { - cipherSuites = new String[] { - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - ((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites()[1] - }; + cipherSuites = new String[] {"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", "TLS_AES_256_GCM_SHA384"}; } else { - cipherSuites = ((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites(); + cipherSuites = new String[] {"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}; } - if (cipherSuites != null && cipherSuites.length > 1) { - sslServerConfigs = serverCertStores.getTrustingConfig(clientCertStores); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[0])); - sslClientConfigs = clientCertStores.getTrustingConfig(serverCertStores); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[1])); + sslServerConfigs = serverCertStores.getTrustingConfig(clientCertStores); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[0])); + sslClientConfigs = clientCertStores.getTrustingConfig(serverCertStores); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[1])); - server = createEchoServer(SecurityProtocol.SSL); - createSelector(sslClientConfigs); + server = createEchoServer(SecurityProtocol.SSL); + createSelector(sslClientConfigs); - checkAuthentiationFailed("1", "TLSv1.1"); - server.verifyAuthenticationMetrics(0, 1); + checkAuthentiationFailed("1", "TLSv1.1"); + server.verifyAuthenticationMetrics(0, 1); - checkAuthentiationFailed("2", "TLSv1"); - server.verifyAuthenticationMetrics(0, 2); - } + checkAuthentiationFailed("2", "TLSv1"); + server.verifyAuthenticationMetrics(0, 2); } /** Checks connection failed using the specified {@code tlsVersion}. */ @@ -639,9 +633,6 @@ public void testUnsupportedTLSVersion() throws Exception { public void testCiphersSuiteForTls12FailsForTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); - SSLContext context = SSLContext.getInstance(tlsProtocol); - context.init(null, null, null); - String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); @@ -686,9 +677,6 @@ public void testCiphersSuiteForTls13() throws Exception { assumeTrue(Java.IS_JAVA11_COMPATIBLE); String node = "0"; - SSLContext context = SSLContext.getInstance(tlsProtocol); - context.init(null, null, null); - String cipherSuite = "TLS_AES_128_GCM_SHA256"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); @@ -712,9 +700,6 @@ public void testCiphersSuiteForTls13() throws Exception { @Test public void testCiphersSuiteForTls12() throws Exception { String node = "0"; - SSLContext context = SSLContext.getInstance(tlsProtocol); - context.init(null, null, null); - String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); From 3e6c445cf9ef9f7d229c90bf3c6debdf19aa96bd Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Mon, 1 Jun 2020 19:34:41 +0300 Subject: [PATCH 26/31] KAFKA-9320: code review fixes. --- .../common/network/SslTransportLayerTest.java | 123 +------------ .../network/SslTransportTls12Tls13Test.java | 169 ++++++++++++++++++ 2 files changed, 170 insertions(+), 122 deletions(-) create mode 100644 clients/src/test/java/org/apache/kafka/common/network/SslTransportTls12Tls13Test.java diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index 82262a2597adc..feeebe153beeb 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -63,7 +63,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeTrue; /** * Tests for the SSL transport layer. These use a test harness that runs a simple socket server that echos back responses. @@ -578,30 +577,6 @@ public void testTLSDefaults() throws Exception { server.verifyAuthenticationMetrics(1, 2); } - @Test - public void testUnsupportedCipher() throws Exception { - String[] cipherSuites; - if (Java.IS_JAVA11_COMPATIBLE) { - cipherSuites = new String[] {"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", "TLS_AES_256_GCM_SHA384"}; - } else { - cipherSuites = new String[] {"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}; - } - - sslServerConfigs = serverCertStores.getTrustingConfig(clientCertStores); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[0])); - sslClientConfigs = clientCertStores.getTrustingConfig(serverCertStores); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuites[1])); - - server = createEchoServer(SecurityProtocol.SSL); - createSelector(sslClientConfigs); - - checkAuthentiationFailed("1", "TLSv1.1"); - server.verifyAuthenticationMetrics(0, 1); - - checkAuthentiationFailed("2", "TLSv1"); - server.verifyAuthenticationMetrics(0, 2); - } - /** Checks connection failed using the specified {@code tlsVersion}. */ private void checkAuthentiationFailed(String node, String tlsVersion) throws IOException { sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(tlsVersion)); @@ -626,104 +601,11 @@ public void testUnsupportedTLSVersion() throws Exception { server.verifyAuthenticationMetrics(0, 1); } - /** - * Tests that connections fails if TLSv1.3 enabled but cipher suite suitable only for TLSv1.2 used. - */ - @Test - public void testCiphersSuiteForTls12FailsForTls13() throws Exception { - assumeTrue(Java.IS_JAVA11_COMPATIBLE); - - String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; - - sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - server = createEchoServer(SecurityProtocol.SSL); - - sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - - checkAuthentiationFailed("0", "TLSv1.3"); - server.verifyAuthenticationMetrics(0, 1); - } - - /** - * Tests that connections can't be made if server uses TLSv1.2 with custom cipher suite and client uses TLSv1.3. - */ - @Test - public void testCiphersSuiteFailForServerTls12ClientTls13() throws Exception { - assumeTrue(Java.IS_JAVA11_COMPATIBLE); - - String tls12CipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; - String tls13CipherSuite = "TLS_AES_128_GCM_SHA256"; - - sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.2")); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls12CipherSuite)); - server = createEchoServer(SecurityProtocol.SSL); - - sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls13CipherSuite)); - - checkAuthentiationFailed("0", "TLSv1.3"); - server.verifyAuthenticationMetrics(0, 1); - } - - /** - * Tests that connections can be made with TLSv1.3 cipher suite. - */ - @Test - public void testCiphersSuiteForTls13() throws Exception { - assumeTrue(Java.IS_JAVA11_COMPATIBLE); - - String node = "0"; - String cipherSuite = "TLS_AES_128_GCM_SHA256"; - - sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - server = createEchoServer(SecurityProtocol.SSL); - - sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); - sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - createSelector(sslClientConfigs); - InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); - selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - NetworkTestUtils.waitForChannelReady(selector, node); - server.verifyAuthenticationMetrics(1, 0); - } - - /** - * Tests that connections can be made with TLSv1.2 cipher suite. - */ - @Test - public void testCiphersSuiteForTls12() throws Exception { - String node = "0"; - String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; - - sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); - sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - server = createEchoServer(SecurityProtocol.SSL); - - sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, SslConfigs.DEFAULT_SSL_PROTOCOL); - sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); - sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); - createSelector(sslClientConfigs); - InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); - selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - - NetworkTestUtils.waitForChannelReady(selector, node); - server.verifyAuthenticationMetrics(1, 0); - } - /** * Tests that connections cannot be made with unsupported TLS cipher suites */ @Test public void testUnsupportedCiphers() throws Exception { - String node = "0"; SSLContext context = SSLContext.getInstance(tlsProtocol); context.init(null, null, null); String[] cipherSuites = context.getDefaultSSLParameters().getCipherSuites(); @@ -732,11 +614,8 @@ public void testUnsupportedCiphers() throws Exception { sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Arrays.asList(cipherSuites[1])); createSelector(sslClientConfigs); - InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); - selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE); - NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.AUTHENTICATION_FAILED); - server.verifyAuthenticationMetrics(0, 1); + checkAuthentiationFailed("1", tlsProtocol); } @Test diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportTls12Tls13Test.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportTls12Tls13Test.java new file mode 100644 index 0000000000000..81b86d4e6a1b7 --- /dev/null +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportTls12Tls13Test.java @@ -0,0 +1,169 @@ +/* + * 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.kafka.common.network; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import org.apache.kafka.common.config.SslConfigs; +import org.apache.kafka.common.metrics.Metrics; +import org.apache.kafka.common.security.TestSecurityConfig; +import org.apache.kafka.common.security.auth.SecurityProtocol; +import org.apache.kafka.common.utils.Java; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Time; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assume.assumeTrue; + +public class SslTransportTls12Tls13Test { + private static final int BUFFER_SIZE = 4 * 1024; + private static final Time TIME = Time.SYSTEM; + + private NioEchoServer server; + private Selector selector; + private Map sslClientConfigs; + private Map sslServerConfigs; + + @Before + public void setup() throws Exception { + // Create certificates for use by client and server. Add server cert to client truststore and vice versa. + CertStores serverCertStores = new CertStores(true, "server", "localhost"); + CertStores clientCertStores = new CertStores(false, "client", "localhost"); + sslServerConfigs = serverCertStores.getTrustingConfig(clientCertStores); + sslClientConfigs = clientCertStores.getTrustingConfig(serverCertStores); + + LogContext logContext = new LogContext(); + ChannelBuilder channelBuilder = new SslChannelBuilder(Mode.CLIENT, null, false, logContext); + channelBuilder.configure(sslClientConfigs); + this.selector = new Selector(5000, new Metrics(), TIME, "MetricGroup", channelBuilder, logContext); + } + + @After + public void teardown() throws Exception { + if (selector != null) + this.selector.close(); + if (server != null) + this.server.close(); + } + + /** + * Tests that connections fails if TLSv1.3 enabled but cipher suite suitable only for TLSv1.2 used. + */ + @Test + public void testCiphersSuiteForTls12FailsForTls13() throws Exception { + assumeTrue(Java.IS_JAVA11_COMPATIBLE); + + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), + SecurityProtocol.SSL, new TestSecurityConfig(sslServerConfigs), null, TIME); + + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.3")); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + + checkAuthentiationFailed(); + } + + /** + * Tests that connections can't be made if server uses TLSv1.2 with custom cipher suite and client uses TLSv1.3. + */ + @Test + public void testCiphersSuiteFailForServerTls12ClientTls13() throws Exception { + assumeTrue(Java.IS_JAVA11_COMPATIBLE); + + String tls12CipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + String tls13CipherSuite = "TLS_AES_128_GCM_SHA256"; + + sslServerConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.2"); + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Collections.singletonList("TLSv1.2")); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls12CipherSuite)); + server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), + SecurityProtocol.SSL, new TestSecurityConfig(sslServerConfigs), null, TIME); + + sslClientConfigs.put(SslConfigs.SSL_PROTOCOL_CONFIG, "TLSv1.3"); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(tls13CipherSuite)); + + checkAuthentiationFailed(); + } + + /** + * Tests that connections can be made with TLSv1.3 cipher suite. + */ + @Test + public void testCiphersSuiteForTls13() throws Exception { + assumeTrue(Java.IS_JAVA11_COMPATIBLE); + + String cipherSuite = "TLS_AES_128_GCM_SHA256"; + + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), + SecurityProtocol.SSL, new TestSecurityConfig(sslServerConfigs), null, TIME); + + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + checkAuthenticationSucceed(); + } + + /** + * Tests that connections can be made with TLSv1.2 cipher suite. + */ + @Test + public void testCiphersSuiteForTls12() throws Exception { + String cipherSuite = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"; + + sslServerConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslServerConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), + SecurityProtocol.SSL, new TestSecurityConfig(sslServerConfigs), null, TIME); + + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList(SslConfigs.DEFAULT_SSL_ENABLED_PROTOCOLS.split(","))); + sslClientConfigs.put(SslConfigs.SSL_CIPHER_SUITES_CONFIG, Collections.singletonList(cipherSuite)); + checkAuthenticationSucceed(); + } + + /** Checks connection failed using the specified {@code tlsVersion}. */ + private void checkAuthentiationFailed() throws IOException, InterruptedException { + sslClientConfigs.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, Arrays.asList("TLSv1.3")); + createSelector(sslClientConfigs); + InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); + selector.connect("0", addr, BUFFER_SIZE, BUFFER_SIZE); + + NetworkTestUtils.waitForChannelClose(selector, "0", ChannelState.State.AUTHENTICATION_FAILED); + server.verifyAuthenticationMetrics(0, 1); + } + + private void checkAuthenticationSucceed() throws IOException, InterruptedException { + createSelector(sslClientConfigs); + InetSocketAddress addr = new InetSocketAddress("localhost", server.port()); + selector.connect("0", addr, BUFFER_SIZE, BUFFER_SIZE); + NetworkTestUtils.waitForChannelReady(selector, "0"); + server.verifyAuthenticationMetrics(1, 0); + } + + private void createSelector(Map sslClientConfigs) { + SslTransportLayerTest.TestSslChannelBuilder channelBuilder = new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); + channelBuilder.configureBufferSizes(null, null, null); + channelBuilder.configure(sslClientConfigs); + this.selector = new Selector(100 * 5000, new Metrics(), TIME, "MetricGroup", channelBuilder, new LogContext()); + } +} From 14bf85aab93eabd5eb5045315edf104079b4d353 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 2 Jun 2020 15:38:30 +0300 Subject: [PATCH 27/31] KAFKA-9320: TLSv1.3 vs TLSv1.2 explanation comments. --- .../common/network/SslTransportLayerTest.java | 1 + .../SslVersionsTransportLayerTest.java | 65 ++++++++++++++----- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java index feeebe153beeb..ac94817dc8ffb 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslTransportLayerTest.java @@ -616,6 +616,7 @@ public void testUnsupportedCiphers() throws Exception { createSelector(sslClientConfigs); checkAuthentiationFailed("1", tlsProtocol); + server.verifyAuthenticationMetrics(0, 1); } @Test diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index 844ab01bf4452..af25e5702c4b3 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -46,13 +46,15 @@ public class SslVersionsTransportLayerTest { private static final int BUFFER_SIZE = 4 * 1024; private static final Time TIME = Time.SYSTEM; - private final List tlsServerProtocols; - private final List tlsClientProtocols; + private final List serverProtocols; + private final List clientProtocols; @Parameterized.Parameters(name = "tlsServerProtocol={0},tlsClientProtocol={1}") public static Collection data() { List values = new ArrayList<>(); + values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.2")}); + if (Java.IS_JAVA11_COMPATIBLE) { values.add(new Object[] {Collections.singletonList("TLSv1.2"), Collections.singletonList("TLSv1.3")}); values.add(new Object[] {Collections.singletonList("TLSv1.3"), Collections.singletonList("TLSv1.2")}); @@ -69,14 +71,18 @@ public static Collection data() { values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Collections.singletonList("TLSv1.2")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.2", "TLSv1.3")}); values.add(new Object[] {Arrays.asList("TLSv1.2", "TLSv1.3"), Arrays.asList("TLSv1.3", "TLSv1.2")}); - } return values; } - public SslVersionsTransportLayerTest(List tlsServerProtocols, List tlsClientProtocols) { - this.tlsServerProtocols = tlsServerProtocols; - this.tlsClientProtocols = tlsClientProtocols; + /** + * Be aware that you can turn on debug mode for a javax.net.ssl library with the line {@code System.setProperty("javax.net.debug", "ssl:handshake");} + * @param serverProtocols Server protocols. + * @param clientProtocols Client protocols. + */ + public SslVersionsTransportLayerTest(List serverProtocols, List clientProtocols) { + this.serverProtocols = serverProtocols; + this.clientProtocols = clientProtocols; } /** @@ -88,20 +94,20 @@ public void testTlsDefaults() throws Exception { CertStores serverCertStores = new CertStores(true, "server", "localhost"); CertStores clientCertStores = new CertStores(false, "client", "localhost"); - Map sslClientConfigs = getTrustingConfig(clientCertStores, serverCertStores, tlsClientProtocols); - Map sslServerConfigs = getTrustingConfig(serverCertStores, clientCertStores, tlsServerProtocols); + Map sslClientConfigs = getTrustingConfig(clientCertStores, serverCertStores, clientProtocols); + Map sslServerConfigs = getTrustingConfig(serverCertStores, clientCertStores, serverProtocols); NioEchoServer server = NetworkTestUtils.createEchoServer(ListenerName.forSecurityProtocol(SecurityProtocol.SSL), - SecurityProtocol.SSL, - new TestSecurityConfig(sslServerConfigs), - null, + SecurityProtocol.SSL, + new TestSecurityConfig(sslServerConfigs), + null, TIME); Selector selector = createSelector(sslClientConfigs); String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); - if (!Collections.disjoint(tlsServerProtocols, tlsClientProtocols)) { + if (isCompatible(serverProtocols, clientProtocols)) { NetworkTestUtils.waitForChannelReady(selector, node); int msgSz = 1024 * 1024; @@ -117,24 +123,51 @@ public void testTlsDefaults() throws Exception { server.waitForMetric("response", 1); } else { NetworkTestUtils.waitForChannelClose(selector, node, ChannelState.State.AUTHENTICATION_FAILED); + server.verifyAuthenticationMetrics(0, 1); } } + /** + *

    + * The explanation of this check in the structure of the ClientHello SSL message. + * Please, take a look at the Guide, + * "Send ClientHello Message" section. + *

    + * > Client version: For TLS 1.3, this has a fixed value, TLSv1.2; TLS 1.3 uses the extension supported_versions and not this field to negotiate protocol version + * ... + * > supported_versions: Lists which versions of TLS the client supports. In particular, if the client + * > requests TLS 1.3, then the client version field has the value TLSv1.2 and this extension + * > contains the value TLSv1.3; if the client requests TLS 1.2, then the client version field has the + * > value TLSv1.2 and this extension either doesn’t exist or contains the value TLSv1.2 but not the value TLSv1.3. + *

    + * + * This mean that TLSv1.3 client can fallback to TLSv1.2 but TLSv1.2 client can't change protocol to TLSv1.3. + * + * @param serverProtocols Server protocols. + * @param clientProtocols Client protocols. + * @return {@code True} if client should be able to connect to the server. + */ + private boolean isCompatible(List serverProtocols, List clientProtocols) { + return serverProtocols.contains(clientProtocols.get(0)) || + (clientProtocols.get(0).equals("TLSv1.3") && clientProtocols.contains("TLSv1.2")); + } + private static Map getTrustingConfig(CertStores certStores, CertStores peerCertStores, List tlsProtocols) { Map configs = certStores.getTrustingConfig(peerCertStores); configs.putAll(sslConfig(tlsProtocols)); return configs; } - private static Map sslConfig(List tlsServerProtocols) { + private static Map sslConfig(List tlsProtocols) { Map sslConfig = new HashMap<>(); - sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsServerProtocols.get(0)); - sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, tlsServerProtocols); + sslConfig.put(SslConfigs.SSL_PROTOCOL_CONFIG, tlsProtocols.get(0)); + sslConfig.put(SslConfigs.SSL_ENABLED_PROTOCOLS_CONFIG, tlsProtocols); return sslConfig; } private Selector createSelector(Map sslClientConfigs) { - SslTransportLayerTest.TestSslChannelBuilder channelBuilder = new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); + SslTransportLayerTest.TestSslChannelBuilder channelBuilder = + new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); channelBuilder.configureBufferSizes(null, null, null); channelBuilder.configure(sslClientConfigs); return new Selector(100 * 5000, new Metrics(), TIME, "MetricGroup", channelBuilder, new LogContext()); From 869e342f4cc0dd5b3f7120cfadb3175bb6f48ff7 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 2 Jun 2020 17:02:26 +0300 Subject: [PATCH 28/31] KAFKA-9320: code review fixes. --- .../kafka/common/network/SslVersionsTransportLayerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index af25e5702c4b3..a4c66de97398b 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -102,7 +102,7 @@ public void testTlsDefaults() throws Exception { new TestSecurityConfig(sslServerConfigs), null, TIME); - Selector selector = createSelector(sslClientConfigs); + Selector selector = createClientSelector(sslClientConfigs); String node = "0"; selector.connect(node, new InetSocketAddress("localhost", server.port()), BUFFER_SIZE, BUFFER_SIZE); @@ -165,7 +165,7 @@ private static Map sslConfig(List tlsProtocols) { return sslConfig; } - private Selector createSelector(Map sslClientConfigs) { + private Selector createClientSelector(Map sslClientConfigs) { SslTransportLayerTest.TestSslChannelBuilder channelBuilder = new SslTransportLayerTest.TestSslChannelBuilder(Mode.CLIENT); channelBuilder.configureBufferSizes(null, null, null); From ca81fcd1c1d40491cb756e12e0c26b875e12c0b4 Mon Sep 17 00:00:00 2001 From: Nikolay Izhikov Date: Tue, 2 Jun 2020 17:37:05 +0300 Subject: [PATCH 29/31] KAFKA-9320: code review fixes. --- .../network/SslVersionsTransportLayerTest.java | 16 ++++++++++++---- docs/upgrade.html | 4 ++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java index a4c66de97398b..9f930a7bf77cd 100644 --- a/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java +++ b/clients/src/test/java/org/apache/kafka/common/network/SslVersionsTransportLayerTest.java @@ -37,6 +37,9 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + /** * Tests for the SSL transport layer. * Checks different versions of the protocol usage on the server and client. @@ -143,13 +146,18 @@ public void testTlsDefaults() throws Exception { * * This mean that TLSv1.3 client can fallback to TLSv1.2 but TLSv1.2 client can't change protocol to TLSv1.3. * - * @param serverProtocols Server protocols. - * @param clientProtocols Client protocols. - * @return {@code True} if client should be able to connect to the server. + * @param serverProtocols Server protocols. Expected to be non empty. + * @param clientProtocols Client protocols. Expected to be non empty. + * @return {@code true} if client should be able to connect to the server. */ private boolean isCompatible(List serverProtocols, List clientProtocols) { + assertNotNull(serverProtocols); + assertFalse(serverProtocols.isEmpty()); + assertNotNull(clientProtocols); + assertFalse(clientProtocols.isEmpty()); + return serverProtocols.contains(clientProtocols.get(0)) || - (clientProtocols.get(0).equals("TLSv1.3") && clientProtocols.contains("TLSv1.2")); + (clientProtocols.get(0).equals("TLSv1.3") && !Collections.disjoint(serverProtocols, clientProtocols)); } private static Map getTrustingConfig(CertStores certStores, CertStores peerCertStores, List tlsProtocols) { diff --git a/docs/upgrade.html b/docs/upgrade.html index 103fb99feaf9b..4ab43d7345e56 100644 --- a/docs/upgrade.html +++ b/docs/upgrade.html @@ -18,6 +18,10 @@