andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Correctly detect if protocol is enabled when using netty-tcnative (#7940)

Browse Source 
Motivation:

We sometimes did not correctly detect when a protocol is not enabled when using netty-tcnative as we did not take into account when the option flag is 0 (as for example in BoringSSL for SSLv2).

Modifications:

- Correctly take an option flag with 0 into account.
- Add unit tests.

Result:

Fixes https://github.com/netty/netty/issues/7935.
This commit is contained in:
Norman Maurer 2018-05-16 07:23:55 +02:00 committed by GitHub
parent ed54ab034d
commit 69c644bb98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
handler/src/main/java/io/netty/handler/ssl/OpenSsl.java
View File

@ -217,19 +217,19 @@ public final class OpenSsl {
Set<String> protocols = new LinkedHashSet<String>(6);
// Seems like there is no way to explicitly disable SSLv2Hello in openssl so it is always enabled
protocols.add(PROTOCOL_SSL_V2_HELLO);
if (doesSupportProtocol(SSL.SSL_PROTOCOL_SSLV2)) {
if (doesSupportProtocol(SSL.SSL_PROTOCOL_SSLV2, SSL.SSL_OP_NO_SSLv2)) {
protocols.add(PROTOCOL_SSL_V2);
}
if (doesSupportProtocol(SSL.SSL_PROTOCOL_SSLV3)) {
if (doesSupportProtocol(SSL.SSL_PROTOCOL_SSLV3, SSL.SSL_OP_NO_SSLv3)) {
protocols.add(PROTOCOL_SSL_V3);
}
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1)) {
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1, SSL.SSL_OP_NO_TLSv1)) {
protocols.add(PROTOCOL_TLS_V1);
}
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_1)) {
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_1, SSL.SSL_OP_NO_TLSv1_1)) {
protocols.add(PROTOCOL_TLS_V1_1);
}
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_2)) {
if (doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_2, SSL.SSL_OP_NO_TLSv1_2)) {
protocols.add(PROTOCOL_TLS_V1_2);
}
@ -237,7 +237,7 @@ public final class OpenSsl {
SUPPORTS_OCSP = doesSupportOcsp();
if (logger.isDebugEnabled()) {
logger.debug("Supported protocols (OpenSSL): {} ", Arrays.asList(SUPPORTED_PROTOCOLS_SET));
logger.debug("Supported protocols (OpenSSL): {} ", SUPPORTED_PROTOCOLS_SET);
logger.debug("Default cipher suites (OpenSSL): {}", DEFAULT_CIPHERS);
}
} else {
@ -271,7 +271,11 @@ public final class OpenSsl {
}
return supportsOcsp;
}
private static boolean doesSupportProtocol(int protocol) {
private static boolean doesSupportProtocol(int protocol, int opt) {
if (opt == 0) {
// If the opt is 0 the protocol is not supported. This is for example the case with BoringSSL and SSLv2.
return false;
}
long sslCtx = -1;
try {
sslCtx = SSLContext.make(protocol, SSL.SSL_MODE_COMBINED);

50
handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
View File

@ -44,6 +44,7 @@ import io.netty.util.internal.EmptyArrays;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.StringUtil;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
@ -62,7 +63,9 @@ import java.security.Provider;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@ -76,7 +79,11 @@ import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSession;
import javax.security.cert.X509Certificate;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V2;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V2_HELLO;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V3;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_1;
import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2;
import static io.netty.handler.ssl.SslUtils.SSL_RECORD_HEADER_LENGTH;
@ -2355,4 +2362,47 @@ public abstract class SSLEngineTest {
cert.delete();
}
}
@Test
public void testDisableProtocols() throws Exception {
testDisableProtocols(PROTOCOL_SSL_V2, PROTOCOL_SSL_V2);
testDisableProtocols(PROTOCOL_SSL_V3, PROTOCOL_SSL_V2, PROTOCOL_SSL_V3);
testDisableProtocols(PROTOCOL_TLS_V1, PROTOCOL_SSL_V2, PROTOCOL_SSL_V3, PROTOCOL_TLS_V1);
testDisableProtocols(PROTOCOL_TLS_V1_1, PROTOCOL_SSL_V2, PROTOCOL_SSL_V3, PROTOCOL_TLS_V1, PROTOCOL_TLS_V1_1);
testDisableProtocols(PROTOCOL_TLS_V1_2, PROTOCOL_SSL_V2,
PROTOCOL_SSL_V3, PROTOCOL_TLS_V1, PROTOCOL_TLS_V1_1, PROTOCOL_TLS_V1_2);
}
private void testDisableProtocols(String protocol, String... disabledProtocols) throws Exception {
SelfSignedCertificate cert = new SelfSignedCertificate();
SslContext ctx = SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslProvider(sslServerProvider())
.build();
SSLEngine server = ctx.newEngine(UnpooledByteBufAllocator.DEFAULT);
try {
Set<String> supported = new HashSet<String>(Arrays.asList(server.getSupportedProtocols()));
if (supported.contains(protocol)) {
server.setEnabledProtocols(server.getSupportedProtocols());
Assert.assertEquals(supported, new HashSet<String>(Arrays.asList(server.getSupportedProtocols())));
for (String disabled: disabledProtocols) {
supported.remove(disabled);
}
if (supported.contains(SslUtils.PROTOCOL_SSL_V2_HELLO) && supported.size() == 1) {
// It's not allowed to set only PROTOCOL_SSL_V2_HELLO if using JDK SSLEngine.
return;
}
server.setEnabledProtocols(supported.toArray(new String[0]));
Assert.assertEquals(supported, new HashSet<String>(Arrays.asList(server.getEnabledProtocols())));
server.setEnabledProtocols(server.getSupportedProtocols());
}
} finally {
cleanupServerSslEngine(server);
cleanupClientSslContext(ctx);
cert.delete();
}
}
}