Skip to content

Commit

Permalink
Only suppert TLSv1.3 when JDK does support it as well (#11604)
Browse files Browse the repository at this point in the history
Motivation:

We tightly integrate the TrustManger and KeyManager into our native SSL implementation which means that both of them need to support TLSv1.3 as protocol. This is not always the case and so can produce runtime exceptions.

As TLSv1.3 support was backported to Java8 quite some time now we should be a bit more conservative and only enable TLSv1.3 for our native implementation if the JDK implementation supports it as well. This also allows us to remove some hacks we had in place to be able to support it before in Java8.

Modifications:

- Only enable TLSv1.3 support for our native SSL implementation when the JDK supports it as well
- Remove OpenSslTlsv13X509ExtendedTrustManager as its not needed anymore

Result:

Fixes #11589
  • Loading branch information
normanmaurer committed Aug 20, 2021
1 parent 22a188c commit fd47554
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 258 deletions.
36 changes: 20 additions & 16 deletions handler/src/main/java/io/netty/handler/ssl/OpenSsl.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,25 +206,29 @@ public final class OpenSsl {
long cert = 0;
long key = 0;
try {
try {
StringBuilder tlsv13Ciphers = new StringBuilder();
// As we delegate to the KeyManager / TrustManager of the JDK we need to ensure it can actually
// handle TLSv13 as otherwise we may see runtime exceptions
if (SslProvider.isTlsv13Supported(SslProvider.JDK)) {
try {
StringBuilder tlsv13Ciphers = new StringBuilder();

for (String cipher: TLSV13_CIPHERS) {
String converted = CipherSuiteConverter.toOpenSsl(cipher, IS_BORINGSSL);
if (converted != null) {
tlsv13Ciphers.append(converted).append(':');
for (String cipher : TLSV13_CIPHERS) {
String converted = CipherSuiteConverter.toOpenSsl(cipher, IS_BORINGSSL);
if (converted != null) {
tlsv13Ciphers.append(converted).append(':');
}
}
}
if (tlsv13Ciphers.length() == 0) {
if (tlsv13Ciphers.length() == 0) {
tlsv13Supported = false;
} else {
tlsv13Ciphers.setLength(tlsv13Ciphers.length() - 1);
SSLContext.setCipherSuite(sslCtx, tlsv13Ciphers.toString(), true);
tlsv13Supported = true;
}

} catch (Exception ignore) {
tlsv13Supported = false;
} else {
tlsv13Ciphers.setLength(tlsv13Ciphers.length() - 1);
SSLContext.setCipherSuite(sslCtx, tlsv13Ciphers.toString() , true);
tlsv13Supported = true;
}

} catch (Exception ignore) {
tlsv13Supported = false;
}

SSLContext.setCipherSuite(sslCtx, "ALL", false);
Expand Down Expand Up @@ -367,7 +371,7 @@ public final class OpenSsl {
protocols.add(SslProtocols.TLS_v1_2);
}

// This is only supported by java11 and later.
// This is only supported by java8u272 and later.
if (tlsv13Supported && doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_3, SSL.SSL_OP_NO_TLSv1_3)) {
protocols.add(SslProtocols.TLS_v1_3);
TLSV13_SUPPORTED = true;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private static final class ExtendedTrustManagerVerifyCallback extends AbstractCe

ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) {
super(engineMap);
this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager);
this.manager = manager;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private static final class ExtendedTrustManagerVerifyCallback extends AbstractCe

ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) {
super(engineMap);
this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager);
this.manager = manager;
}

@Override
Expand Down

0 comments on commit fd47554

Please sign in to comment.