Browse Source

TLS: start shutdown only when peer did not already close

- When curl sees a TCP close from the peer, do not start a TLS shutdown.
  TLS shutdown is a handshake and if the peer already closed the
  connection, it is not interested in participating.

Reported-by: dfdity on github
Assisted-by: Jiří Bok
Assisted-by: Pēteris Caune
Fixes #10290
Closes #13087
Stefan Eissing 1 month ago
parent
commit
c765b04d11
6 changed files with 44 additions and 7 deletions
  1. 1 1
      lib/cw-out.c
  2. 3 1
      lib/vtls/gtls.c
  3. 31 3
      lib/vtls/openssl.c
  4. 4 1
      lib/vtls/rustls.c
  5. 1 0
      lib/vtls/vtls_int.h
  6. 4 1
      lib/vtls/wolfssl.c

+ 1 - 1
lib/cw-out.c

@@ -231,7 +231,7 @@ static CURLcode cw_out_ptr_flush(struct cw_out_ctx *ctx,
     }
     if(nwritten != wlen) {
       failf(data, "Failure writing output to destination, "
-            "passed %zu returned %zu", wlen, nwritten);
+            "passed %zu returned %zd", wlen, nwritten);
       return CURLE_WRITE_ERROR;
     }
     *pconsumed += nwritten;

+ 3 - 1
lib/vtls/gtls.c

@@ -117,6 +117,8 @@ static ssize_t gtls_pull(void *s, void *buf, size_t blen)
                                (CURLE_AGAIN == result)? EAGAIN : EINVAL);
     nread = -1;
   }
+  else if(nread == 0)
+    connssl->peer_closed = TRUE;
   return nread;
 }
 
@@ -1489,7 +1491,7 @@ static int gtls_shutdown(struct Curl_cfilter *cf,
     bool done = FALSE;
     char buf[120];
 
-    while(!done) {
+    while(!done && !connssl->peer_closed) {
       int what = SOCKET_READABLE(Curl_conn_cf_get_socket(cf, data),
                                  SSL_SHUTDOWN_TIMEOUT);
       if(what > 0) {

+ 31 - 3
lib/vtls/openssl.c

@@ -769,6 +769,9 @@ static int ossl_bio_cf_in_read(BIO *bio, char *buf, int blen)
     if(CURLE_AGAIN == result)
       BIO_set_retry_read(bio);
   }
+  else if(nread == 0) {
+    connssl->peer_closed = TRUE;
+  }
 
   /* Before returning server replies to the SSL instance, we need
    * to have setup the x509 store or verification will fail. */
@@ -1887,16 +1890,41 @@ static void ossl_close(struct Curl_cfilter *cf, struct Curl_easy *data)
   DEBUGASSERT(backend);
 
   if(backend->handle) {
-    if(cf->next && cf->next->connected) {
+    /* Send the TLS shutdown if we are still connected *and* if
+     * the peer did not already close the connection. */
+    if(cf->next && cf->next->connected && !connssl->peer_closed) {
       char buf[1024];
       int nread, err;
       long sslerr;
 
       /* Maybe the server has already sent a close notify alert.
          Read it to avoid an RST on the TCP connection. */
-      (void)SSL_read(backend->handle, buf, (int)sizeof(buf));
       ERR_clear_error();
-      if(SSL_shutdown(backend->handle) == 1) {
+      nread = SSL_read(backend->handle, buf, (int)sizeof(buf));
+      err = SSL_get_error(backend->handle, nread);
+      if(!nread && err == SSL_ERROR_ZERO_RETURN) {
+        CURLcode result;
+        ssize_t n;
+        size_t blen = sizeof(buf);
+        CURL_TRC_CF(data, cf, "peer has shutdown TLS");
+        /* SSL_read() will not longer touch the socket, let's receive
+         * directly from the next filter to see if the underlying
+         * connection has also been closed. */
+        n = Curl_conn_cf_recv(cf->next, data, buf, blen, &result);
+        if(!n) {
+          connssl->peer_closed = TRUE;
+          CURL_TRC_CF(data, cf, "peer closed connection");
+        }
+      }
+      ERR_clear_error();
+      if(connssl->peer_closed) {
+        /* As the peer closed, we do not expect it to read anything more we
+         * may send. It may be harmful, leading to TCP RST and delaying
+         * a lingering close. Just leave. */
+        CURL_TRC_CF(data, cf, "not from sending TLS shutdown on "
+                    "connection closed by peer");
+      }
+      else if(SSL_shutdown(backend->handle) == 1) {
         CURL_TRC_CF(data, cf, "SSL shutdown finished");
       }
       else {

+ 4 - 1
lib/vtls/rustls.c

@@ -86,6 +86,7 @@ static int
 read_cb(void *userdata, uint8_t *buf, uintptr_t len, uintptr_t *out_n)
 {
   struct io_ctx *io_ctx = userdata;
+  struct ssl_connect_data *const connssl = io_ctx->cf->ctx;
   CURLcode result;
   int ret = 0;
   ssize_t nread = Curl_conn_cf_recv(io_ctx->cf->next, io_ctx->data,
@@ -97,6 +98,8 @@ read_cb(void *userdata, uint8_t *buf, uintptr_t len, uintptr_t *out_n)
     else
       ret = EINVAL;
   }
+  else if(nread == 0)
+    connssl->peer_closed = TRUE;
   *out_n = (int)nread;
   return ret;
 }
@@ -697,7 +700,7 @@ cr_close(struct Curl_cfilter *cf, struct Curl_easy *data)
 
   DEBUGASSERT(backend);
 
-  if(backend->conn) {
+  if(backend->conn && !connssl->peer_closed) {
     rustls_connection_send_close_notify(backend->conn);
     n = cr_send(cf, data, NULL, 0, &tmperr);
     if(n < 0) {

+ 1 - 0
lib/vtls/vtls_int.h

@@ -76,6 +76,7 @@ struct ssl_connect_data {
   int port;                         /* remote port at origin */
   BIT(use_alpn);                    /* if ALPN shall be used in handshake */
   BIT(reused_session);              /* session-ID was reused for this */
+  BIT(peer_closed);                 /* peer has closed connection */
 };
 
 

+ 4 - 1
lib/vtls/wolfssl.c

@@ -321,6 +321,8 @@ static int wolfssl_bio_cf_in_read(WOLFSSL_BIO *bio, char *buf, int blen)
   wolfSSL_BIO_clear_retry_flags(bio);
   if(nread < 0 && CURLE_AGAIN == result)
     BIO_set_retry_read(bio);
+  else if(nread == 0)
+    connssl->peer_closed = TRUE;
   return (int)nread;
 }
 
@@ -1059,7 +1061,8 @@ static void wolfssl_close(struct Curl_cfilter *cf, struct Curl_easy *data)
     /* Maybe the server has already sent a close notify alert.
        Read it to avoid an RST on the TCP connection. */
     (void)wolfSSL_read(backend->handle, buf, (int)sizeof(buf));
-    (void)wolfSSL_shutdown(backend->handle);
+    if(!connssl->peer_closed)
+      (void)wolfSSL_shutdown(backend->handle);
     wolfSSL_free(backend->handle);
     backend->handle = NULL;
   }