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

OpenSslEngine protocol selection must be contiguous

Browse Source 
Motivation:
TLS doesn't support a way to advertise non-contiguous versions from the client's perspective, and the client just advertises the max supported version. The TLS protocol also doesn't support all different combinations of discrete protocols, and instead assumes contiguous ranges. OpenSSL has some unexpected behavior (e.g. handshake failures) if non-contiguous protocols are used even where there is a compatible set of protocols and ciphers. For these reasons this method will determine the minimum protocol and the maximum protocol and enabled a contiguous range from [min protocol, max protocol] in OpenSSL.

Modifications:
- ReferenceCountedOpenSslEngine#setEnabledProtocols should determine the min/max protocol versions and enable a contiguous range

Result:
OpenSslEngine is more consistent with the JDK's SslEngineImpl and no more unexpected handshake failures due to protocol selection quirks.
This commit is contained in:
Scott Mitchell 2017-07-12 09:46:26 -07:00
parent f57d8a1509
commit 5fd6c9d5df
2 changed files with 92 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
View File

@ -93,6 +93,18 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
new SSLException("renegotiation unsupported"), ReferenceCountedOpenSslEngine.class, "beginHandshake()");
private static final ResourceLeakDetector<ReferenceCountedOpenSslEngine> leakDetector =
ResourceLeakDetectorFactory.instance().newResourceLeakDetector(ReferenceCountedOpenSslEngine.class);
private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV2 = 0;
private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV3 = 1;
private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1 = 2;
private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1 = 3;
private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2 = 4;
private static final int[] OPENSSL_OP_NO_PROTOCOLS = new int[] {
SSL.SSL_OP_NO_SSLv2,
SSL.SSL_OP_NO_SSLv3,
SSL.SSL_OP_NO_TLSv1,
SSL.SSL_OP_NO_TLSv1_1,
SSL.SSL_OP_NO_TLSv1_2
};
/**
* <a href="https://www.openssl.org/docs/man1.0.2/crypto/X509_check_host.html">The flags argument is usually 0</a>.
*/
@ -1342,54 +1354,77 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return (opts & disableMask) == 0 && OpenSsl.SUPPORTED_PROTOCOLS_SET.contains(protocolString);
}
/**
* {@inheritDoc}
* TLS doesn't support a way to advertise non-contiguous versions from the client's perspective, and the client
* just advertises the max supported version. The TLS protocol also doesn't support all different combinations of
* discrete protocols, and instead assumes contiguous ranges. OpenSSL has some unexpected behavior
* (e.g. handshake failures) if non-contiguous protocols are used even where there is a compatible set of protocols
* and ciphers. For these reasons this method will determine the minimum protocol and the maximum protocol and
* enabled a contiguous range from [min protocol, max protocol] in OpenSSL.
*/
@Override
public final void setEnabledProtocols(String[] protocols) {
if (protocols == null) {
// This is correct from the API docs
throw new IllegalArgumentException();
}
boolean sslv2 = false;
boolean sslv3 = false;
boolean tlsv1 = false;
boolean tlsv1_1 = false;
boolean tlsv1_2 = false;
int minProtocolIndex = OPENSSL_OP_NO_PROTOCOLS.length;
int maxProtocolIndex = 0;
for (String p: protocols) {
if (!OpenSsl.SUPPORTED_PROTOCOLS_SET.contains(p)) {
throw new IllegalArgumentException("Protocol " + p + " is not supported.");
}
if (p.equals(OpenSsl.PROTOCOL_SSL_V2)) {
sslv2 = true;
if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV2) {
minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV2;
}
if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV2) {
maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV2;
}
} else if (p.equals(OpenSsl.PROTOCOL_SSL_V3)) {
sslv3 = true;
if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV3) {
minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV3;
}
if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV3) {
maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_SSLV3;
}
} else if (p.equals(OpenSsl.PROTOCOL_TLS_V1)) {
tlsv1 = true;
if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1) {
minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1;
}
if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1) {
maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1;
}
} else if (p.equals(OpenSsl.PROTOCOL_TLS_V1_1)) {
tlsv1_1 = true;
if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1) {
minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1;
}
if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1) {
maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1;
}
} else if (p.equals(OpenSsl.PROTOCOL_TLS_V1_2)) {
tlsv1_2 = true;
if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2) {
minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2;
}
if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2) {
maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2;
}
}
}
synchronized (this) {
if (!isDestroyed()) {
// Clear out options which disable protocols
SSL.clearOptions(ssl, SSL.SSL_OP_NO_SSLv2 | SSL.SSL_OP_NO_SSLv3 | SSL.SSL_OP_NO_TLSv1 |
SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2);
SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2);
int opts = 0;
if (!sslv2) {
opts |= SSL.SSL_OP_NO_SSLv2;
for (int i = 0; i < minProtocolIndex; ++i) {
opts |= OPENSSL_OP_NO_PROTOCOLS[i];
}
if (!sslv3) {
opts |= SSL.SSL_OP_NO_SSLv3;
}
if (!tlsv1) {
opts |= SSL.SSL_OP_NO_TLSv1;
}
if (!tlsv1_1) {
opts |= SSL.SSL_OP_NO_TLSv1_1;
}
if (!tlsv1_2) {
opts |= SSL.SSL_OP_NO_TLSv1_2;
assert maxProtocolIndex != MAX_VALUE;
for (int i = maxProtocolIndex + 1; i < OPENSSL_OP_NO_PROTOCOLS.length; ++i) {
opts |= OPENSSL_OP_NO_PROTOCOLS[i];
}
// Disable protocols we do not want

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

@ -57,13 +57,15 @@ import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.security.KeyStore;
import java.security.Provider;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.Provider;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
@ -1675,6 +1677,36 @@ public abstract class SSLEngineTest {
}
}
@Test
public void testHandshakeCompletesWithNonContiguousProtocolsTLSv1_2CipherOnly() throws Exception {
SelfSignedCertificate ssc = new SelfSignedCertificate();
// Select a mandatory cipher from the TLSv1.2 RFC https://www.ietf.org/rfc/rfc5246.txt so handshakes won't fail
// due to no shared/supported cipher.
final String sharedCipher = "TLS_RSA_WITH_AES_128_CBC_SHA";
clientSslCtx = SslContextBuilder.forClient()
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.ciphers(Arrays.asList(sharedCipher))
.protocols(OpenSsl.PROTOCOL_TLS_V1_2, OpenSsl.PROTOCOL_TLS_V1)
.sslProvider(sslClientProvider())
.build();
serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
.ciphers(Arrays.asList(sharedCipher))
.protocols(OpenSsl.PROTOCOL_TLS_V1_2, OpenSsl.PROTOCOL_TLS_V1)
.sslProvider(sslServerProvider())
.build();
SSLEngine clientEngine = null;
SSLEngine serverEngine = null;
try {
clientEngine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
serverEngine = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
handshake(clientEngine, serverEngine);
} finally {
cleanupClientSslEngine(clientEngine);
cleanupServerSslEngine(serverEngine);
}
}
@Test
public void testPacketBufferSizeLimit() throws Exception {
SelfSignedCertificate cert = new SelfSignedCertificate();