diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 7ef7423b13..0c3abc8442 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -638,8 +638,19 @@ connection_free_minimal(connection_t *conn) if (connection_speaks_cells(conn)) { or_connection_t *or_conn = TO_OR_CONN(conn); - tor_tls_free(or_conn->tls); - or_conn->tls = NULL; + if (or_conn->tls) { + if (! SOCKET_OK(conn->s)) { + /* The socket has been closed by somebody else; we must tell the + * TLS object not to close it. */ + tor_tls_release_socket(or_conn->tls); + } else { + /* The tor_tls_free() call below will close the socket; we must tell + * the code below not to close it a second time. */ + conn->s = TOR_INVALID_SOCKET; + } + tor_tls_free(or_conn->tls); + or_conn->tls = NULL; + } or_handshake_state_free(or_conn->handshake_state); or_conn->handshake_state = NULL; tor_free(or_conn->nickname); diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index 4591927081..3f1098bbac 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 53adfedf32..1b2032764d 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 tls 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 dc6c0bee9c..534a90de5d 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 tls 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) {