[tor-commits] [tor/master] Add a tor_tls_release_socket() function.

[nickm at torproject.org]

commit ae5692994fc31cc5fa25fb5681e59e326e6c5dbe
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Sep 12 11:02:59 2018 -0400

    Add a tor_tls_release_socket() function.
    
    This function tells the underlying TLS object that it shouldn't
    close the fd on exit.  Mostly, we hope not to have to use it, since
    the NSS implementation is kludgey, but it should allow us to fix
---
 src/lib/tls/tortls.h         |  1 +
 src/lib/tls/tortls_nss.c     | 37 +++++++++++++++++++++++++++++++++++++
 src/lib/tls/tortls_openssl.c | 24 +++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h
index 459192708..3f1098bba 100644
--- a/src/lib/tls/tortls.h
+++ b/src/lib/tls/tortls.h
@@ -94,6 +94,7 @@ void tor_tls_set_renegotiate_callback(tor_tls_t *tls,
                                       void (*cb)(tor_tls_t *, void *arg),
                                       void *arg);
 int tor_tls_is_server(tor_tls_t *tls);
+void tor_tls_release_socket(tor_tls_t *tls);
 void tor_tls_free_(tor_tls_t *tls);
 #define tor_tls_free(tls) FREE_AND_NULL(tor_tls_t, tor_tls_free_, (tls))
 int tor_tls_peer_has_cert(tor_tls_t *tls);
diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c
index 53adfedf3..1b2032764 100644
--- a/src/lib/tls/tortls_nss.c
+++ b/src/lib/tls/tortls_nss.c
@@ -414,6 +414,43 @@ tor_tls_set_renegotiate_callback(tor_tls_t *tls,
   /* We don't support renegotiation-based TLS with NSS. */
 }
 
+/**
+ * Tell the TLS library that the underlying socket for <b>tls</b> has been
+ * closed, and the library should not attempt to free that socket itself.
+ */
+void
+tor_tls_release_socket(tor_tls_t *tls)
+{
+  if (! tls)
+    return;
+
+  /* NSS doesn't have the equivalent of BIO_NO_CLOSE.  If you replace the
+   * fd with something that's invalid, it causes a memory leak in PR_Close.
+   *
+   * If there were a way to put the PRFileDesc into the CLOSED state, that
+   * would prevent it from closing its fd -- but there doesn't seem to be a
+   * supported way to do that either.
+   *
+   * So instead: we make a new sacrificial socket, and replace the original
+   * socket with that one. This seems to be the best we can do, until we
+   * redesign the mainloop code enough to make this function unnecessary.
+   */
+  tor_socket_t sock =
+    tor_open_socket_nonblocking(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  if (!sock) {
+    log_warn(LD_NET, "Out of sockets when trying to shut down an NSS "
+             "connection");
+    return;
+  }
+
+  PRFileDesc *tcp = PR_GetIdentitiesLayer(tls->ssl, PR_NSPR_IO_LAYER);
+  if (BUG(! tcp)) {
+    return;
+  }
+
+  PR_ChangeFileDescNativeHandle(tcp, sock);
+}
+
 void
 tor_tls_impl_free_(tor_tls_impl_t *tls)
 {
diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c
index dc6c0bee9..534a90de5 100644
--- a/src/lib/tls/tortls_openssl.c
+++ b/src/lib/tls/tortls_openssl.c
@@ -1048,7 +1048,7 @@ tor_tls_new(tor_socket_t sock, int isServer)
     goto err;
   }
   result->socket = sock;
-  bio = BIO_new_socket(sock, BIO_NOCLOSE);
+  bio = BIO_new_socket(sock, BIO_CLOSE);
   if (! bio) {
     tls_log_errors(NULL, LOG_WARN, LD_NET, "opening BIO");
 #ifdef SSL_set_tlsext_host_name
@@ -1154,6 +1154,28 @@ tor_tls_assert_renegotiation_unblocked(tor_tls_t *tls)
 #endif /* defined(SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) && ... */
 }
 
+/**
+ * Tell the TLS library that the underlying socket for <b>tls</b> has been
+ * closed, and the library should not attempt to free that socket itself.
+ */
+void
+tor_tls_release_socket(tor_tls_t *tls)
+{
+  if (! tls)
+    return;
+
+  BIO *rbio, *wbio;
+  rbio = SSL_get_rbio(tls->ssl);
+  wbio = SSL_get_wbio(tls->ssl);
+
+  if (rbio) {
+    BIO_set_close(rbio, BIO_NOCLOSE);
+  }
+  if (wbio && wbio != rbio) {
+    BIO_set_close(wbio, BIO_NOCLOSE);
+  }
+}
+
 void
 tor_tls_impl_free_(tor_tls_impl_t *ssl)
 {