svn commit: r1843954 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c

[rj...@apache.org]

Author: rjung
Date: Mon Oct 15 21:14:21 2018
New Revision: 1843954

URL: http://svn.apache.org/viewvc?rev=1843954&view=rev
Log:
SSL_read() doesn't distinguish between return value 0 and <0,
at least not for OpenSSL 1.1.1. This is documented in the man
page for SSL_read and let to h2 failures when using OpenSSL 1.1.1.

When no data could be read, our code returned EAGAIN up until
OpenSSL 1.1.0, but APR_EOF for OpenSSL 1.1.1.

Now instead check SSL_get_error() also when SSL_read() returns 0.

To keep changes small, this change should not influence behavior,
when (rc=SSL_read()):
- rc < 0
- rc == 0 && *len > 0
- rc == 0 &&
  (APR_STATUS_IS_EAGAIN(inctx->rc) || APR_STATUS_IS_EINTR(inctx->rc) &&
  inctx->block == APR_NONBLOCK_READ

Behavior changes if
- rc == 0 &&
  !(APR_STATUS_IS_EAGAIN(inctx->rc) || APR_STATUS_IS_EINTR(inctx->rc) &&
  !*len > 0
  Instead of APR_EOF:
  - same behavior as rc < 0 for SSL_ERROR_WANT_READ
  - same behavior as rc < 0 for SSL_ERROR_SYSCALL && APR_STATUS_IS_EAGAIN(inctx->rc)

Another change is that rc == 0 && ssl_err == SSL_ERROR_ZERO_RETURN
also results in APR_EOF.

Modified:
    httpd/httpd/trunk/modules/ssl/ssl_engine_io.c

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1843954&r1=1843953&r2=1843954&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Mon Oct 15 21:14:21 2018
@@ -715,37 +715,36 @@ static apr_status_t ssl_io_input_read(bi
             }
             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) */ {
+            int ssl_err;
+            conn_rec *c;
+            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);
+            ssl_err = SSL_get_error(inctx->filter_ctx->pssl, rc);
+            c = (conn_rec*)SSL_get_app_data(inctx->filter_ctx->pssl);
 
             if (ssl_err == SSL_ERROR_WANT_READ) {
                 /*
@@ -789,6 +788,10 @@ static apr_status_t ssl_io_input_read(bi
                                   "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.
@@ -798,6 +801,10 @@ static apr_status_t ssl_io_input_read(bi
                 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;
             }