You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by ko...@apache.org on 2023/05/18 09:32:30 UTC

svn commit: r1909903 - in /serf/branches/1.3.x: ./ STATUS buckets/ssl_buckets.c test/server/test_server.h test/server/test_sslserver.c

Author: kotkov
Date: Thu May 18 09:32:30 2023
New Revision: 1909903

URL: http://svn.apache.org/viewvc?rev=1909903&view=rev
Log:
On the '1.3.x' branch: Merge r1902208, r1902304 from trunk:

 * r1902208, r1902304
   Rework BIO control handlers to support BIO_CTRL_EOF and to properly respond
   to unknown control values.
   Justification:
     - Fixes "unexpected eof while reading" errors with OpenSSL 3, also observed
       in the test suite.
     - Fixes a user-reported issue with OpenSSL 3 where serf BIOs are incorrectly
       assumed to support KTLS:
       https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253135
   Branch: ^/serf/branches/1.3.x-r1902208+r1902304
   Votes:
     +1: kotkov, ivan

Modified:
    serf/branches/1.3.x/   (props changed)
    serf/branches/1.3.x/STATUS
    serf/branches/1.3.x/buckets/ssl_buckets.c
    serf/branches/1.3.x/test/server/test_server.h
    serf/branches/1.3.x/test/server/test_sslserver.c

Propchange: serf/branches/1.3.x/
------------------------------------------------------------------------------
  Merged /serf/branches/1.3.x-r1902208+r1902304:r1909396-1909902
  Merged /serf/trunk:r1902208,1902304

Modified: serf/branches/1.3.x/STATUS
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/STATUS?rev=1909903&r1=1909902&r2=1909903&view=diff
==============================================================================
--- serf/branches/1.3.x/STATUS (original)
+++ serf/branches/1.3.x/STATUS Thu May 18 09:32:30 2023
@@ -35,19 +35,6 @@ Candidate changes:
            this point for the 1.3.x branch
          - Seems to only be required for LibreSSL, not OpenSSL)
 
- * r1902208, r1902304
-   Rework BIO control handlers to support BIO_CTRL_EOF and to properly respond
-   to unknown control values.
-   Justification:
-     - Fixes "unexpected eof while reading" errors with OpenSSL 3, also observed
-       in the test suite.
-     - Fixes a user-reported issue with OpenSSL 3 where serf BIOs are incorrectly
-       assumed to support KTLS:
-       https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253135
-   Branch: ^/serf/branches/1.3.x-r1902208+r1902304
-   Votes:
-     +1: kotkov, ivan
-
  * r1909252, r1909385, r1909406, r1909413, r1909433
    Do not use OpenSSL functions that operate with FILE to avoid potential CRT
    versions mismatch. Use BIO based functions instead.

