This is an automated email from the ASF dual-hosted git repository.
manikumar pushed a commit to branch 2.7
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/2.7 by this push:
new 45abdc2 KAFKA-8562; SaslChannelBuilder - Avoid (reverse) DNS lookup
while building SslTransportLayer (#10059)
45abdc2 is described below
commit 45abdc2db8de25783eeed3b786e5c6b30071981c
Author: Davor Poldrugo <[email protected]>
AuthorDate: Fri Feb 26 02:02:40 2021 +0100
KAFKA-8562; SaslChannelBuilder - Avoid (reverse) DNS lookup while building
SslTransportLayer (#10059)
This patch moves the `peerHost` helper defined in `SslChannelBuilder` into
`SslFactor`. `SaslChannelBuilder` is then updated to use a new
`createSslEngine` overload which relies on `peerHost` when building its
`SslEngine`. The purpose is to avoid the reverse DNS in `getHostName`.
Reviewers: Ismael Juma <[email protected]>, Manikumar Reddy
<[email protected]>, Jason Gustafson <[email protected]>
---
.../kafka/common/network/SaslChannelBuilder.java | 3 +-
.../kafka/common/network/SslChannelBuilder.java | 48 ++--------------------
.../kafka/common/security/ssl/SslFactory.java | 48 ++++++++++++++++++++++
.../kafka/common/network/SslSelectorTest.java | 4 +-
.../common/network/SslTransportLayerTest.java | 4 +-
.../SaslAuthenticatorFailureDelayTest.java | 2 +-
.../authenticator/SaslAuthenticatorTest.java | 10 ++---
7 files changed, 62 insertions(+), 57 deletions(-)
diff --git
a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
index f01c4ef..25b04c9 100644
---
a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
+++
b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
@@ -244,8 +244,7 @@ public class SaslChannelBuilder implements ChannelBuilder,
ListenerReconfigurabl
ChannelMetadataRegistry
metadataRegistry) throws IOException {
if (this.securityProtocol == SecurityProtocol.SASL_SSL) {
return SslTransportLayer.create(id, key,
-
sslFactory.createSslEngine(socketChannel.socket().getInetAddress().getHostName(),
- socketChannel.socket().getPort()),
+ sslFactory.createSslEngine(socketChannel.socket()),
metadataRegistry);
} else {
return new PlaintextTransportLayer(key);
diff --git
a/clients/src/main/java/org/apache/kafka/common/network/SslChannelBuilder.java
b/clients/src/main/java/org/apache/kafka/common/network/SslChannelBuilder.java
index 327fff8..a093ebe 100644
---
a/clients/src/main/java/org/apache/kafka/common/network/SslChannelBuilder.java
+++
b/clients/src/main/java/org/apache/kafka/common/network/SslChannelBuilder.java
@@ -32,7 +32,6 @@ import org.slf4j.Logger;
import java.io.Closeable;
import java.io.IOException;
import java.net.InetAddress;
-import java.net.InetSocketAddress;
import java.nio.channels.SelectionKey;
import java.nio.channels.SocketChannel;
import java.util.Map;
@@ -101,8 +100,7 @@ public class SslChannelBuilder implements ChannelBuilder,
ListenerReconfigurable
public KafkaChannel buildChannel(String id, SelectionKey key, int
maxReceiveSize,
MemoryPool memoryPool,
ChannelMetadataRegistry metadataRegistry) throws KafkaException {
try {
- SslTransportLayer transportLayer = buildTransportLayer(sslFactory,
id, key,
- peerHost(key), metadataRegistry);
+ SslTransportLayer transportLayer = buildTransportLayer(sslFactory,
id, key, metadataRegistry);
Supplier<Authenticator> authenticatorCreator = () ->
new SslAuthenticator(configs, transportLayer, listenerName,
sslPrincipalMapper);
return new KafkaChannel(id, transportLayer, authenticatorCreator,
maxReceiveSize,
@@ -118,53 +116,13 @@ public class SslChannelBuilder implements ChannelBuilder,
ListenerReconfigurable
if (sslFactory != null) sslFactory.close();
}
- protected SslTransportLayer buildTransportLayer(SslFactory sslFactory,
String id, SelectionKey key,
- String host,
ChannelMetadataRegistry metadataRegistry) throws IOException {
+ protected SslTransportLayer buildTransportLayer(SslFactory sslFactory,
String id, SelectionKey key, ChannelMetadataRegistry metadataRegistry) throws
IOException {
SocketChannel socketChannel = (SocketChannel) key.channel();
- return SslTransportLayer.create(id, key,
sslFactory.createSslEngine(host, socketChannel.socket().getPort()),
+ return SslTransportLayer.create(id, key,
sslFactory.createSslEngine(socketChannel.socket()),
metadataRegistry);
}
/**
- * Returns host/IP address of remote host without reverse DNS lookup to be
used as the host
- * for creating SSL engine. This is used as a hint for session reuse
strategy and also for
- * hostname verification of server hostnames.
- * <p>
- * Scenarios:
- * <ul>
- * <li>Server-side
- * <ul>
- * <li>Server accepts connection from a client. Server knows only
client IP
- * address. We want to avoid reverse DNS lookup of the client IP
address since the server
- * does not verify or use client hostname. The IP address can be used
directly.</li>
- * </ul>
- * </li>
- * <li>Client-side
- * <ul>
- * <li>Client connects to server using hostname. No lookup is necessary
- * and the hostname should be used to create the SSL engine. This
hostname is validated
- * against the hostname in SubjectAltName (dns) or CommonName in the
certificate if
- * hostname verification is enabled. Authentication fails if hostname
does not match.</li>
- * <li>Client connects to server using IP address, but certificate
contains only
- * SubjectAltName (dns). Use of reverse DNS lookup to determine
hostname introduces
- * a security vulnerability since authentication would be reliant on a
secure DNS.
- * Hence hostname verification should fail in this case.</li>
- * <li>Client connects to server using IP address and certificate
contains
- * SubjectAltName (ipaddress). This could be used when Kafka is on a
private network.
- * If reverse DNS lookup is used, authentication would succeed using
IP address if lookup
- * fails and IP address is used, but authentication would fail if
lookup succeeds and
- * dns name is used. For consistency and to avoid dependency on a
potentially insecure
- * DNS, reverse DNS lookup should be avoided and the IP address
specified by the client for
- * connection should be used to create the SSL engine.</li>
- * </ul></li>
- * </ul>
- */
- private String peerHost(SelectionKey key) {
- SocketChannel socketChannel = (SocketChannel) key.channel();
- return new InetSocketAddress(socketChannel.socket().getInetAddress(),
0).getHostString();
- }
-
- /**
* Note that client SSL authentication is handled in {@link
SslTransportLayer}. This class is only used
* to transform the derived principal using a {@link
KafkaPrincipalBuilder} configured by the user.
*/
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
index 9b305db..e1372bf 100644
--- a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
+++ b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
@@ -31,6 +31,8 @@ import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLException;
import java.io.Closeable;
+import java.net.InetSocketAddress;
+import java.net.Socket;
import java.nio.ByteBuffer;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
@@ -181,6 +183,14 @@ public class SslFactory implements Reconfigurable,
Closeable {
}
}
+ public SSLEngine createSslEngine(Socket socket) {
+ return createSslEngine(peerHost(socket), socket.getPort());
+ }
+
+ /**
+ * Prefer `createSslEngine(Socket)` if a `Socket` instance is available.
If using this overload,
+ * avoid reverse DNS resolution in the computation of `peerHost`.
+ */
public SSLEngine createSslEngine(String peerHost, int peerPort) {
if (sslEngineFactory == null) {
throw new IllegalStateException("SslFactory has not been
configured.");
@@ -192,6 +202,44 @@ public class SslFactory implements Reconfigurable,
Closeable {
}
}
+ /**
+ * Returns host/IP address of remote host without reverse DNS lookup to be
used as the host
+ * for creating SSL engine. This is used as a hint for session reuse
strategy and also for
+ * hostname verification of server hostnames.
+ * <p>
+ * Scenarios:
+ * <ul>
+ * <li>Server-side
+ * <ul>
+ * <li>Server accepts connection from a client. Server knows only
client IP
+ * address. We want to avoid reverse DNS lookup of the client IP
address since the server
+ * does not verify or use client hostname. The IP address can be used
directly.</li>
+ * </ul>
+ * </li>
+ * <li>Client-side
+ * <ul>
+ * <li>Client connects to server using hostname. No lookup is necessary
+ * and the hostname should be used to create the SSL engine. This
hostname is validated
+ * against the hostname in SubjectAltName (dns) or CommonName in the
certificate if
+ * hostname verification is enabled. Authentication fails if hostname
does not match.</li>
+ * <li>Client connects to server using IP address, but certificate
contains only
+ * SubjectAltName (dns). Use of reverse DNS lookup to determine
hostname introduces
+ * a security vulnerability since authentication would be reliant on a
secure DNS.
+ * Hence hostname verification should fail in this case.</li>
+ * <li>Client connects to server using IP address and certificate
contains
+ * SubjectAltName (ipaddress). This could be used when Kafka is on a
private network.
+ * If reverse DNS lookup is used, authentication would succeed using
IP address if lookup
+ * fails and IP address is used, but authentication would fail if
lookup succeeds and
+ * dns name is used. For consistency and to avoid dependency on a
potentially insecure
+ * DNS, reverse DNS lookup should be avoided and the IP address
specified by the client for
+ * connection should be used to create the SSL engine.</li>
+ * </ul></li>
+ * </ul>
+ */
+ private String peerHost(Socket socket) {
+ return new InetSocketAddress(socket.getInetAddress(),
0).getHostString();
+ }
+
public SslEngineFactory sslEngineFactory() {
return sslEngineFactory;
}
diff --git
a/clients/src/test/java/org/apache/kafka/common/network/SslSelectorTest.java
b/clients/src/test/java/org/apache/kafka/common/network/SslSelectorTest.java
index 8a36fbf..7c48d09 100644
--- a/clients/src/test/java/org/apache/kafka/common/network/SslSelectorTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/network/SslSelectorTest.java
@@ -382,9 +382,9 @@ public class SslSelectorTest extends SelectorTest {
@Override
protected SslTransportLayer buildTransportLayer(SslFactory sslFactory,
String id, SelectionKey key,
- String host,
ChannelMetadataRegistry metadataRegistry) throws IOException {
+
ChannelMetadataRegistry metadataRegistry) throws IOException {
SocketChannel socketChannel = (SocketChannel) key.channel();
- SSLEngine sslEngine = sslFactory.createSslEngine(host,
socketChannel.socket().getPort());
+ SSLEngine sslEngine =
sslFactory.createSslEngine(socketChannel.socket());
TestSslTransportLayer transportLayer = new
TestSslTransportLayer(id, key, sslEngine, metadataRegistry);
return transportLayer;
}
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 ccfb2a9..9b659ec 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
@@ -1319,9 +1319,9 @@ public class SslTransportLayerTest {
@Override
protected SslTransportLayer buildTransportLayer(SslFactory sslFactory,
String id, SelectionKey key,
- String host,
ChannelMetadataRegistry metadataRegistry) throws IOException {
+
ChannelMetadataRegistry metadataRegistry) throws IOException {
SocketChannel socketChannel = (SocketChannel) key.channel();
- SSLEngine sslEngine = sslFactory.createSslEngine(host,
socketChannel.socket().getPort());
+ SSLEngine sslEngine =
sslFactory.createSslEngine(socketChannel.socket());
return newTransportLayer(id, key, sslEngine);
}
diff --git
a/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorFailureDelayTest.java
b/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorFailureDelayTest.java
index 19003ed..df4e041 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorFailureDelayTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorFailureDelayTest.java
@@ -238,7 +238,7 @@ public class SaslAuthenticatorFailureDelayTest {
private void createClientConnection(SecurityProtocol securityProtocol,
String node) throws Exception {
createSelector(securityProtocol, saslClientConfigs);
- InetSocketAddress addr = new InetSocketAddress("127.0.0.1",
server.port());
+ 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/security/authenticator/SaslAuthenticatorTest.java
b/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorTest.java
index 5c1ce3c..d6f80fd 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorTest.java
@@ -248,7 +248,7 @@ public class SaslAuthenticatorTest {
SecurityProtocol securityProtocol = SecurityProtocol.SASL_SSL;
server = createEchoServer(securityProtocol);
createSelector(securityProtocol, saslClientConfigs);
- InetSocketAddress addr = new InetSocketAddress("127.0.0.1",
server.port());
+ InetSocketAddress addr = new InetSocketAddress("localhost",
server.port());
try {
selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE);
fail("SASL/PLAIN channel created without username");
@@ -272,7 +272,7 @@ public class SaslAuthenticatorTest {
SecurityProtocol securityProtocol = SecurityProtocol.SASL_SSL;
server = createEchoServer(securityProtocol);
createSelector(securityProtocol, saslClientConfigs);
- InetSocketAddress addr = new InetSocketAddress("127.0.0.1",
server.port());
+ InetSocketAddress addr = new InetSocketAddress("localhost",
server.port());
try {
selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE);
fail("SASL/PLAIN channel created without password");
@@ -389,7 +389,7 @@ public class SaslAuthenticatorTest {
saslClientConfigs.put(SaslConfigs.SASL_MECHANISM, "DIGEST-MD5");
createSelector(securityProtocol, saslClientConfigs);
selector2 = selector;
- InetSocketAddress addr = new InetSocketAddress("127.0.0.1",
server.port());
+ InetSocketAddress addr = new InetSocketAddress("localhost",
server.port());
selector.connect(node2, addr, BUFFER_SIZE, BUFFER_SIZE);
NetworkTestUtils.checkClientConnection(selector, node2, 100, 10);
selector = null; // keeps it from being closed when next one is
created
@@ -399,7 +399,7 @@ public class SaslAuthenticatorTest {
saslClientConfigs.put(SaslConfigs.SASL_MECHANISM, "SCRAM-SHA-256");
createSelector(securityProtocol, saslClientConfigs);
selector3 = selector;
- selector.connect(node3, new InetSocketAddress("127.0.0.1",
server.port()), BUFFER_SIZE, BUFFER_SIZE);
+ selector.connect(node3, new InetSocketAddress("localhost",
server.port()), BUFFER_SIZE, BUFFER_SIZE);
NetworkTestUtils.checkClientConnection(selector, node3, 100, 10);
server.verifyAuthenticationMetrics(3, 0);
@@ -1970,7 +1970,7 @@ public class SaslAuthenticatorTest {
};
clientChannelBuilder.configure(saslClientConfigs);
this.selector = NetworkTestUtils.createSelector(clientChannelBuilder,
time);
- InetSocketAddress addr = new InetSocketAddress("127.0.0.1",
server.port());
+ InetSocketAddress addr = new InetSocketAddress("localhost",
server.port());
selector.connect(node, addr, BUFFER_SIZE, BUFFER_SIZE);
}
