You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2018/10/02 08:57:30 UTC

[Bug 62791] New: SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #5 from Mark Thomas <ma...@apache.org> ---
I agree the check is pointless for the reasons outlined in comment #3.

Having looked at the openjdk source I think there is a lot of scope to clean
this up. The TLS spec specifies a maximum of 2^14. Java defaults to this or, if
jsse.SSLEngine.acceptLargeFragments is set, defaults to 2^15 to allow for
non-compliant TLS stacks. After that any renegotiation of the app buffer size
is only ever going to be downwards - not upwards - for any given session. The
same goes for the packet buffer.

I think we can remove the whole resize concept. We added it thinking the
language in the Javadoc meant it could be resized upwards but my understanding
of the code I have read is that it will only ever go downwards.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #9 from Maksym <mf...@atlassian.com> ---
Thank you for fixing it! Does it make sense to backport this fix to tomcat
8.0.x as well (especially taking into account that it was fixed in 7.0.x)?

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
I'll do some svn archaeology to see if I can find out why that check was added
in the first place and if that reason is still valid.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- trunk for 9.0.13 onwards
- 8.5.x for 8.5.35 onwards
- 7.0.x for 7.0.92 onwards

I'll look into what, if any, further clean-up is possible separately.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #11 from Maksym <mf...@atlassian.com> ---
(In reply to Mark Thomas from comment #6)
> Fixed in:
> - trunk for 9.0.13 onwards
> - 8.5.x for 8.5.35 onwards
> - 7.0.x for 7.0.92 onwards
> 
> I'll look into what, if any, further clean-up is possible separately.

When 9.0.13 is planned for release?

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #10 from Remy Maucherat <re...@apache.org> ---
The Tomcat 8.0 branch is EOL.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #8 from Remy Maucherat <re...@apache.org> ---
+1 to leave it as is.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #1 from Remy Maucherat <re...@apache.org> ---
I couldn't figure out why it was such a drama to not use the container buffer
for NIO SSL (except: no resize ...), so I didn't put it in NIO2, that's why it
works. The code path clearly uses the application provided buffer in
SecureNioChannel in the situation you describe where there are no bytes left.

Either way, I would simply remove the check.

Comments ?

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #7 from Mark Thomas <ma...@apache.org> ---
Looks like I missed something when looking at the OpenJDK code.

From the Oracle JSSE docs:

<quote>
Note: The SSL/TLS protocols specify that implementations are to produce packets
containing at most 16 kilobytes (KB) of plain text. However, some
implementations violate the specification and generate large records up to 32
KB. If the SSLEngine.unwrap() code detects large inbound packets, then the
buffer sizes returned by SSLSession will be updated dynamically. Applications
should always check the BUFFER_OVERFLOW and BUFFER_UNDERFLOW statuses and
enlarge the corresponding buffers if necessary. SunJSSE will always send
standard compliant 16 KB records and allow incoming 32 KB records. For a
workaround, see the System property jsse.SSLEngine.acceptLargeFragments in
Customizing JSSE.
</quote>


If we removed the resizing then any spec non-complaint clients are going to
fail until Tomcat is restarted with the above system property set. On balance,
I think it is best to leave things as they are.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #3 from Remy Maucherat <re...@apache.org> ---
Either way the check is pointless, removing it means it will cause an IOE later
only if a resize is (somehow) needed. The scenario is handled well in the code.

-- 
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


[Bug 62791] SecureNioChannel fails with "IllegalArgumentException: You can only read using the application read buffer provided by the handler."

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62791

--- Comment #4 from Mark Thomas <ma...@apache.org> ---
I am currently looking at the openjdk sources to see what circumstances, if
any, a resize would be required.

-- 
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