Modified: serf/branches/1.3.x/buckets/ssl_buckets.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/buckets/ssl_buckets.c?rev=1909903&r1=1909902&r2=1909903&view=diff
==============================================================================
--- serf/branches/1.3.x/buckets/ssl_buckets.c (original)
+++ serf/branches/1.3.x/buckets/ssl_buckets.c Thu May 18 09:32:30 2023
@@ -176,6 +176,10 @@ struct serf_ssl_context_t {
     /* Status of a fatal error, returned on subsequent encrypt or decrypt
        requests. */
     apr_status_t fatal_err;
+
+    /* OpenSSL 1.1.1e introduced BIO_FLAGS_IN_EOF, but we implement
+       our own hit eof to support versions < 1.1.1e. */
+    int hit_eof;
 };
 
 typedef struct {
@@ -284,6 +288,10 @@ static int bio_bucket_read(BIO *bio, cha
     serf__log(SSL_VERBOSE, __FILE__, "bio_bucket_read received %d bytes (%d)\n",
               len, status);
 
+    if (APR_STATUS_IS_EOF(status)) {
+        ctx->hit_eof = 1;
+    }
+
     if (!SERF_BUCKET_READ_ERROR(status)) {
         /* Oh suck. */
         if (len) {
@@ -407,21 +415,43 @@ static int bio_bucket_destroy(BIO *bio)
 
 static long bio_bucket_ctrl(BIO *bio, int cmd, long num, void *ptr)
 {
-    long ret = 1;
+    serf_ssl_context_t *ctx = bio_get_data(bio);
 
     switch (cmd) {
+    case BIO_CTRL_FLUSH:
+        /* At this point we can't force a flush. */
+        return 1;
+    case BIO_CTRL_PUSH:
+    case BIO_CTRL_POP:
+        return 0;
+    case BIO_CTRL_EOF:
+        return ctx->hit_eof;
     default:
         /* abort(); */
-        break;
+        return 0;
+    }
+}
+
+static long bio_file_ctrl(BIO *bio, int cmd, long num, void *ptr)
+{
+    apr_file_t *file = bio_get_data(bio);
+
+    switch (cmd) {
     case BIO_CTRL_FLUSH:
         /* At this point we can't force a flush. */
-        break;
+        return 1;
     case BIO_CTRL_PUSH:
     case BIO_CTRL_POP:
-        ret = 0;
-        break;
+        return 0;
+    case BIO_CTRL_EOF:
+        if (apr_file_eof(file) == APR_EOF)
+            return 1;
+        else
+            return 0;
+    default:
+        /* abort(); */
+        return 0;
     }
-    return ret;
 }
 
 #ifndef USE_OPENSSL_1_1_API
@@ -447,7 +477,7 @@ static BIO_METHOD bio_file_method = {
     bio_file_read,
     NULL,                        /* Is this called? */
     bio_file_gets,               /* Is this called? */
-    bio_bucket_ctrl,
+    bio_file_ctrl,
     bio_bucket_create,
     bio_bucket_destroy,
 #ifdef OPENSSL_VERSION_NUMBER
@@ -487,7 +517,7 @@ static BIO_METHOD *bio_meth_file_new(voi
     BIO_meth_set_write(biom, bio_file_write);
     BIO_meth_set_read(biom, bio_file_read);
     BIO_meth_set_gets(biom, bio_file_gets);
-    BIO_meth_set_ctrl(biom, bio_bucket_ctrl);
+    BIO_meth_set_ctrl(biom, bio_file_ctrl);
     BIO_meth_set_create(biom, bio_bucket_create);
     BIO_meth_set_destroy(biom, bio_bucket_destroy);
 #else
@@ -1410,6 +1440,7 @@ static serf_ssl_context_t *ssl_init_cont
     ssl_ctx->cached_cert_pw = 0;
     ssl_ctx->pending_err = APR_SUCCESS;
     ssl_ctx->fatal_err = APR_SUCCESS;
+    ssl_ctx->hit_eof = 0;
 
     ssl_ctx->cert_callback = NULL;
     ssl_ctx->cert_pw_callback = NULL;

Modified: serf/branches/1.3.x/test/server/test_server.h
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/test/server/test_server.h?rev=1909903&r1=1909902&r2=1909903&view=diff
==============================================================================
--- serf/branches/1.3.x/test/server/test_server.h (original)
+++ serf/branches/1.3.x/test/server/test_server.h Thu May 18 09:32:30 2023
@@ -115,6 +115,7 @@ struct serv_ctx_t {
     void *ssl_ctx;
     const char *client_cn;
     apr_status_t bio_read_status;
+    int hit_eof;
 };
 
 void setup_test_server(serv_ctx_t **servctx_p,

Modified: serf/branches/1.3.x/test/server/test_sslserver.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/test/server/test_sslserver.c?rev=1909903&r1=1909902&r2=1909903&view=diff
==============================================================================
--- serf/branches/1.3.x/test/server/test_sslserver.c (original)
+++ serf/branches/1.3.x/test/server/test_sslserver.c Thu May 18 09:32:30 2023
@@ -35,6 +35,7 @@ static int init_done = 0;
 
 typedef struct ssl_context_t {
     int handshake_done;
+    int hit_eof;
 
     SSL_CTX* ctx;
     SSL* ssl;
@@ -96,21 +97,22 @@ static int bio_apr_socket_destroy(BIO *b
 
 static long bio_apr_socket_ctrl(BIO *bio, int cmd, long num, void *ptr)
 {
-    long ret = 1;
+    serv_ctx_t *serv_ctx = bio_get_data(bio);
+    ssl_context_t *ssl_ctx = serv_ctx->ssl_ctx;
 
     switch (cmd) {
-        default:
-            /* abort(); */
-            break;
         case BIO_CTRL_FLUSH:
             /* At this point we can't force a flush. */
-            break;
+            return 1;
         case BIO_CTRL_PUSH:
         case BIO_CTRL_POP:
-            ret = 0;
-            break;
+            return 0;
+        case BIO_CTRL_EOF:
+            return ssl_ctx->hit_eof;
+        default:
+            /* abort(); */
+            return 0;
     }
-    return ret;
 }
 
 /* Returns the amount read. */
@@ -127,6 +129,10 @@ static int bio_apr_socket_read(BIO *bio,
     serf__log_skt(TEST_VERBOSE, __FILE__, serv_ctx->client_sock,
                   "Read %d bytes from socket with status %d.\n", len, status);
 
+    if (APR_STATUS_IS_EOF(status)) {
+        serv_ctx->hit_eof = 1;
+    }
+
     if (status == APR_EAGAIN) {
         BIO_set_retry_read(bio);
         if (len == 0)
@@ -305,6 +311,7 @@ static apr_status_t ssl_reset(serv_ctx_t
     serf__log(TEST_VERBOSE, __FILE__, "Reset ssl context.\n");
 
     ssl_ctx->handshake_done = 0;
+    ssl_ctx->hit_eof = 0;
     if (ssl_ctx)
         SSL_clear(ssl_ctx->ssl);
     init_ssl(serv_ctx);