Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to rely on Netty for Hostname Verification #15824

Merged
merged 4 commits into from
Jun 1, 2022
Merged

Switch to rely on Netty for Hostname Verification #15824

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
037d982
Switch to relying on Netty for Hostname Verification
michaeljmarshall May 27, 2022
3b0695e
Fix AuthenticationTlsHostnameVerificationTest
lhotari May 30, 2022
1cf1a5c
Configure tlsHostnameVerificationEnable for topic lookup connections …
lhotari May 30, 2022
707204b
Fix ProxyWithAuthorizationTest
lhotari May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ...-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public boolean keepAlive(InetSocketAddress remoteAddress, Request ahcRequest,
params != null ? params.getKeyStoreType() : null,
params != null ? params.getKeyStorePath() : null,
params != null ? params.getKeyStorePassword() : null,
conf.isTlsAllowInsecureConnection() || !conf.isTlsHostnameVerificationEnable(),
conf.isTlsAllowInsecureConnection(),
conf.getTlsTrustStoreType(),
conf.getTlsTrustStorePath(),
conf.getTlsTrustStorePassword(),
Expand All @@ -148,27 +148,28 @@ public boolean keepAlive(InetSocketAddress remoteAddress, Request ahcRequest,
sslCtx = authData.getTlsTrustStoreStream() == null
? SecurityUtility.createAutoRefreshSslContextForClient(
sslProvider,
conf.isTlsAllowInsecureConnection() || !conf.isTlsHostnameVerificationEnable(),
conf.isTlsAllowInsecureConnection(),
conf.getTlsTrustCertsFilePath(), authData.getTlsCerificateFilePath(),
authData.getTlsPrivateKeyFilePath(), null, autoCertRefreshTimeSeconds, delayer)
: SecurityUtility.createNettySslContextForClient(
sslProvider,
conf.isTlsAllowInsecureConnection() || !conf.isTlsHostnameVerificationEnable(),
conf.isTlsAllowInsecureConnection(),
authData.getTlsTrustStoreStream(), authData.getTlsCertificates(),
authData.getTlsPrivateKey(),
conf.getTlsCiphers(),
conf.getTlsProtocols());
} else {
sslCtx = SecurityUtility.createNettySslContextForClient(
sslProvider,
conf.isTlsAllowInsecureConnection() || !conf.isTlsHostnameVerificationEnable(),
conf.isTlsAllowInsecureConnection(),
conf.getTlsTrustCertsFilePath(),
conf.getTlsCiphers(),
conf.getTlsProtocols());
}
confBuilder.setSslContext(sslCtx);
}
}
confBuilder.setDisableHttpsEndpointIdentificationAlgorithm(!conf.isTlsHostnameVerificationEnable());
}
httpClient = new DefaultAsyncHttpClient(confBuilder.build());
this.readTimeout = Duration.ofMillis(readTimeoutMs);
Expand Down
49 changes: 0 additions & 49 deletions pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@
import com.google.common.collect.Queues;
import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.unix.Errors.NativeIoException;
import io.netty.handler.codec.LengthFieldBasedFrameDecoder;
import io.netty.handler.ssl.SslHandler;
import io.netty.util.concurrent.Promise;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
Expand All @@ -45,7 +43,6 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
import javax.net.ssl.SSLSession;
import lombok.Getter;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.lang3.tuple.Pair;
Expand Down Expand Up @@ -93,7 +90,6 @@
import org.apache.pulsar.common.protocol.PulsarHandler;
import org.apache.pulsar.common.protocol.schema.SchemaVersion;
import org.apache.pulsar.common.schema.SchemaInfo;
import org.apache.pulsar.common.tls.TlsHostnameVerifier;
import org.apache.pulsar.common.util.FutureUtil;
import org.apache.pulsar.common.util.collections.ConcurrentLongHashMap;
import org.slf4j.Logger;
Expand Down Expand Up @@ -152,9 +148,6 @@ public class ClientCnx extends PulsarHandler {
protected String proxyToTargetBrokerAddress = null;
// Remote hostName with which client is connected
protected String remoteHostName = null;
private boolean isTlsHostnameVerificationEnable;

private static final TlsHostnameVerifier HOSTNAME_VERIFIER = new TlsHostnameVerifier();

private ScheduledFuture<?> timeoutTask;
private SocketAddress localAddress;
Expand Down Expand Up @@ -221,7 +214,6 @@ public ClientCnx(ClientConfigurationData conf, EventLoopGroup eventLoopGroup, in
this.maxNumberOfRejectedRequestPerConnection = conf.getMaxNumberOfRejectedRequestPerConnection();
this.operationTimeoutMs = conf.getOperationTimeoutMs();
this.state = State.None;
this.isTlsHostnameVerificationEnable = conf.isTlsHostnameVerificationEnable();
this.protocolVersion = protocolVersion;
}

Expand Down Expand Up @@ -322,14 +314,6 @@ public static boolean isKnownException(Throwable t) {

@Override
protected void handleConnected(CommandConnected connected) {

if (isTlsHostnameVerificationEnable && remoteHostName != null && !verifyTlsHostName(remoteHostName, ctx)) {
// close the connection if host-verification failed with the broker
log.warn("[{}] Failed to verify hostname of {}", ctx.channel(), remoteHostName);
ctx.close();
return;
}

checkArgument(state == State.SentConnectFrame || state == State.Connecting);
if (connected.hasMaxMessageSize()) {
if (log.isDebugEnabled()) {
Expand Down Expand Up @@ -1082,39 +1066,6 @@ private void incrementRejectsAndMaybeClose() {
}
}

/**
* verifies host name provided in x509 Certificate in tls session
*
* it matches hostname with below scenarios
*
* <pre>
* 1. Supports IPV4 and IPV6 host matching
* 2. Supports wild card matching for DNS-name
* eg:
* HostName CN Result
* 1. localhost localhost PASS
* 2. localhost local* PASS
* 3. pulsar1-broker.com pulsar*.com PASS
* </pre>
*
* @param ctx
* @return true if hostname is verified else return false
*/
private boolean verifyTlsHostName(String hostname, ChannelHandlerContext ctx) {
ChannelHandler sslHandler = ctx.channel().pipeline().get("tls");

SSLSession sslSession = null;
if (sslHandler != null) {
sslSession = ((SslHandler) sslHandler).engine().getSession();
if (log.isDebugEnabled()) {
log.debug("Verifying HostName for {}, Cipher {}, Protocols {}", hostname, sslSession.getCipherSuite(),
sslSession.getProtocol());
}
return HOSTNAME_VERIFIER.verify(hostname, sslSession);
}
return false;
}

void registerConsumer(final long consumerId, final ConsumerImpl<?> consumer) {
consumers.put(consumerId, consumer);
}
Expand Down
1 change: 1 addition & 0 deletions pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ public boolean keepAlive(InetSocketAddress remoteAddress, Request ahcRequest,
}

confBuilder.setUseInsecureTrustManager(conf.isTlsAllowInsecureConnection());
confBuilder.setDisableHttpsEndpointIdentificationAlgorithm(!conf.isTlsHostnameVerificationEnable());
} catch (GeneralSecurityException e) {
throw new PulsarClientException.InvalidConfigurationException(e);
} catch (Exception e) {
Expand Down
7 changes: 7 additions & 0 deletions pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class PulsarChannelInitializer extends ChannelInitializer<SocketChannel>
private final Supplier<ClientCnx> clientCnxSupplier;
@Getter
private final boolean tlsEnabled;
private final boolean tlsHostnameVerificationEnabled;
private final boolean tlsEnabledWithKeyStore;
private final InetSocketAddress socks5ProxyAddress;
private final String socks5ProxyUsername;
Expand All @@ -66,6 +67,7 @@ public PulsarChannelInitializer(ClientConfigurationData conf, Supplier<ClientCnx
super();
this.clientCnxSupplier = clientCnxSupplier;
this.tlsEnabled = conf.isUseTls();
this.tlsHostnameVerificationEnabled = conf.isTlsHostnameVerificationEnable();
this.socks5ProxyAddress = conf.getSocks5ProxyAddress();
this.socks5ProxyUsername = conf.getSocks5ProxyUsername();
this.socks5ProxyPassword = conf.getSocks5ProxyPassword();
Expand Down Expand Up @@ -167,6 +169,11 @@ CompletableFuture<Channel> initTls(Channel ch, InetSocketAddress sniHost) {
? new SslHandler(nettySSLContextAutoRefreshBuilder.get()
.createSSLEngine(sniHost.getHostString(), sniHost.getPort()))
: sslContextSupplier.get().newHandler(ch.alloc(), sniHost.getHostString(), sniHost.getPort());

if (tlsHostnameVerificationEnabled) {
SecurityUtility.configureSSLHandler(handler);
}

ch.pipeline().addFirst(TLS_HANDLER, handler);
initTlsFuture.complete(ch);
} catch (Throwable t) {
Expand Down
3 changes: 1 addition & 2 deletions ...ar-common/src/main/java/org/apache/pulsar/common/util/NettyClientSslContextRefresher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ public NettyClientSslContextRefresher(SslProvider sslProvider, boolean allowInse
AuthenticationDataProvider authData,
Set<String> ciphers,
Set<String> protocols,
long delayInSeconds)
throws IOException, GeneralSecurityException {
long delayInSeconds) {
super(delayInSeconds);
this.tlsAllowInsecureConnection = allowInsecure;
this.tlsTrustCertsFilePath = new FileModifiedTimeUpdater(trustCertsFilePath);
Expand Down
10 changes: 10 additions & 0 deletions pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.netty.handler.ssl.ClientAuth;
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslHandler;
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
import java.io.BufferedReader;
Expand Down Expand Up @@ -57,7 +58,9 @@
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -548,6 +551,13 @@ private static void setupClientAuthentication(SslContextBuilder builder,
}
}

public static void configureSSLHandler(SslHandler handler) {
SSLEngine sslEngine = handler.engine();
SSLParameters sslParameters = sslEngine.getSSLParameters();
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
sslEngine.setSSLParameters(sslParameters);
}

public static Provider resolveProvider(String providerName) throws NoSuchAlgorithmException {
Provider provider = null;
if (!StringUtils.isEmpty(providerName)) {
Expand Down
7 changes: 4 additions & 3 deletions pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,11 @@ protected HttpClient newHttpClient() {
);
}


SslContextFactory contextFactory = new SslContextFactory.Client(true);
SslContextFactory contextFactory = new SslContextFactory.Client();
contextFactory.setSslContext(sslCtx);

if (!config.isTlsHostnameVerificationEnabled()) {
contextFactory.setEndpointIdentificationAlgorithm(null);
}
return new JettyHttpClient(contextFactory);
} catch (Exception e) {
LOG.error("new jetty http client exception ", e);
Expand Down
Loading