Am 29.11.2018 um 15:55 schrieb Mark Thomas:
On 29/11/2018 13:27, Rémy Maucherat wrote:
On Sun, Nov 25, 2018 at 10:42 AM Rainer Jung <rainer.j...@kippdata.de>
wrote:
In our Java code, what happens is a call to unwrap() in OpenSSLEngine.
This call writes I think 146 bytes, then checks
pendingReadableBytesInSSL(). That call in turn calls SSL.readFromSSL()
and gets back "0" (from SSL_read()). Up in unwrap() we then skip the
while loop and finally return with BUFFER_UNDERFLOW. Then we hang,
probably because the data was read by OpenSSL and no more socket event
happens. If I artificially add another call to
pendingReadableBytesInSSL() which triggers another SSL_read(), the hang
does not occur.
IMHO TLS 1.0 is not such a big problem, but we should at least document
it when we do a new release.
Last time, Mark fixed pendingReadableBytesInSSL (=
SSL.pendingReadableBytesInSSL) not working by using a fake read
(SSL.readFromSSL(ssl, EMPTY_ADDR, 0)) to start a new record. So then we
actually need to make *two* fake reads (if the result of the first is zero)
and we would be fine ? This OpenSSL API sounds ridiculously bad ... (IMO)
My research is leading me in that direction. So far, it looks like TLS
1.0 is sending two application data packets (I assume this is mitigation
for BEAST) whereas TLS 1.1 and later only send 1. It appears those two
packets each need a priming read. Not sure why. I'd expect to be able to
read a single character after the first packet but the API indicates 0
characters are available.
I have been trying to find some API to call that reliably indicates that
another priming read is required but I haven't found it yet. I am
currently looking at triggering a second priming read if:
- TLS 1.0 is being used
- a priming read returns 0
- SSL_get_error returns 5
- pendingReadableBytesInSSL returns 0
I also want to think about trying to reduce the number of times we call
into native code. I'm not sure how practical this is and - since it only
affects TLS 1.0 - I'm not overly concerned since folks should really be
on TLS 1.2 / 1.3 by now.
I like the restriction of the workaround for TLS 1.0. My experiments
seemed to indicate, that such an additional priming read might not
directly help at the point in time where the normal read returns 0 byte,
but it might help indirectly, because it is also done at some
communication stage before and then the first read after processing no
longer returns 0 but instead some bytes. Everything quite vague, I know.
Regards,
Rainer
So you made it sound like it would work:
Index: java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
===================================================================
--- java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java (revision
1847712)
+++ java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java (working
copy)
@@ -635,6 +635,9 @@
// See https://www.openssl.org/docs/manmaster/ssl/SSL_pending.html
clearLastError();
int lastPrimingReadResult = SSL.readFromSSL(ssl, EMPTY_ADDR, 0);
// priming read
+ if (lastPrimingReadResult == 0) {
+ lastPrimingReadResult = SSL.readFromSSL(ssl, EMPTY_ADDR, 0);
+ }
// check if SSL_read returned <= 0. In this case we need to check
the error and see if it was something
// fatal.
if (lastPrimingReadResult <= 0) {
Yep. That is a much cleaner version of what I have currently working.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
