This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 1fa73349d8 Fix BZ 67938 - Handle large client hello messages
1fa73349d8 is described below
commit 1fa73349d870147c46d1029c589989edf64c58a9
Author: Mark Thomas <[email protected]>
AuthorDate: Fri Nov 3 19:05:43 2023 +0000
Fix BZ 67938 - Handle large client hello messages
The removed calls to ByteBuffer.clear() have been present since the code
was first added to Tomcat. From the code coverage results, clear() was
never called by the unit tests.
At the point clear() might have been called, the ByteBuffer was in
"write to" mode. Therefore, the call to clear() was always going to
corrupt the buffer if called.
https://bz.apache.org/bugzilla/show_bug.cgi?id=67938
---
java/org/apache/tomcat/util/net/SecureNio2Channel.java | 11 ++++++-----
java/org/apache/tomcat/util/net/SecureNioChannel.java | 11 ++++++-----
webapps/docs/changelog.xml | 4 ++++
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
index e33edb205c..474ad033f2 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -324,10 +324,6 @@ public class SecureNio2Channel extends Nio2Channel {
handshakeStatus = tasks();
}
} else if (handshake.getStatus() ==
Status.BUFFER_UNDERFLOW) {
- if (netInBuffer.position() == netInBuffer.limit()) {
- //clear the buffer if we have emptied it out on
data
- netInBuffer.clear();
- }
//read more data
if (async) {
sc.read(netInBuffer,
AbstractEndpoint.toTimeout(timeout),
@@ -565,7 +561,12 @@ public class SecureNio2Channel extends Nio2Channel {
//call unwrap
getBufHandler().configureReadBufferForWrite();
result = sslEngine.unwrap(netInBuffer,
getBufHandler().getReadBuffer());
- //compact the buffer, this is an optional method, wonder what
would happen if we didn't
+ /*
+ * ByteBuffer.compact() is an optional method but netInBuffer is
created from either ByteBuffer.allocate()
+ * or ByteBuffer.allocateDirect() and the ByteBuffers returned by
those methods do implement compact().
+ * The ByteBuffer must be in 'read from' mode when compact() is
called and will be in 'write to' mode
+ * afterwards.
+ */
netInBuffer.compact();
//read in the status
handshakeStatus = result.getHandshakeStatus();
diff --git a/java/org/apache/tomcat/util/net/SecureNioChannel.java
b/java/org/apache/tomcat/util/net/SecureNioChannel.java
index beac2d62e3..863b6215df 100644
--- a/java/org/apache/tomcat/util/net/SecureNioChannel.java
+++ b/java/org/apache/tomcat/util/net/SecureNioChannel.java
@@ -461,10 +461,6 @@ public class SecureNioChannel extends NioChannel {
*/
protected SSLEngineResult handshakeUnwrap(boolean doread) throws
IOException {
- if (netInBuffer.position() == netInBuffer.limit()) {
- //clear the buffer if we have emptied it out on data
- netInBuffer.clear();
- }
if (doread) {
//if we have data to read, read it
int read = sc.read(netInBuffer);
@@ -481,7 +477,12 @@ public class SecureNioChannel extends NioChannel {
//call unwrap
getBufHandler().configureReadBufferForWrite();
result = sslEngine.unwrap(netInBuffer,
getBufHandler().getReadBuffer());
- //compact the buffer, this is an optional method, wonder what
would happen if we didn't
+ /*
+ * ByteBuffer.compact() is an optional method but netInBuffer is
created from either ByteBuffer.allocate()
+ * or ByteBuffer.allocateDirect() and the ByteBuffers returned by
those methods do implement compact().
+ * The ByteBuffer must be in 'read from' mode when compact() is
called and will be in 'write to' mode
+ * afterwards.
+ */
netInBuffer.compact();
//read in the status
handshakeStatus = result.getHandshakeStatus();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 480626a10f..d71eb9e23f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -173,6 +173,10 @@
<bug>67927</bug>: Reloading TLS configuration can cause the Connector
to
refuse new connections or the JVM to crash. (markt)
</fix>
+ <fix>
+ <bug>67938</bug>: Correct handling of large TLS client hello messages
that
+ were causing the TLS handshake to fail. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
