This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 1fe0def33f Fix BZ 67938 - Handle large client hello messages
1fe0def33f is described below
commit 1fe0def33fe3da644f697631f2cc2e7941bba84a
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 0016b01bb4..b41b05ac0e 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 dc001e62b8..2de47812fa 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 244cd99db7..43ca6513ef 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: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
