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
