Am 15.10.2018 um 10:02 schrieb Stefan Eissing:
Am 14.10.2018 um 00:46 schrieb Rainer Jung <rainer.j...@kippdata.de>:
It seems the h2 failure only happens when building httpd against
OpenSSL 1.1.1 (independent of TLS version used). I did a quick check
with an httpd build against 1.1.0i and there the same vhost of the
test framework instance worked with the same clients, that failed for
1.1.1.
The client side OpenSSL version seems not to matter.
When using 1.1.1 in the server even browsers seem to fail with h2.
Anyone who can successfully use h2 with 2.4.36 build against OpenSSL
1.1.1?
Me, and I got reports from others.
When comparing trace logs between the 1.1.1 server and the 1.1.0i
server, one can see, that a failing OpenSSL read (0 bytes) results in
APR_EOF (70014) for 1.1.1 but in errno 11 (EAGAIN) for 1.1.0i. I
wonder whether this is due to the new SSL_MODE_AUTO_RETRY and maybe
the following change:
+#if OPENSSL_VERSION_NUMBER >= 0x1010100fL
+ /* For OpenSSL >=1.1.1, disable auto-retry mode so it's possible
+ * to consume handshake records without blocking for app-data.
+ * https://github.com/openssl/openssl/issues/7178 */
+ SSL_CTX_clear_mode(ctx, SSL_MODE_AUTO_RETRY);
+#endif
Hmm, just read the ticket made by Joe regarding this change.
I try to summarize my understanding:
- this has nothing to do with HTTP/2 in particular. It's only
triggered by the h2 calling sequence.
- SSL_read() only returns transport data, but sometimes reads (part
of) TLS meta data, such as handshake messages.
- When it reads this meta data (and has processed it), it can either
do another read directly after or return with
its equivalent of EAGAIN. By default, it does the first.
- The change in handshake handling between TLSv1.2 and TLSv1.3 led, in
Joe's testing, to the case where SSL_read()'s
internal re-read() on the socket blocked, as the client was not
sending anything.
- So, after discussion on the openssl issue tracker, Joe changed the
OpenSSL behaviour of this by inserting the code above.
- Now, when SSL_read() is called, processes some meta data, it will
not read() on the socket again, but return a read of length 0.
- mod_ssl interprets this as EOF:
ssl_engine_io.c, lines 673..
else if (rc == 0) {
/* If EAGAIN, we will loop given a blocking read,
* otherwise consider ourselves at EOF.
*/
and returns APR_EOF to h2 which then shuts the connection down.
To my understanding, mod_ssl's ssl_engine_io.c, ssl_io_input_read()
has the special case handling that:
- if it gets a SSL_ERROR_WANT_READ from SSL_read(), it remembers that,
calls again
- if SSL_read() == 0, non-blocking, it
a) on having seen SSL_ERROR_WANT_READ before, return APR_EGAIN
b) otherwise, return APR_EOF
OpenSSL's documentation of SSL_read() now states:
Old documentation indicated a difference between 0 and -1, and that
-1 was retry-able.
You should instead call SSL_get_error() to find out if it's retry-able.
So, we should change our logic here. Anyone having the courage to do
so? ;-)
Thanks for the further investigations.
I'm currently testing the following patch which looks OK wrt. test suite
results. Need to run more combinations (OpenSSL version client versus
server) though. Server with 1.1.1 and with 1.0.2p both look OK
(including the h2 tests). Maybe some cases could be folded together or
be dropped, but I tried to make the logic changes not to big. The
SSL_ERROR_ZERO_RETURN part is new, because without that we get an
ssl:info log line AH01992 with error 6 (SSL_ERROR_ZERO_RETURN) at the
end of the response (at least with 1.1.1).
--- modules/ssl/ssl_engine_io.c.orig 2018-08-15 17:01:08.000000000 +0200
+++ modules/ssl/ssl_engine_io.c 2018-10-15 11:46:01.258042000 +0200
@@ -680,35 +680,32 @@
}
return inctx->rc;
}
- else if (rc == 0) {
- /* If EAGAIN, we will loop given a blocking read,
- * otherwise consider ourselves at EOF.
- */
- if (APR_STATUS_IS_EAGAIN(inctx->rc)
- || APR_STATUS_IS_EINTR(inctx->rc)) {
- /* Already read something, return APR_SUCCESS instead.
- * On win32 in particular, but perhaps on other kernels,
- * a blocking call isn't 'always' blocking.
+ else /* (rc <= 0) */ {
+ if (rc == 0) {
+ /* If EAGAIN, we will loop given a blocking read,
+ * otherwise consider ourselves at EOF.
*/
- if (*len > 0) {
- inctx->rc = APR_SUCCESS;
- break;
- }
- if (inctx->block == APR_NONBLOCK_READ) {
- break;
- }
- }
- else {
- if (*len > 0) {
- inctx->rc = APR_SUCCESS;
+ if (APR_STATUS_IS_EAGAIN(inctx->rc)
+ || APR_STATUS_IS_EINTR(inctx->rc)) {
+ /* Already read something, return APR_SUCCESS instead.
+ * On win32 in particular, but perhaps on other
kernels,
+ * a blocking call isn't 'always' blocking.
+ */
+ if (*len > 0) {
+ inctx->rc = APR_SUCCESS;
+ break;
+ }
+ if (inctx->block == APR_NONBLOCK_READ) {
+ break;
+ }
}
else {
- inctx->rc = APR_EOF;
+ if (*len > 0) {
+ inctx->rc = APR_SUCCESS;
+ break;
+ }
}
- break;
}
- }
- else /* (rc < 0) */ {
int ssl_err = SSL_get_error(inctx->filter_ctx->pssl, rc);
conn_rec *c =
(conn_rec*)SSL_get_app_data(inctx->filter_ctx->pssl);
@@ -754,6 +751,10 @@
"SSL input filter read failed.");
}
}
+ else if (rc == 0 && ssl_err == SSL_ERROR_ZERO_RETURN) {
+ inctx->rc = APR_EOF;
+ break;
+ }
else /* if (ssl_err == SSL_ERROR_SSL) */ {
/*
* Log SSL errors and any unexpected conditions.
@@ -763,6 +764,10 @@
ssl_log_ssl_error(SSLLOG_MARK, APLOG_INFO,
mySrvFromConn(c));
}
+ if (rc == 0) {
+ inctx->rc = APR_EOF;
+ break;
+ }
if (inctx->rc == APR_SUCCESS) {
inctx->rc = APR_EGENERAL;
}
Regards,
Rainer
