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.

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

Respect jdk.tls.namedGroups when using native SSL implementation #11660

Merged
merged 6 commits into from Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 42 additions & 0 deletions handler/src/main/java/io/netty/handler/ssl/OpenSsl.java
Expand Up @@ -38,6 +38,7 @@
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -67,6 +68,11 @@ public final class OpenSsl {
static final Set<String> SUPPORTED_PROTOCOLS_SET;
static final String[] EXTRA_SUPPORTED_TLS_1_3_CIPHERS;
static final String EXTRA_SUPPORTED_TLS_1_3_CIPHERS_STRING;
static final String[] NAMED_GROUPS;

// Use default that is supported in java 11 and earlier.
// See https://github.com/netty/netty-tcnative/issues/567
private static final String[] DEFAULT_NAMED_GROUPS = { "P-256", "P-384", "X25519" };
normanmaurer marked this conversation as resolved.
Show resolved Hide resolved

// self-signed certificate for netty.io and the matching private-key
private static final String CERT = "-----BEGIN CERTIFICATE-----\n" +
Expand Down Expand Up @@ -181,6 +187,7 @@ public final class OpenSsl {
boolean supportsKeyManagerFactory = false;
boolean useKeyManagerFactory = false;
boolean tlsv13Supported = false;
String[] namedGroups = DEFAULT_NAMED_GROUPS;

IS_BORINGSSL = "BoringSSL".equals(versionString());
if (IS_BORINGSSL) {
Expand Down Expand Up @@ -313,12 +320,46 @@ public final class OpenSsl {
SSL.freePrivateKey(key);
}
}

String groups = SystemPropertyUtil.get("jdk.tls.namedGroups", null);
if (groups != null) {
String[] nGroups = groups.split(",");
Set<String> supportedNamedGroups = new LinkedHashSet<String>(nGroups.length);
Set<String> unsupportedNamedGroups = new LinkedHashSet<String>();
for (String namedGroup : nGroups) {
if (SSLContext.setCurvesList(sslCtx, namedGroup)) {
chrisvest marked this conversation as resolved.
Show resolved Hide resolved
supportedNamedGroups.add(namedGroup);
} else {
unsupportedNamedGroups.add(namedGroup);
}
}

if (supportedNamedGroups.isEmpty()) {
namedGroups = DEFAULT_NAMED_GROUPS;
logger.info("All configured namedGroups are not supported: {}. Use default: .",
normanmaurer marked this conversation as resolved.
Show resolved Hide resolved
Arrays.toString(unsupportedNamedGroups.toArray(EmptyArrays.EMPTY_STRINGS)),
Arrays.toString(DEFAULT_NAMED_GROUPS));
} else {
namedGroups = supportedNamedGroups.toArray(EmptyArrays.EMPTY_STRINGS);
if (unsupportedNamedGroups.isEmpty()) {
logger.info("Use configured namedGroups -D 'jdk.tls.namedGroup': {} ",
normanmaurer marked this conversation as resolved.
Show resolved Hide resolved
Arrays.toString(namedGroups));
} else {
logger.info("Use supported configured namedGroups: {}. Unsupported namedGroups: {}. ",
normanmaurer marked this conversation as resolved.
Show resolved Hide resolved
Arrays.toString(namedGroups),
Arrays.toString(unsupportedNamedGroups.toArray(EmptyArrays.EMPTY_STRINGS)));
}
}
} else {
namedGroups = DEFAULT_NAMED_GROUPS;
}
} finally {
SSLContext.free(sslCtx);
}
} catch (Exception e) {
logger.warn("Failed to get the list of available OpenSSL cipher suites.", e);
}
NAMED_GROUPS = namedGroups;
AVAILABLE_OPENSSL_CIPHER_SUITES = Collections.unmodifiableSet(availableOpenSslCipherSuites);
final Set<String> availableJavaCipherSuites = new LinkedHashSet<String>(
AVAILABLE_OPENSSL_CIPHER_SUITES.size() * 2);
Expand Down Expand Up @@ -399,6 +440,7 @@ public final class OpenSsl {
IS_BORINGSSL = false;
EXTRA_SUPPORTED_TLS_1_3_CIPHERS = EmptyArrays.EMPTY_STRINGS;
EXTRA_SUPPORTED_TLS_1_3_CIPHERS_STRING = StringUtil.EMPTY_STRING;
NAMED_GROUPS = DEFAULT_NAMED_GROUPS;
}
}

Expand Down
Expand Up @@ -117,6 +117,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
// https://mail.openjdk.java.net/pipermail/security-dev/2021-March/024758.html
static final boolean CLIENT_ENABLE_SESSION_CACHE =
SystemPropertyUtil.getBoolean("io.netty.handler.ssl.openssl.sessionCacheClient", false);

/**
* The OpenSSL SSL_CTX object.
*
Expand Down Expand Up @@ -384,6 +385,8 @@ public ApplicationProtocolConfig.SelectedListenerFailureBehavior selectedListene
if (asyncPrivateKeyMethod != null) {
SSLContext.setPrivateKeyMethod(ctx, new AsyncPrivateKeyMethod(engineMap, asyncPrivateKeyMethod));
}
// Set the curves.
SSLContext.setCurvesList(ctx, OpenSsl.NAMED_GROUPS);
success = true;
} finally {
if (!success) {
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -508,7 +508,7 @@
<!-- keep in sync with PlatformDependent#ALLOWED_LINUX_OS_CLASSIFIERS -->
<os.detection.classifierWithLikes>fedora,suse,arch</os.detection.classifierWithLikes>
<tcnative.artifactId>netty-tcnative</tcnative.artifactId>
<tcnative.version>2.0.41.Final</tcnative.version>
<tcnative.version>2.0.42.Final-SNAPSHOT</tcnative.version>
<tcnative.classifier>${os.detected.classifier}</tcnative.classifier>
<conscrypt.groupId>org.conscrypt</conscrypt.groupId>
<conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>
Expand Down