Merge pull request #9295 from rizlik/shutdown_nonblocking_fix

wolfSSL_shutdown: handle non-blocking I/O

Author: dgarske

src/internal.c

@@ -26433,20 +26433,6 @@ static int SendAlert_ex(WOLFSSL* ssl, int severity, int type)
         }
     #endif
 
-    /*
-     * We check if we are trying to send a
-     * CLOSE_NOTIFY alert.
-     * */
-    if (type == close_notify) {
-        if (!ssl->options.sentNotify) {
-            ssl->options.sentNotify = 1;
-        }
-        else {
-            /* CLOSE_NOTIFY already sent */
-            return 0;
-        }
-    }
-
     ssl->buffers.outputBuffer.length += (word32)sendSz;
 
     ret = SendBuffered(ssl);

src/ssl.c

@@ -4471,15 +4471,53 @@ int wolfSSL_shutdown(WOLFSSL* ssl)
         ret = WOLFSSL_SUCCESS;
     }
     else {
+
+        /* Try to flush the buffer first, it might contain the alert */
+        if (ssl->error == WC_NO_ERR_TRACE(WANT_WRITE) &&
+            ssl->buffers.outputBuffer.length > 0) {
+            ret = SendBuffered(ssl);
+            if (ret != 0) {
+                ssl->error = ret;
+                /* for error tracing */
+                if (ret != WC_NO_ERR_TRACE(WANT_WRITE))
+                    WOLFSSL_ERROR(ret);
+                ret = WOLFSSL_FATAL_ERROR;
+                WOLFSSL_LEAVE("wolfSSL_shutdown", ret);
+                return ret;
+            }
+
+            ssl->error = WOLFSSL_ERROR_NONE;
+            /* we succeeded in sending the alert now */
+            if (ssl->options.sentNotify)  {
+                /* just after we send the alert, if we didn't receive the alert
+                 * from the other peer yet, return WOLFSSL_STHUDOWN_NOT_DONE */
+                if (!ssl->options.closeNotify) {
+                    ret = WOLFSSL_SHUTDOWN_NOT_DONE;
+                    WOLFSSL_LEAVE("wolfSSL_shutdown", ret);
+                    return ret;
+                }
+                else {
+                    ssl->options.shutdownDone = 1;
+                    ret = WOLFSSL_SUCCESS;
+                }
+            }
+        }
+
         /* try to send close notify, not an error if can't */
         if (!ssl->options.isClosed && !ssl->options.connReset &&
                                       !ssl->options.sentNotify) {
             ssl->error = SendAlert(ssl, alert_warning, close_notify);
+
+            /* the alert is now sent or sitting in the buffer,
+             * where will be sent eventually */
+            if (ssl->error == 0 || ssl->error == WC_NO_ERR_TRACE(WANT_WRITE))
+                ssl->options.sentNotify = 1;
+
             if (ssl->error < 0) {
                 WOLFSSL_ERROR(ssl->error);
                 return WOLFSSL_FATAL_ERROR;
             }
-            ssl->options.sentNotify = 1;  /* don't send close_notify twice */
+
             if (ssl->options.closeNotify) {
                 ret = WOLFSSL_SUCCESS;
                 ssl->options.shutdownDone = 1;
@@ -4499,7 +4537,7 @@ int wolfSSL_shutdown(WOLFSSL* ssl)
         }
 #endif
 
-        /* call wolfSSL_shutdown again for bidirectional shutdown */
+        /* wolfSSL_shutdown called again for bidirectional shutdown */
         if (ssl->options.sentNotify && !ssl->options.closeNotify) {
             ret = ProcessReply(ssl);
             if ((ret == WC_NO_ERR_TRACE(ZERO_RETURN)) ||
@@ -4509,11 +4547,18 @@ int wolfSSL_shutdown(WOLFSSL* ssl)
                 /* Clear error */
                 ssl->error = WOLFSSL_ERROR_NONE;
                 ret = WOLFSSL_SUCCESS;
-            } else if (ret == WC_NO_ERR_TRACE(MEMORY_E)) {
+            }
+            else if (ret == WC_NO_ERR_TRACE(MEMORY_E)) {
                 ret = WOLFSSL_FATAL_ERROR;
-            } else if (ssl->error == WOLFSSL_ERROR_NONE) {
+            }
+            else if (ret == WC_NO_ERR_TRACE(WANT_READ)) {
+                ssl->error = ret;
+                ret = WOLFSSL_FATAL_ERROR;
+            }
+            else if (ssl->error == WOLFSSL_ERROR_NONE) {
                 ret = WOLFSSL_SHUTDOWN_NOT_DONE;
-            } else {
+            }
+            else {
                 WOLFSSL_ERROR(ssl->error);
                 ret = WOLFSSL_FATAL_ERROR;
             }

tests/api.c

@@ -46751,25 +46751,12 @@ static int test_extra_alerts_bad_psk(void)
 }
 #endif
 
-#if defined(OPENSSL_EXTRA) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12)
-/*
- * Emulates wolfSSL_shutdown that goes on EAGAIN,
- * by returning on output WOLFSSL_ERROR_WANT_WRITE.*/
-static int custom_wolfSSL_shutdown(WOLFSSL *ssl, char *buf,
-        int sz, void *ctx)
-{
-    (void)ssl;
-    (void)buf;
-    (void)ctx;
-    (void)sz;
-
-    return WOLFSSL_CBIO_ERR_WANT_WRITE;
-}
-
-static int test_multiple_alerts_EAGAIN(void)
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12)
+static int test_multiple_shutdown_nonblocking(void)
 {
     EXPECT_DECLS;
     size_t size_of_last_packet = 0;
+    int dummy_recv_buffer;
 
     /* declare wolfSSL objects */
     struct test_memio_ctx test_ctx;
@@ -46779,46 +46766,68 @@ static int test_multiple_alerts_EAGAIN(void)
     XMEMSET(&test_ctx, 0, sizeof(test_ctx));
 
     /* Create and initialize WOLFSSL_CTX and WOLFSSL objects */
-#ifdef USE_TLSV13
-    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
-          wolfTLSv1_3_client_method,  wolfTLSv1_3_server_method), 0);
-#else
     ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
          wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
