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

HBASE-28008 Add support for netty tcnative #5363

Merged
merged 2 commits into from
Sep 12, 2023
Merged

HBASE-28008 Add support for netty tcnative #5363

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1c4821
HBASE-28008 Add support for netty tcnative
bbeaudreault Aug 21, 2023
a78a35d
log openssl available vs enabled
bbeaudreault Sep 11, 2023
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
75 changes: 68 additions & 7 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@
import java.security.Security;
import java.security.cert.PKIXBuilderParameters;
import java.security.cert.X509CertSelector;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.CertPathTrustManagerParameters;
import javax.net.ssl.KeyManager;
Expand All @@ -49,8 +52,10 @@
import org.slf4j.LoggerFactory;

import org.apache.hbase.thirdparty.com.google.common.collect.ObjectArrays;
import org.apache.hbase.thirdparty.io.netty.handler.ssl.OpenSsl;
import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslProvider;

/**
* Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
Expand Down Expand Up @@ -83,9 +88,10 @@ public final class X509Util {
public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
public static final String TLS_CONFIG_REVERSE_DNS_LOOKUP_ENABLED =
CONFIG_PREFIX + "host-verification.reverse-dns.enabled";
private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
public static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
public static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
public static final String TLS_CERT_RELOAD = CONFIG_PREFIX + "certReload";
public static final String TLS_USE_OPENSSL = CONFIG_PREFIX + "useOpenSsl";
Comment on lines +91 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you make these public? For consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, IMO we should keep most config constants public. Even for an IA.Private class, if a user wants to pull in the config constants they should be able to. They open themselves up to risk of compatibility issues since it's still an IA.Private class, but imo when it comes to config constants it is less risky to use a constant than it is to hardcode a string that may change. Ideally we could refactor our config constant approach across the board to make them easier to integrate, but barring that I've been making consistency changes like this whenever I see them.

public static final String DEFAULT_PROTOCOL = "TLSv1.2";

//
Expand Down Expand Up @@ -131,6 +137,34 @@ private static String[] getCBCCiphers() {
private static final String[] DEFAULT_CIPHERS_JAVA9 =
ObjectArrays.concat(getGCMCiphers(), getCBCCiphers(), String.class);

private static final String[] DEFAULT_CIPHERS_OPENSSL = getOpenSslFilteredDefaultCiphers();

/**
* Not all of our default ciphers are available in OpenSSL. Takes our default cipher lists and
* filters them to only those available in OpenSsl. Does GCM first, then CBC because GCM tends to
* be better and faster, and we don't need to worry about the java8 vs 9 performance issue if
* OpenSSL is handling it.
*/
Comment on lines +142 to +147
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice approach. In ZooKeeper we simply don't set default cipher suites just let OpenSSL to decide. User can still override them though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I went with this approach because I'm really not an SSL expert and dont have any opinions about the cipher suites used here or what's in openssl as defaults. So I figured I could make a least-invasive change which uses what was defaulted here an just filters to those that work.

private static String[] getOpenSslFilteredDefaultCiphers() {
if (!OpenSsl.isAvailable()) {
return new String[0];
}

Set<String> openSslSuites = OpenSsl.availableJavaCipherSuites();
List<String> defaultSuites = new ArrayList<>();
for (String cipher : getGCMCiphers()) {
if (openSslSuites.contains(cipher)) {
defaultSuites.add(cipher);
}
}
for (String cipher : getCBCCiphers()) {
if (openSslSuites.contains(cipher)) {
defaultSuites.add(cipher);
}
}
return defaultSuites.toArray(new String[0]);
}

/**
* Enum specifying the client auth requirement of server-side TLS sockets created by this
* X509Util.
Expand Down Expand Up @@ -176,7 +210,10 @@ private X509Util() {
// disabled
}

static String[] getDefaultCipherSuites() {
static String[] getDefaultCipherSuites(boolean useOpenSsl) {
if (useOpenSsl) {
return DEFAULT_CIPHERS_OPENSSL;
}
return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
}

Expand All @@ -202,6 +239,7 @@ public static SslContext createSslContextForClient(Configuration config)

SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();

boolean useOpenSsl = configureOpenSslIfAvailable(sslContextBuilder, config);
String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
char[] keyStorePassword = config.getPassword(TLS_CONFIG_KEYSTORE_PASSWORD);
String keyStoreType = config.get(TLS_CONFIG_KEYSTORE_TYPE, "");
Expand Down Expand Up @@ -234,11 +272,33 @@ public static SslContext createSslContextForClient(Configuration config)

sslContextBuilder.enableOcsp(sslOcspEnabled);
sslContextBuilder.protocols(getEnabledProtocols(config));
sslContextBuilder.ciphers(Arrays.asList(getCipherSuites(config)));
sslContextBuilder.ciphers(Arrays.asList(getCipherSuites(config, useOpenSsl)));

return sslContextBuilder.build();
}

/**
* Adds SslProvider.OPENSSL if OpenSsl is available and enabled. In order to make it available,
* one must ensure that a properly shaded netty-tcnative is on the classpath. Properly shaded
* means relocated to be prefixed with "org.apache.hbase.thirdparty" like the rest of the netty
* classes.
*/
private static boolean configureOpenSslIfAvailable(SslContextBuilder sslContextBuilder,
Configuration conf) {
if (OpenSsl.isAvailable() && conf.getBoolean(TLS_USE_OPENSSL, true)) {
LOG.debug("Using netty-tcnative to accelerate SSL handling");
sslContextBuilder.sslProvider(SslProvider.OPENSSL);
return true;
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("Using default JDK SSL provider because netty-tcnative is not {}",
OpenSsl.isAvailable() ? "enabled" : "available");
}
sslContextBuilder.sslProvider(SslProvider.JDK);
bbeaudreault marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}

public static SslContext createSslContextForServer(Configuration config)
throws X509Exception, IOException {
String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
Expand All @@ -254,6 +314,7 @@ public static SslContext createSslContextForServer(Configuration config)
sslContextBuilder = SslContextBuilder
.forServer(createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType));

boolean useOpenSsl = configureOpenSslIfAvailable(sslContextBuilder, config);
String trustStoreLocation = config.get(TLS_CONFIG_TRUSTSTORE_LOCATION, "");
char[] trustStorePassword = config.getPassword(TLS_CONFIG_TRUSTSTORE_PASSWORD);
String trustStoreType = config.get(TLS_CONFIG_TRUSTSTORE_TYPE, "");
Expand All @@ -277,7 +338,7 @@ public static SslContext createSslContextForServer(Configuration config)

sslContextBuilder.enableOcsp(sslOcspEnabled);
sslContextBuilder.protocols(getEnabledProtocols(config));
sslContextBuilder.ciphers(Arrays.asList(getCipherSuites(config)));
sslContextBuilder.ciphers(Arrays.asList(getCipherSuites(config, useOpenSsl)));
sslContextBuilder.clientAuth(clientAuth.toNettyClientAuth());

return sslContextBuilder.build();
Expand Down Expand Up @@ -393,10 +454,10 @@ private static String[] getEnabledProtocols(Configuration config) {
return enabledProtocolsInput.split(",");
}

private static String[] getCipherSuites(Configuration config) {
private static String[] getCipherSuites(Configuration config, boolean useOpenSsl) {
String cipherSuitesInput = config.get(TLS_CIPHER_SUITES);
if (cipherSuitesInput == null) {
return getDefaultCipherSuites();
return getDefaultCipherSuites(useOpenSsl);
} else {
return cipherSuitesInput.split(",");
}
Expand Down