tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 62791] New: SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."
Date Tue, 02 Oct 2018 08:57:30 GMT
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

            Bug ID: 62791
           Summary: SecureNioChannel fails with "IllegalArgumentException:
                    You can only read using the application read buffer
                    provided by the handler."
           Product: Tomcat 9
           Version: 9.0.12
          Hardware: PC
                OS: Mac OS X 10.1
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Connectors
          Assignee: dev@tomcat.apache.org
          Reporter: mfedoryshyn@atlassian.com
  Target Milestone: -----

I ran into issue with Tomcat 9 failing to handle data exchange via WSS: 

02-Oct-2018 17:02:01.421 SEVERE [https-jsse-nio-8443-exec-9]
org.apache.coyote.AbstractProtocol$ConnectionHandler.process Error reading
request, ignored
 java.lang.IllegalArgumentException: You can only read using the application
read buffer provided by the handler.
        at
org.apache.tomcat.util.net.SecureNioChannel.read(SecureNioChannel.java:571)
        at
org.apache.tomcat.util.net.NioEndpoint$NioSocketWrapper.fillReadBuffer(NioEndpoint.java:1204)
        at
org.apache.tomcat.util.net.NioEndpoint$NioSocketWrapper.read(NioEndpoint.java:1140)
        at
org.apache.tomcat.websocket.server.WsFrameServer.onDataAvailable(WsFrameServer.java:72)
        at
org.apache.tomcat.websocket.server.WsFrameServer.doOnDataAvailable(WsFrameServer.java:171)
        at
org.apache.tomcat.websocket.server.WsFrameServer.notifyDataAvailable(WsFrameServer.java:151)
        at
org.apache.tomcat.websocket.server.WsHttpUpgradeHandler.upgradeDispatch(WsHttpUpgradeHandler.java:148)
        at
org.apache.coyote.http11.upgrade.UpgradeProcessorInternal.dispatch(UpgradeProcessorInternal.java:54)
        at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
        at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:770)
        at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1415)
        at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
        at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
        at java.lang.Thread.run(Thread.java:748)


On a frontend side, I have a pretty simple javascript, which sends its state
updates via web sockets (text) to the backend service (simple spring app
running in Tomcat).
After Tomcat upgrade (from 8.0.53 to 9.0.12), application started failing to
establish web socket connections with ssl-enabled connector ("plain" web
sockets work fine). 