-#endif
+
     ExpectNotNull(ctx_c);
     ExpectNotNull(ssl_c);
     ExpectNotNull(ctx_s);
     ExpectNotNull(ssl_s);
 
-    /* Load client certificates into WOLFSSL_CTX */
-    ExpectIntEQ(wolfSSL_CTX_load_verify_locations(ctx_c, "./certs/ca-cert.pem", NULL), WOLFSSL_SUCCESS);
-
     ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
 
-    /*
-     * We set the custom callback for the IO to emulate multiple EAGAINs
-     * on shutdown, so we can check that we don't send multiple packets.
-     * */
-    wolfSSL_SSLSetIOSend(ssl_c, custom_wolfSSL_shutdown);
+    /* buffers should be empty now */
+    ExpectIntEQ(test_ctx.c_len, 0);
+    ExpectIntEQ(test_ctx.s_len, 0);
+    ExpectIntEQ(ssl_c->buffers.outputBuffer.length, 0);
+
+    test_memio_simulate_want_write(&test_ctx, 0, 1);
 
     /*
-     * We call wolfSSL_shutdown multiple times to reproduce the behaviour,
-     * to check that it doesn't add the CLOSE_NOTIFY packet multiple times
-     * on the output buffer.
+     * We call wolfSSL_shutdown multiple times to  to check that it doesn't add
+     * the CLOSE_NOTIFY packet multiple times on the output buffer.
      * */
-    wolfSSL_shutdown(ssl_c);
-    wolfSSL_shutdown(ssl_c);
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, 0), WOLFSSL_ERROR_WANT_WRITE);
 
+    /* store the size of the packet */
     if (ssl_c != NULL) {
         size_of_last_packet = ssl_c->buffers.outputBuffer.length;
     }
-    wolfSSL_shutdown(ssl_c);
 
-    /*
-     * Finally we check the length of the output buffer.
-     * */
-    ExpectIntEQ((ssl_c->buffers.outputBuffer.length - size_of_last_packet), 0);
+    /* invoke it multiple times shouldn't change the wolfssl internal output buffer size */
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, 0), WOLFSSL_ERROR_WANT_WRITE);
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, 0), WOLFSSL_ERROR_WANT_WRITE);
+
+    ExpectIntEQ(ssl_c->buffers.outputBuffer.length, size_of_last_packet);
+
+    /* now send the CLOSE_NOTIFY to the server for real, expecting shutdown not done */
+    test_memio_simulate_want_write(&test_ctx, 0, 0);
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), WOLFSSL_SHUTDOWN_NOT_DONE);
+
+    /* output buffer should be empty and socket buffer should contain the message */
+    ExpectIntEQ(ssl_c->buffers.outputBuffer.length, 0);
+    ExpectIntEQ(test_ctx.s_len, size_of_last_packet);
+
+
+    /* this should try to read from the socket */
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+
+    /* complete the bidirectional shutdown */
+
+    /* check that server received the shutdown alert */
+    ExpectIntEQ(wolfSSL_recv(ssl_s, &dummy_recv_buffer, 0, 0), 0);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, 0), WOLFSSL_ERROR_ZERO_RETURN);
+
+    /* send the shutdown from the server side */
+    ExpectIntEQ(wolfSSL_shutdown(ssl_s), WOLFSSL_SUCCESS);
+
+    /* This should return success and zero return */
+    ExpectIntEQ(wolfSSL_shutdown(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, 0), WOLFSSL_ERROR_ZERO_RETURN);
 
     /* Cleanup and return */
     wolfSSL_CTX_free(ctx_c);
