tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject SSL errors with tc-native
Date Wed, 22 Jun 2016 11:20:41 GMT
A while ago I observed unexpected APR_EGENERAL errors being returned
when performing SSL reads. I was unable to identify the root cause but I
did discover that if those errors were treated as EAGAIN, processing
continued normally. As a result, I committed [1].

A report on the users list [2] has highlighted that, in some
circumstances at least, [1] is the wrong thing to do. I have therefore
investigated the circumstances that led to [1] further. The relevant
code is [3].

With some local debug logging I have discovered that in the case [1] was
trying to address the result of the SSL_read call at [3] is as follows:
s == -1
i == 5 (SSL_ERROR_SYSCALL)
rv = 730035

Subtracting the 720000 offset from rv gives the OS error as 10035 which
[4] gives as WSAEWOULDBLOCK which looks exactly like EAGAIN to me.

Based on the above, my conclusion is that [2] was caused by some other
windows error which was incorrectly handled as EAGAIN.

Therefore, I'd like to propose something along the following lines for
tc-native:
Index: src/sslnetwork.c
===================================================================
--- src/sslnetwork.c    (revision 1749592)
+++ src/sslnetwork.c    (working copy)
@@ -427,7 +427,11 @@
                         con->shutdown_type = SSL_SHUTDOWN_TYPE_STANDARD;
                         return APR_EOF;
                     }
-#if !defined(_WIN32)
+#if defined(_WIN32)
+                    else if (rv == 730035 && timeout == 0) {
+                        return APR_EAGAIN;
+                    }
+#else
                     else if (APR_STATUS_IS_EINTR(rv)) {
                         /* Interrupted by signal
                          */

I'd appreciate some review of this change as I know C is not my strong
point. The hard-coded value for the test of rv looks wrong to me. Is
there a better way to do this? Any other review comments?

Obviously some changes will be required on the Tomcat side as well. I'm
still looking at those as I think I have discovered another issue that
was masked by [1].

Thanks,

Mark



[1] http://svn.apache.org/r1534619
[2]
https://lists.apache.org/thread.html/170386a4bd6ec3d66bf81ca1e43838d11a403569993cb5bd2bddbdb2@%3Cusers.tomcat.apache.org%3E
[3]
http://svn.apache.org/viewvc/tomcat/native/trunk/native/src/sslnetwork.c?view=annotate#l407
[4]
https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668(v=vs.85).aspx

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message