commit dd04fc35c665976f9fc9ff586cbf7fe34d9cc241 Author: Nick Mathewson nickm@torproject.org Date: Thu Aug 23 09:32:20 2018 -0400
Remove tor_tls_shutdown()
This function was supposed to implement a half-duplex mode for our TLS connections. However, nothing in Tor actually uses it (besides some unit tests), and the implementation looks really questionable to me. It's probably best to remove it. We can add a tested one later if we need one in the future. --- src/lib/tls/tortls.h | 1 - src/lib/tls/tortls_nss.c | 9 ---- src/lib/tls/tortls_openssl.c | 63 --------------------------- src/test/test_tortls_openssl.c | 96 ------------------------------------------ 4 files changed, 169 deletions(-)
diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index 7bbb42b2f..a8bc7370a 100644 --- a/src/lib/tls/tortls.h +++ b/src/lib/tls/tortls.h @@ -111,7 +111,6 @@ int tor_tls_finish_handshake(tor_tls_t *tls); void tor_tls_unblock_renegotiation(tor_tls_t *tls); void tor_tls_block_renegotiation(tor_tls_t *tls); void tor_tls_assert_renegotiation_unblocked(tor_tls_t *tls); -int tor_tls_shutdown(tor_tls_t *tls); int tor_tls_get_pending_bytes(tor_tls_t *tls); size_t tor_tls_get_forced_write_size(tor_tls_t *tls);
diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index d2b81ad08..671c01847 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -534,15 +534,6 @@ tor_tls_assert_renegotiation_unblocked(tor_tls_t *tls) }
int -tor_tls_shutdown(tor_tls_t *tls) -{ - tor_assert(tls); - /* XXXX This is not actually used, so I'm not implementing it. We can - * XXXX remove this function entirely someday. */ - return -1; -} - -int tor_tls_get_pending_bytes(tor_tls_t *tls) { tor_assert(tls); diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index c4e9e7770..a9bab67a0 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -1323,69 +1323,6 @@ tor_tls_finish_handshake(tor_tls_t *tls) return r; }
-/** Shut down an open tls connection <b>tls</b>. When finished, returns - * TOR_TLS_DONE. On failure, returns TOR_TLS_ERROR, TOR_TLS_WANTREAD, - * or TOR_TLS_WANTWRITE. - */ -int -tor_tls_shutdown(tor_tls_t *tls) -{ - int r, err; - char buf[128]; - tor_assert(tls); - tor_assert(tls->ssl); - check_no_tls_errors(); - - while (1) { - if (tls->state == TOR_TLS_ST_SENTCLOSE) { - /* If we've already called shutdown once to send a close message, - * we read until the other side has closed too. - */ - do { - r = SSL_read(tls->ssl, buf, 128); - } while (r>0); - err = tor_tls_get_error(tls, r, CATCH_ZERO, "reading to shut down", - LOG_INFO, LD_NET); - if (err == TOR_TLS_ZERORETURN_) { - tls->state = TOR_TLS_ST_GOTCLOSE; - /* fall through... */ - } else { - return err; - } - } - - r = SSL_shutdown(tls->ssl); - if (r == 1) { - /* If shutdown returns 1, the connection is entirely closed. */ - tls->state = TOR_TLS_ST_CLOSED; - return TOR_TLS_DONE; - } - err = tor_tls_get_error(tls, r, CATCH_SYSCALL|CATCH_ZERO, "shutting down", - LOG_INFO, LD_NET); - if (err == TOR_TLS_SYSCALL_) { - /* The underlying TCP connection closed while we were shutting down. */ - tls->state = TOR_TLS_ST_CLOSED; - return TOR_TLS_DONE; - } else if (err == TOR_TLS_ZERORETURN_) { - /* The TLS connection says that it sent a shutdown record, but - * isn't done shutting down yet. Make sure that this hasn't - * happened before, then go back to the start of the function - * and try to read. - */ - if (tls->state == TOR_TLS_ST_GOTCLOSE || - tls->state == TOR_TLS_ST_SENTCLOSE) { - log_warn(LD_NET, - "TLS returned "half-closed" value while already half-closed"); - return TOR_TLS_ERROR_MISC; - } - tls->state = TOR_TLS_ST_SENTCLOSE; - /* fall through ... */ - } else { - return err; - } - } /* end loop */ -} - /** Return true iff this TLS connection is authenticated. */ int diff --git a/src/test/test_tortls_openssl.c b/src/test/test_tortls_openssl.c index 12a05b303..b7e28f376 100644 --- a/src/test/test_tortls_openssl.c +++ b/src/test/test_tortls_openssl.c @@ -1710,102 +1710,6 @@ dummy_handshake_func(SSL *s) return 1; }
-static void -test_tortls_shutdown(void *ignored) -{ - (void)ignored; - int ret; - tor_tls_t *tls; - SSL_METHOD *method = give_me_a_test_method(); - setup_capture_of_logs(LOG_WARN); - - tls = tor_malloc_zero(sizeof(tor_tls_t)); - tls->ssl = tor_malloc_zero(sizeof(SSL)); - tls->ssl->method = method; - method->ssl_read = fixed_ssl_read; - method->ssl_shutdown = fixed_ssl_shutdown; - - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, -9); - - tls->state = TOR_TLS_ST_SENTCLOSE; - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = -1; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, -9); - -#ifndef LIBRESSL_VERSION_NUMBER - tls->ssl->handshake_func = dummy_handshake_func; - - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = 42; - fixed_ssl_read_result[2] = 0; - fixed_ssl_shutdown_result = 1; - ERR_clear_error(); - tls->ssl->version = SSL2_VERSION; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, TOR_TLS_DONE); - tt_int_op(tls->state, OP_EQ, TOR_TLS_ST_CLOSED); - - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = 42; - fixed_ssl_read_result[2] = 0; - fixed_ssl_shutdown_result = 0; - ERR_clear_error(); - tls->ssl->version = 0; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, TOR_TLS_DONE); - tt_int_op(tls->state, OP_EQ, TOR_TLS_ST_CLOSED); - - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = 42; - fixed_ssl_read_result[2] = 0; - fixed_ssl_shutdown_result = 0; - ERR_clear_error(); - tls->ssl->version = 0; - method->ssl_shutdown = setting_version_ssl_shutdown; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, TOR_TLS_ERROR_MISC); - - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = 42; - fixed_ssl_read_result[2] = 0; - fixed_ssl_shutdown_result = 0; - fixed_tls = tls; - fixed_ssl_state_to_set = TOR_TLS_ST_GOTCLOSE; - ERR_clear_error(); - tls->ssl->version = 0; - method->ssl_shutdown = setting_version_and_state_ssl_shutdown; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, TOR_TLS_ERROR_MISC); - - fixed_ssl_read_result_index = 0; - fixed_ssl_read_result[0] = 10; - fixed_ssl_read_result[1] = 42; - fixed_ssl_read_result[2] = 0; - fixed_ssl_read_result[3] = -1; - fixed_ssl_shutdown_result = 0; - fixed_tls = tls; - fixed_ssl_state_to_set = 0; - ERR_clear_error(); - tls->ssl->version = 0; - method->ssl_shutdown = setting_version_and_state_ssl_shutdown; - ret = tor_tls_shutdown(tls); - tt_int_op(ret, OP_EQ, TOR_TLS_ERROR_MISC); -#endif /* !defined(LIBRESSL_VERSION_NUMBER) */ - - done: - teardown_capture_of_logs(); - tor_free(method); - tor_free(tls->ssl); - tor_free(tls); -} - static int negotiated_callback_called;
static void