From 737144a35e80911fbb9264f5a0ff137f32447065 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 30 Oct 2019 15:29:20 -0500 Subject: [PATCH] Issue #4217 - Allowing Large TLS Records in Java 11+ in Jetty 9.2.x Signed-off-by: Joakim Erdfelt --- .../eclipse/jetty/io/ssl/SslConnection.java | 147 ++++++++++++++---- 1 file changed, 117 insertions(+), 30 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 819e41b4865a..3d8f1414e25b 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -28,6 +28,7 @@ import javax.net.ssl.SSLEngineResult.HandshakeStatus; import javax.net.ssl.SSLEngineResult.Status; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSession; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.AbstractEndPoint; @@ -142,6 +143,57 @@ public void setRenegotiationAllowed(boolean renegotiationAllowed) this._renegotiationAllowed = renegotiationAllowed; } + private int getApplicationBufferSize() + { + SSLSession hsSession = _sslEngine.getHandshakeSession(); + SSLSession session = _sslEngine.getSession(); + int size = session.getApplicationBufferSize(); + if (hsSession == null || hsSession == session) + return size; + int hsSize = hsSession.getApplicationBufferSize(); + return Math.max(hsSize, size); + } + + private int getPacketBufferSize() + { + SSLSession hsSession = _sslEngine.getHandshakeSession(); + SSLSession session = _sslEngine.getSession(); + int size = session.getPacketBufferSize(); + if (hsSession == null || hsSession == session) + return size; + int hsSize = hsSession.getPacketBufferSize(); + return Math.max(hsSize, size); + } + + private void acquireEncryptedInput() + { + if (_encryptedInput == null) + _encryptedInput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers); + } + + private void acquireEncryptedOutput() + { + if (_encryptedOutput == null) + _encryptedOutput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers); + } + + private void releaseEncryptedInputBuffer() + { + if (_encryptedInput != null && !_encryptedInput.hasRemaining()) + { + _bufferPool.release(_encryptedInput); + _encryptedInput = null; + } + } + + protected void releaseDecryptedInputBuffer() + { + if (_decryptedInput != null && !_decryptedInput.hasRemaining()) + { + _bufferPool.release(_decryptedInput); + _decryptedInput = null; + } + } @Override public void onOpen() { @@ -480,9 +532,12 @@ public void setConnection(Connection connection) { if (connection instanceof AbstractConnection) { - AbstractConnection a = (AbstractConnection)connection; - if (a.getInputBufferSize()<_sslEngine.getSession().getApplicationBufferSize()) - a.setInputBufferSize(_sslEngine.getSession().getApplicationBufferSize()); + // This is an optimization to avoid that upper layer connections use small + // buffers and we need to copy decrypted data rather than decrypting in place. + AbstractConnection c = (AbstractConnection)connection; + int appBufferSize = getApplicationBufferSize(); + if (c.getInputBufferSize() < appBufferSize) + c.setInputBufferSize(appBufferSize); } super.setConnection(connection); } @@ -509,18 +564,22 @@ public synchronized int fill(ByteBuffer buffer) throws IOException else BufferUtil.compact(_encryptedInput); - // We also need an app buffer, but can use the passed buffer if it is big enough - ByteBuffer app_in; - if (BufferUtil.space(buffer) > _sslEngine.getSession().getApplicationBufferSize()) - app_in = buffer; - else if (_decryptedInput == null) - app_in = _decryptedInput = _bufferPool.acquire(_sslEngine.getSession().getApplicationBufferSize(), _decryptedDirectBuffers); - else - app_in = _decryptedInput; - // loop filling and unwrapping until we have something while (true) { + // We also need an app buffer, but can use the passed buffer if it is big enough + ByteBuffer app_in; + int appBufferSize = getApplicationBufferSize(); + + if (BufferUtil.space(buffer) > appBufferSize) + app_in = buffer; + else if (_decryptedInput == null) + app_in = _decryptedInput = _bufferPool.acquire(appBufferSize, _decryptedDirectBuffers); + else + app_in = _decryptedInput; + + acquireEncryptedInput(); + // Let's try reading some encrypted data... even if we have some already. int net_filled = getEndPoint().fill(_encryptedInput); if (DEBUG) @@ -540,8 +599,13 @@ else if (_decryptedInput == null) { BufferUtil.flipToFlush(app_in, pos); } - if (DEBUG) - LOG.debug("{} unwrap {}", SslConnection.this, unwrapResult); + if (LOG.isDebugEnabled()) + LOG.debug("unwrap net_filled={} {} encryptedBuffer={} unwrapBuffer={} appBuffer={}", + net_filled, + unwrapResult.toString().replace('\n',' '), + BufferUtil.toSummaryString(_encryptedInput), + BufferUtil.toDetailString(app_in), + BufferUtil.toDetailString(buffer)); HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus(); HandshakeStatus unwrapHandshakeStatus = unwrapResult.getHandshakeStatus(); @@ -594,6 +658,19 @@ else if (_decryptedInput == null) } } } + case BUFFER_OVERFLOW: + // It's possible that SSLSession.applicationBufferSize has been expanded + // by the SSLEngine implementation. Unwrapping a large encrypted buffer + // causes BUFFER_OVERFLOW because the (old) applicationBufferSize is + // too small. Release the decrypted input buffer so it will be re-acquired + // with the larger capacity. + // See also system property "jsse.SSLEngine.acceptLargeFragments". + if (BufferUtil.isEmpty(_decryptedInput) && appBufferSize < getApplicationBufferSize()) + { + releaseDecryptedInputBuffer(); + break decryption; + } + throw new IllegalStateException("Unexpected unwrap result " + unwrapResultStatus); case BUFFER_UNDERFLOW: case OK: { @@ -693,16 +770,9 @@ else if (_decryptedInput == null) getExecutor().execute(_runCompletWrite); } - if (_encryptedInput != null && !_encryptedInput.hasRemaining()) - { - _bufferPool.release(_encryptedInput); - _encryptedInput = null; - } - if (_decryptedInput != null && !_decryptedInput.hasRemaining()) - { - _bufferPool.release(_decryptedInput); - _decryptedInput = null; - } + releaseEncryptedInputBuffer(); + releaseDecryptedInputBuffer(); + if (DEBUG) LOG.debug("{} fill exit", SslConnection.this); } @@ -754,19 +824,20 @@ public synchronized boolean flush(ByteBuffer... appOuts) throws IOException return false; } - // We will need a network buffer - if (_encryptedOutput == null) - _encryptedOutput = _bufferPool.acquire(_sslEngine.getSession().getPacketBufferSize(), _encryptedDirectBuffers); - while (true) { - // We call sslEngine.wrap to try to take bytes from appOut buffers and encrypt them into the _netOut buffer + int packetBufferSize = getPacketBufferSize(); + acquireEncryptedOutput(); + + // We call sslEngine.wrap to try to take bytes from appOuts + // buffers and encrypt them into the _encryptedOutput buffer. + BufferUtil.compact(_encryptedOutput); int pos = BufferUtil.flipToFill(_encryptedOutput); SSLEngineResult wrapResult; try { - wrapResult = wrap(_sslEngine, appOuts, _encryptedOutput); + wrapResult = wrap(_sslEngine, appOuts,_encryptedOutput); } finally { @@ -808,6 +879,22 @@ public synchronized boolean flush(ByteBuffer... appOuts) throws IOException } return allConsumed; + case BUFFER_OVERFLOW: + { + // It's possible that SSLSession.packetBufferSize has been expanded + // by the SSLEngine implementation. Wrapping a large application buffer + // causes BUFFER_OVERFLOW because the (old) packetBufferSize is + // too small. Release the encrypted output buffer so that it will + // be re-acquired with the larger capacity. + // See also system property "jsse.SSLEngine.acceptLargeFragments". + if (packetBufferSize < getPacketBufferSize()) + { + releaseEncryptedOutputBuffer(); + continue; + } + throw new IllegalStateException("Unexpected wrap result " + wrapResultStatus); + } + case BUFFER_UNDERFLOW: throw new IllegalStateException();