This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 87a973b200 Fix BZ 67938 - Handle large client hello messages
87a973b200 is described below
commit 87a973b2005a6828b0777be7d05f51fffc62970f
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 c83e7559fc..459137e871 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -326,10 +326,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),
@@ -567,7 +563,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 b15f15db06..ff6fb548ce 100644
--- a/java/org/apache/tomcat/util/net/SecureNioChannel.java
+++ b/java/org/apache/tomcat/util/net/SecureNioChannel.java
@@ -463,10 +463,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);
@@ -483,7 +479,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 2fd739ca48..ebc7c43140 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -178,6 +178,10 @@
prefer 1.2.x since it supports the APR/Native connector whereas 2.0.x
does not. (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="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
