From 91f050d2ef6b22b7aec187aa1cf4593955dcea82 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 24 Jan 2017 11:49:10 +0100 Subject: [PATCH] More precise calculate the maximum record size when using SslProvider.OPENSSL* and so decrease mem usage. Motivation: We used ca 2k as maximum overhead for encrypted packets which is a lot more then what is needed in reality by OpenSSL. This could lead to the need of more memory. Modification: - Use a lower overhead of 86 bytes as defined by the spec and openssl itself - Fix unit test to use the correct session to calculate needed buffer size Result: Less memory usage. --- .../ssl/ReferenceCountedOpenSslEngine.java | 22 +- .../java/io/netty/handler/ssl/SslHandler.java | 5 +- .../netty/handler/ssl/OpenSslEngineTest.java | 220 ++++++++++++++++++ .../io/netty/handler/ssl/SSLEngineTest.java | 2 +- 4 files changed, 241 insertions(+), 8 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index a9af794e53..1178a55083 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -151,14 +151,24 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc SET_SERVER_NAMES_METHOD = setServerNamesMethod; } - private static final int MAX_PLAINTEXT_LENGTH = 16 * 1024; // 2^14 - private static final int MAX_COMPRESSED_LENGTH = MAX_PLAINTEXT_LENGTH + 1024; - private static final int MAX_CIPHERTEXT_LENGTH = MAX_COMPRESSED_LENGTH + 1024; + static final int MAX_PLAINTEXT_LENGTH = 16 * 1024; // 2^14 - // Header (5) + Data (2^14) + Compression (1024) + Encryption (1024) + MAC (20) + Padding (256) - static final int MAX_ENCRYPTED_PACKET_LENGTH = MAX_CIPHERTEXT_LENGTH + 5 + 20 + 256; + /** + * This is the maximum overhead when encrypting plaintext as defined by + * rfc5264, + * rfc5289 and openssl implementation itself. + * + * Please note that we use a padding of 16 here as openssl uses PKC#5 which uses 16 bytes while the spec itself + * allow up to 255 bytes. 16 bytes is the max for PKC#5 (which handles it the same way as PKC#7) as we use a block + * size of 16. See rfc5652#section-6.3. + * + * 16 (IV) + 48 (MAC) + 1 (Padding_length field) + 15 (Padding) + 1 (ContentType) + 2 (ProtocolVersion) + 2 (Length) + * + * TODO: We may need to review this calculation once TLS 1.3 becomes available. + */ + static final int MAX_ENCRYPTION_OVERHEAD_LENGTH = 15 + 48 + 1 + 16 + 1 + 2 + 2; - static final int MAX_ENCRYPTION_OVERHEAD_LENGTH = MAX_ENCRYPTED_PACKET_LENGTH - MAX_PLAINTEXT_LENGTH; + static final int MAX_ENCRYPTED_PACKET_LENGTH = MAX_PLAINTEXT_LENGTH + MAX_ENCRYPTION_OVERHEAD_LENGTH; private static final int MAX_ENCRYPTION_OVERHEAD_DIFF = Integer.MAX_VALUE - MAX_ENCRYPTION_OVERHEAD_LENGTH; diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 83503c8593..8e920c285f 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -734,7 +734,10 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH // See https://github.com/netty/netty/issues/5860 while (!ctx.isRemoved()) { if (out == null) { - out = allocateOutNetBuf(ctx, 0); + // As this is called for the handshake we have no real idea how big the buffer needs to be. + // That said 2048 should give us enough room to include everything like ALPN / NPN data. + // If this is not enough we will increase the buffer in wrap(...). + out = allocateOutNetBuf(ctx, 2048); } SSLEngineResult result = wrap(alloc, engine, Unpooled.EMPTY_BUFFER, out); diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java index 986b20a37b..1d05039369 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -27,12 +27,17 @@ import org.junit.Test; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -42,6 +47,151 @@ public class OpenSslEngineTest extends SSLEngineTest { private static final String PREFERRED_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http2"; private static final String FALLBACK_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http1_1"; + private static final Set TLS_V1_1_CIPHERS = new HashSet(Arrays.asList( + "ECDHE-RSA-AES256-SHA", + "DHE-RSA-AES256-SHA", + "DHE-RSA-CAMELLIA256-SHA", + "AECDH-AES256-SHA", + "ADH-AES256-SHA", + "ADH-CAMELLIA256-SHA", + "AES256-SHA", + "CAMELLIA256-SHA", + "ECDHE-RSA-AES128-SHA", + "DHE-RSA-AES128-SHA", + "DHE-RSA-SEED-SHA", + "DHE-RSA-CAMELLIA128-SHA", + "AECDH-AES128-SHA", + "ADH-AES128-SHA", + "ADH-SEED-SHA", + "ADH-CAMELLIA128-SHA", + "AES128-SHA", + "SEED-SHA", + "CAMELLIA128-SHA", + "IDEA-CBC-SHA", + "ECDHE-RSA-RC4-SHA", + "AECDH-RC4-SHA", + "ADH-RC4-MD5", + "RC4-SHA", + "RC4-MD5", + "ECDHE-RSA-DES-CBC3-SHA", + "EDH-RSA-DES-CBC3-SHA", + "AECDH-DES-CBC3-SHA", + "ADH-DES-CBC3-SHA", + "DES-CBC3-SHA" + )); + + private static final Set TLS_V1_2_CIPHERS = new HashSet(Arrays.asList( + "ECDHE-RSA-AES256-GCM-SHA384", + "ECDHE-RSA-AES256-SHA384", + "ECDHE-RSA-AES256-SHA", + "DHE-RSA-AES256-GCM-SHA384", + "DHE-RSA-AES256-SHA256", + "DHE-RSA-AES256-SHA", + "DHE-RSA-CAMELLIA256-SHA", + "AECDH-AES256-SHA", + "ADH-AES256-GCM-SHA384", + "ADH-AES256-SHA256", + "ADH-AES256-SHA", + "ADH-CAMELLIA256-SHA", + "AES256-GCM-SHA384", + "AES256-SHA256", + "AES256-SHA", + "CAMELLIA256-SHA", + "ECDHE-RSA-AES128-GCM-SHA256", + "ECDHE-RSA-AES128-SHA256", + "ECDHE-RSA-AES128-SHA", + "DHE-RSA-AES128-GCM-SHA256", + "DHE-RSA-AES128-SHA256", + "DHE-RSA-AES128-SHA", + "DHE-RSA-SEED-SHA", + "DHE-RSA-CAMELLIA128-SHA", + "AECDH-AES128-SHA", + "ADH-AES128-GCM-SHA256", + "ADH-AES128-SHA256", + "ADH-AES128-SHA", + "ADH-SEED-SHA", + "ADH-CAMELLIA128-SHA", + "AES128-GCM-SHA256", + "AES128-SHA256", + "AES128-SHA", + "SEED-SHA", + "CAMELLIA128-SHA", + "IDEA-CBC-SHA", + "ECDHE-RSA-RC4-SHA", + "AECDH-RC4-SHA", + "ADH-RC4-MD5", + "RC4-SHA", "RC4-MD5", + "ECDHE-RSA-DES-CBC3-SHA", + "EDH-RSA-DES-CBC3-SHA", + "AECDH-DES-CBC3-SHA", + "ADH-DES-CBC3-SHA", + "DES-CBC3-SHA" + )); + + private static final Set TLS_V1_CIPHERS = new HashSet(Arrays.asList( + "ECDHE-RSA-AES256-SHA", + "DHE-RSA-AES256-SHA", + "DHE-RSA-CAMELLIA256-SHA", + "AECDH-AES256-SHA", + "ADH-AES256-SHA", + "ADH-CAMELLIA256-SHA", + "AES256-SHA", + "CAMELLIA256-SHA", + "ECDHE-RSA-AES128-SHA", + "DHE-RSA-AES128-SHA", + "DHE-RSA-SEED-SHA", + "DHE-RSA-CAMELLIA128-SHA", + "AECDH-AES128-SHA", + "ADH-AES128-SHA", + "ADH-SEED-SHA", + "ADH-CAMELLIA128-SHA", + "AES128-SHA", + "SEED-SHA", + "CAMELLIA128-SHA", + "IDEA-CBC-SHA", + "ECDHE-RSA-RC4-SHA", + "AECDH-RC4-SHA", + "ADH-RC4-MD5", + "RC4-SHA", + "RC4-MD5", + "ECDHE-RSA-DES-CBC3-SHA", + "EDH-RSA-DES-CBC3-SHA", + "AECDH-DES-CBC3-SHA", + "ADH-DES-CBC3-SHA", + "DES-CBC3-SHA" + )); + + private static final Set SSL_V3_CIPHERS = new HashSet(Arrays.asList( + "ADH-AES128-SHA", + "AES128-SHA", + "ADH-CAMELLIA128-SHA", + "DES-CBC3-SHA", + "AECDH-AES128-SHA", + "AECDH-DES-CBC3-SHA", + "CAMELLIA128-SHA", + "DHE-RSA-AES256-SHA", + "SEED-SHA", + "RC4-MD5", + "ADH-AES256-SHA", + "AES256-SHA", + "ADH-SEED-SHA", + "ADH-DES-CBC3-SHA", + "EDH-RSA-DES-CBC3-SHA", + "ADH-RC4-MD5", + "IDEA-CBC-SHA", + "DHE-RSA-AES128-SHA", + "RC4-SHA", + "CAMELLIA256-SHA", + "AECDH-RC4-SHA", + "DHE-RSA-SEED-SHA", + "AECDH-AES256-SHA", + "ECDHE-RSA-DES-CBC3-SHA", + "ADH-CAMELLIA256-SHA", + "DHE-RSA-CAMELLIA256-SHA", + "DHE-RSA-CAMELLIA128-SHA", + "ECDHE-RSA-RC4-SHA" + )); + public OpenSslEngineTest(BufferType type) { super(type); } @@ -259,6 +409,76 @@ public class OpenSslEngineTest extends SSLEngineTest { ReferenceCountedOpenSslEngine.calculateOutNetBufSize(Integer.MAX_VALUE)); } + @Test + public void testCalculateOutNetBufSize0() { + assertEquals(ReferenceCountedOpenSslEngine.MAX_ENCRYPTION_OVERHEAD_LENGTH, + ReferenceCountedOpenSslEngine.calculateOutNetBufSize(0)); + } + + @Test + public void testWrapWithDifferentSizes() throws Exception { + clientSslCtx = SslContextBuilder.forClient() + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .build(); + SelfSignedCertificate ssc = new SelfSignedCertificate(); + serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(sslServerProvider()) + .build(); + // SSLv2 is not supported on most openssl installations so skip it. + testWrapWithDifferentSizes(OpenSsl.PROTOCOL_TLS_V1, TLS_V1_CIPHERS); + testWrapWithDifferentSizes(OpenSsl.PROTOCOL_TLS_V1_1, TLS_V1_1_CIPHERS); + testWrapWithDifferentSizes(OpenSsl.PROTOCOL_TLS_V1_2, TLS_V1_2_CIPHERS); + testWrapWithDifferentSizes(OpenSsl.PROTOCOL_SSL_V3, SSL_V3_CIPHERS); + } + + private void testWrapWithDifferentSizes(String protocol, Set ciphers) throws Exception { + for (String cipher : ciphers) { + if (!OpenSsl.isCipherSuiteAvailable(cipher)) { + continue; + } + SSLEngine clientEngine = null; + SSLEngine serverEngine = null; + try { + clientEngine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); + serverEngine = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); + clientEngine.setEnabledCipherSuites(new String[] { cipher }); + clientEngine.setEnabledProtocols(new String[] { protocol }); + serverEngine.setEnabledCipherSuites(new String[] { cipher }); + serverEngine.setEnabledProtocols(new String[] { protocol }); + + handshake(clientEngine, serverEngine); + + int srcLen = 64; + do { + testWrapDstBigEnough(clientEngine, srcLen); + srcLen += 64; + } while (srcLen < ReferenceCountedOpenSslEngine.MAX_PLAINTEXT_LENGTH); + + testWrapDstBigEnough(clientEngine, ReferenceCountedOpenSslEngine.MAX_PLAINTEXT_LENGTH); + } finally { + cleanupClientSslEngine(clientEngine); + cleanupServerSslEngine(serverEngine); + } + } + } + + private void testWrapDstBigEnough(SSLEngine engine, int srcLen) throws SSLException { + ByteBuffer src = allocateBuffer(srcLen); + ByteBuffer dst = allocateBuffer(srcLen + ReferenceCountedOpenSslEngine.MAX_ENCRYPTION_OVERHEAD_LENGTH); + + SSLEngineResult result = engine.wrap(src, dst); + assertEquals(SSLEngineResult.Status.OK, result.getStatus()); + int consumed = result.bytesConsumed(); + int produced = result.bytesProduced(); + assertEquals(srcLen, consumed); + assertTrue(produced > consumed); + + dst.flip(); + assertEquals(produced, dst.remaining()); + assertFalse(src.hasRemaining()); + } + @Override protected SslProvider sslClientProvider() { return SslProvider.OPENSSL; diff --git a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java index e0d9976115..cf19ef8715 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java @@ -1599,7 +1599,7 @@ public abstract class SSLEngineTest { SSLEngine server = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); try { - ByteBuffer dst = allocateBuffer(server.getSession().getPacketBufferSize()); + ByteBuffer dst = allocateBuffer(client.getSession().getPacketBufferSize()); ByteBuffer src = allocateBuffer(1024); handshake(client, server);