@@ -46829,7 +46838,7 @@ static int test_multiple_alerts_EAGAIN(void)
     return EXPECT_RESULT();
 }
 #else
-static int test_multiple_alerts_EAGAIN(void)
+static int test_multiple_shutdown_nonblocking(void)
 {
     return TEST_SKIPPED;
 }
@@ -51365,7 +51374,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_extra_alerts_wrong_cs),
     TEST_DECL(test_extra_alerts_skip_hs),
     TEST_DECL(test_extra_alerts_bad_psk),
-    TEST_DECL(test_multiple_alerts_EAGAIN),
+    TEST_DECL(test_multiple_shutdown_nonblocking),
     /* Can't memory test as client/server Asserts. */
     TEST_DECL(test_harden_no_secure_renegotiation),
     TEST_DECL(test_override_alt_cert_chain),

tests/utils.c

@@ -48,6 +48,7 @@ int test_memio_write_cb(WOLFSSL *ssl, char *data, int sz, void *ctx)
     int *len;
     int *msg_sizes;
     int *msg_count;
+    int *forceWantWrite;
 
     test_ctx = (struct test_memio_ctx*)ctx;
 
@@ -56,14 +57,19 @@ int test_memio_write_cb(WOLFSSL *ssl, char *data, int sz, void *ctx)
         len = &test_ctx->c_len;
         msg_sizes = test_ctx->c_msg_sizes;
         msg_count = &test_ctx->c_msg_count;
+        forceWantWrite = &test_ctx->c_force_want_write;
     }
     else {
         buf = test_ctx->s_buff;
         len = &test_ctx->s_len;
         msg_sizes = test_ctx->s_msg_sizes;
         msg_count = &test_ctx->s_msg_count;
+        forceWantWrite = &test_ctx->s_force_want_write;
     }
 
+    if (*forceWantWrite)
+        return WOLFSSL_CBIO_ERR_WANT_WRITE;
+
     if ((unsigned)(*len + sz) > TEST_MEMIO_BUF_SZ)
         return WOLFSSL_CBIO_ERR_WANT_WRITE;
 
@@ -362,16 +368,30 @@ int test_memio_setup_ex(struct test_memio_ctx *ctx,
     return 0;
 }
 
+void test_memio_simulate_want_write(struct test_memio_ctx *ctx, int is_client,
+        int enable)
+{
+    if (ctx == NULL)
+        return;
+
+    if (is_client)
+        ctx->c_force_want_write = (enable != 0);
+    else
+        ctx->s_force_want_write = (enable != 0);
+}
+
 void test_memio_clear_buffer(struct test_memio_ctx *ctx, int is_client)
 {
     if (is_client) {
         ctx->c_len = 0;
         ctx->c_msg_pos = 0;
         ctx->c_msg_count = 0;
+        ctx->c_force_want_write = 0;
     } else {
         ctx->s_len = 0;
         ctx->s_msg_pos = 0;
         ctx->s_msg_count = 0;
+        ctx->s_force_want_write = 0;
     }
 }
 

tests/utils.h

@@ -44,6 +44,9 @@ struct test_memio_ctx
     int s_len;
     const char* s_ciphers;
 
+    int c_force_want_write;
+    int s_force_want_write;
+
     int c_msg_sizes[TEST_MEMIO_MAX_MSGS];
     int c_msg_count;
     int c_msg_pos;
@@ -64,6 +67,8 @@ int test_memio_setup_ex(struct test_memio_ctx *ctx,
     method_provider method_c, method_provider method_s,
     byte *caCert, int caCertSz, byte *serverCert, int serverCertSz,
     byte *serverKey, int serverKeySz);
+void test_memio_simulate_want_write(struct test_memio_ctx *ctx, int is_client,
+        int enable);
 void test_memio_clear_buffer(struct test_memio_ctx *ctx, int is_client);
 int test_memio_inject_message(struct test_memio_ctx *ctx, int client, const char *data, int sz);
 int test_memio_copy_message(const struct test_memio_ctx *ctx, int client,