Steps to reproduce:
1. Generate certificate (I used steps from here:
https://confluence.atlassian.com/doc/running-confluence-over-ssl-or-https-161203.html#RunningConfluenceOverSSLorHTTPS-Option1:Createaself-signedcertificate)
2. Configure "secured" connector in server.xml:
<Connector port="8443" maxHttpHeaderSize="8192"
                   maxThreads="150" minSpareThreads="25"
                   protocol="org.apache.coyote.http11.Http11NioProtocol"
                   enableLookups="false" disableUploadTimeout="true"
                   acceptCount="100" scheme="https" secure="true"
                   clientAuth="false" sslProtocols="TLSv1,TLSv1.1,TLSv1.2"      
                   sslEnabledProtocols="TLSv1,TLSv1.1,TLSv1.2"
                   SSLEnabled="true"
                   URIEncoding="UTF-8" keystorePass="changeit"
                   keystoreFile="/Users/user/.keystore"/>

With such connector settings, all attempts to establish web socket connections
over ssl fail with exception above. At the same time:
1. Plain web sockets work
<Connector port="8090" connectionTimeout="20000" 
                   maxThreads="48" minSpareThreads="10"
                   enableLookups="false" acceptCount="10" debug="0"
URIEncoding="UTF-8"
                   protocol="org.apache.coyote.http11.Http11NioProtocol"/>
2. All non-websockets requests work (both http and https)
3. Everything works when I downgrade Tomcat to version 8.0.53 (I didn't try
other versions)

Looking a bit deeper into the problem, it seems that handshake succeeds, but
Tomcat fails on the first attempt to read the data from a web socket.

It looks like WsFrameServer does not respect SocketBufferHandler settings from
NioEndpoint$NioSocketWrapper at the time of its (WsFrameServer) instantiation
in WsHttpUpgradeHandler, and always creates its own read buffer
(WsFrameBase.java:93). Later, NioEndpoint$NioSocketWrapper tries its best to
fill in this buffer as much as it could, and attempts to read directly to the
buffer from the websocket (NioEndpoint.java:1204), which causes buffer check
failure in SecureNioChannel.

There are several ways of fixing it.
1. The most obvious solution is to disable direct reads from the socket. Easy,
but probably comes with performance degradation in some cases:
Index: java/org/apache/tomcat/util/net/NioEndpoint.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/tomcat/util/net/NioEndpoint.java    (revision
b648bba7acc3529c35d7bc5c54639a3dcb08e812)
+++ java/org/apache/tomcat/util/net/NioEndpoint.java    (date 1538469054000)
@@ -1135,26 +1135,16 @@

             // The socket read buffer capacity is socket.appReadBufSize
             int limit = socketBufferHandler.getReadBuffer().capacity();
-            if (to.remaining() >= limit) {
-                to.limit(to.position() + limit);
-                nRead = fillReadBuffer(block, to);
-                if (log.isDebugEnabled()) {
-                    log.debug("Socket: [" + this + "], Read direct from
socket: [" + nRead + "]");
-                }
-                updateLastRead();
-            } else {
-                // Fill the read buffer as best we can.
-                nRead = fillReadBuffer(block);
-                if (log.isDebugEnabled()) {
-                    log.debug("Socket: [" + this + "], Read into buffer: [" +
nRead + "]");
-                }
-                updateLastRead();
+            nRead = fillReadBuffer(block);
+            if (log.isDebugEnabled()) {
+                log.debug("Socket: [" + this + "], Read into buffer: [" +
nRead + "]");
+            }
+            updateLastRead();

-                // Fill as much of the remaining byte array as possible with
the
-                // data that was just read
-                if (nRead > 0) {
-                    nRead = populateReadBuffer(to);
-                }
+            // Fill as much of the remaining byte array as possible with the
+            // data that was just read
+            if (nRead > 0) {
+                nRead = populateReadBuffer(to);
             }
             return nRead;
         }

2. Another possible solution would be to use temporary buffer in WsFrameServer:
Index: java/org/apache/tomcat/websocket/server/WsFrameServer.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/tomcat/websocket/server/WsFrameServer.java  (revision
b648bba7acc3529c35d7bc5c54639a3dcb08e812)
+++ java/org/apache/tomcat/websocket/server/WsFrameServer.java  (date
1538469610000)
@@ -69,7 +69,10 @@
             // Fill up the input buffer with as much data as we can
             inputBuffer.mark();
            
inputBuffer.position(inputBuffer.limit()).limit(inputBuffer.capacity());
-            int read = socketWrapper.read(false, inputBuffer);
+            int capacity = inputBuffer.capacity();
+            byte[] byteArray = new byte[capacity];
+            int read = socketWrapper.read(false, byteArray, 0, capacity);
+            inputBuffer.put(byteArray);
             inputBuffer.limit(inputBuffer.position()).reset();
             if (read < 0) {
                 throw new EOFException();
Disadvantages for this approach would be creation of unnecessary byte buffer
(i.e. allocating unnecessary memory) and (possibly) race conditions (if
inputBuffer is written while we're reading to byteArray)

3. We may also get rid of buffer check in SecureNioChannel (security concern)
or add some parameter to ignore that check (a bit "dirty" solution as for me):
Index: java/org/apache/tomcat/util/net/NioEndpoint.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/tomcat/util/net/NioEndpoint.java    (revision
b648bba7acc3529c35d7bc5c54639a3dcb08e812)
+++ java/org/apache/tomcat/util/net/NioEndpoint.java    (date 1538470221000)
@@ -1201,7 +1201,7 @@
                     }
                 }
             } else {
-                nRead = channel.read(to);
+                nRead = channel instanceof SecureNioChannel ?
((SecureNioChannel)channel).read(to, true) : channel.read(to);
                 if (nRead == -1) {
                     throw new EOFException();
                 }
Index: java/org/apache/tomcat/util/net/SecureNioChannel.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/tomcat/util/net/SecureNioChannel.java       (revision
b648bba7acc3529c35d7bc5c54639a3dcb08e812)
+++ java/org/apache/tomcat/util/net/SecureNioChannel.java       (date
1538470221000)
@@ -565,8 +565,12 @@
      */
     @Override
     public int read(ByteBuffer dst) throws IOException {
+        return read(dst, false);
+    }
+
+    public int read(ByteBuffer dst, boolean ignoreBufferCheck) throws
IOException {
         // Make sure we only use the ApplicationBufferHandler's buffers
-        if (dst != getBufHandler().getReadBuffer() && (getAppReadBufHandler()
== null
+        if (!ignoreBufferCheck && dst != getBufHandler().getReadBuffer() &&
(getAppReadBufHandler() == null
                 || dst != getAppReadBufHandler().getByteBuffer())) {
             throw new
IllegalArgumentException(sm.getString("channel.nio.ssl.invalidBuffer"));
         }



There is a workaround: to use Http11Nio2Protocol instead of Http11NioProtocol

Java version: 1.8.0_171-b11

OS: OSX and Amazon Linux

Related changes:
Introduce fillReadBuffer(boolean, ByteBuffer)  git-svn-id:
https://svn.apache.org/repos/asf/tomcat/trunk@1758000
13f79535-47bb-0310-9956-ffa450edef68
Introduce a new method SocketWrapperBase.read(boolean, ByteBuffer)  git-svn-id:
https://svn.apache.org/repos/asf/tomcat/trunk@1758058
13f79535-47bb-0310-9956-ffa450edef68

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message