ijuma commented on a change in pull request #10059: URL: https://github.com/apache/kafka/pull/10059#discussion_r582643189
########## File path: clients/src/main/java/org/apache/kafka/common/network/ChannelBuilderUtils.java ########## @@ -0,0 +1,63 @@ +/* + * 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.channels.SelectionKey; +import java.nio.channels.SocketChannel; + +public class ChannelBuilderUtils { + + /** + * 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> + */ + static String peerHost(SelectionKey key) { Review comment: Instead of doing this, I think we can have the following 3 methods in `SslFactory`. Thoughts? ```java 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."); } if (mode == Mode.SERVER) { return sslEngineFactory.createServerSslEngine(peerHost, peerPort); } else { return sslEngineFactory.createClientSslEngine(peerHost, peerPort, endpointIdentification); } } /** * 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(); }``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
