This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 7dd70b6c07 Fix BZ 67938 - Handle large client hello messages
7dd70b6c07 is described below
commit 7dd70b6c071c1d2651305047b83491967cac80d9
Author: Mark Thomas <ma...@apache.org>
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 bb196c9f89..d7edbd7afa 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -329,10 +329,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),
@@ -570,7 +566,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 23b420a4fc..36912dbe52 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 9ec87f1caf..9085d69063 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -184,6 +184,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="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
