From 50fd99d7ef01f74fd794eb430cf28b5c84e48446 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:19 -0500 Subject: [PATCH 01/16] Revert "Set renegotiation callbacks immediately on tls inititation" This reverts commit e27a26d568a257cf350814a9abfa47d3b41ad9f3. --- src/common/tortls.c | 2 ++ src/or/connection_or.c | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 18f2684707..a6947c87d8 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1573,6 +1573,7 @@ tor_tls_set_renegotiate_callbacks(tor_tls_t *tls, tls->excess_renegotiations_callback = cb2; tls->callback_arg = arg; tls->got_renegotiate = 0; + SSL_set_info_callback(tls->ssl, tor_tls_state_changed_callback); } /** If this version of openssl requires it, turn on renegotiation on @@ -1776,6 +1777,7 @@ tor_tls_finish_handshake(tor_tls_t *tls) { int r = TOR_TLS_DONE; if (tls->isServer) { + SSL_set_info_callback(tls->ssl, NULL); SSL_set_verify(tls->ssl, SSL_VERIFY_PEER, always_accept_verify_cb); /* There doesn't seem to be a clear OpenSSL API to clear mode flags. */ tls->ssl->mode &= ~SSL_MODE_NO_AUTO_CHAIN; diff --git a/src/or/connection_or.c b/src/or/connection_or.c index b865e13664..6082be4121 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -42,7 +42,6 @@ static int connection_or_check_valid_tls_handshake(or_connection_t *conn, char *digest_rcvd_out); static void connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn); -static void connection_or_close_connection_cb(void *_conn); #ifdef USE_BUFFEREVENTS static void connection_or_handle_event_cb(struct bufferevent *bufev, @@ -1103,16 +1102,12 @@ connection_tls_start_handshake(or_connection_t *conn, int receiving) conn->_base.state = OR_CONN_STATE_TLS_HANDSHAKING; tor_assert(!conn->tls); conn->tls = tor_tls_new(conn->_base.s, receiving); + tor_tls_set_logged_address(conn->tls, // XXX client and relay? + escaped_safe_str(conn->_base.address)); if (!conn->tls) { log_warn(LD_BUG,"tor_tls_new failed. Closing."); return -1; } - tor_tls_set_logged_address(conn->tls, // XXX client and relay? - escaped_safe_str(conn->_base.address)); - tor_tls_set_renegotiate_callbacks(conn->tls, - connection_or_tls_renegotiated_cb, - connection_or_close_connection_cb, - conn); #ifdef USE_BUFFEREVENTS if (connection_type_uses_bufferevent(TO_CONN(conn))) { const int filtering = get_options()->_UseFilteringSSLBufferevents; @@ -1238,6 +1233,10 @@ connection_tls_continue_handshake(or_connection_t *conn) /* v2/v3 handshake, but not a client. */ log_debug(LD_OR, "Done with initial SSL handshake (server-side). " "Expecting renegotiation or VERSIONS cell"); + tor_tls_set_renegotiate_callbacks(conn->tls, + connection_or_tls_renegotiated_cb, + connection_or_close_connection_cb, + conn); conn->_base.state = OR_CONN_STATE_TLS_SERVER_RENEGOTIATING; connection_stop_writing(TO_CONN(conn)); connection_start_reading(TO_CONN(conn)); @@ -1298,6 +1297,10 @@ connection_or_handle_event_cb(struct bufferevent *bufev, short event, } else if (tor_tls_get_num_server_handshakes(conn->tls) == 1) { /* v2 or v3 handshake, as a server. Only got one handshake, so * wait for the next one. */ + tor_tls_set_renegotiate_callbacks(conn->tls, + connection_or_tls_renegotiated_cb, + connection_or_close_connection_cb, + conn); conn->_base.state = OR_CONN_STATE_TLS_SERVER_RENEGOTIATING; /* return 0; */ return; /* ???? */ From 135a5102a3e5422a1c9c8ad28f58888eea4a2545 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 02/16] Revert "Make pending libevent actions cancelable" This reverts commit aba25a6939a5907d40dbcff7433a8c130ffd12ad. --- src/common/compat_libevent.c | 24 +++++++----------------- src/common/compat_libevent.h | 8 +++----- src/common/tortls.c | 11 +++++++++-- src/or/connection_or.c | 25 ++++--------------------- src/or/or.h | 2 -- 5 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 67f465927c..3a754bef70 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -558,17 +558,17 @@ tor_check_libevent_header_compatibility(void) #endif } -struct tor_libevent_action_t { +typedef struct runnable_t { struct event *ev; void (*cb)(void *arg); void *arg; -}; +} runnable_t; /** Callback for tor_run_in_libevent_loop */ static void run_runnable_cb(evutil_socket_t s, short what, void *arg) { - tor_libevent_action_t *r = arg; + runnable_t *r = arg; void (*cb)(void *) = r->cb; void *cb_arg = r->arg; (void)what; @@ -584,32 +584,22 @@ run_runnable_cb(evutil_socket_t s, short what, void *arg) * deep inside a no-reentrant code and there's some function you want to call * without worrying about whether it might cause reeentrant invocation. */ -tor_libevent_action_t * +int tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg) { - tor_libevent_action_t *r = tor_malloc(sizeof(tor_libevent_action_t)); + runnable_t *r = tor_malloc(sizeof(runnable_t)); r->cb = cb; r->arg = arg; r->ev = tor_event_new(tor_libevent_get_base(), -1, EV_TIMEOUT, run_runnable_cb, r); if (!r->ev) { tor_free(r); - return NULL; + return -1; } /* Make the event active immediately. */ event_active(r->ev, EV_TIMEOUT, 1); - return r; -} - -/** - * Cancel action without running it. - */ -void -tor_cancel_libevent_action(tor_libevent_action_t *action) -{ - tor_event_free(action->ev); - tor_free(action); + return 0; } /* diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 4076cc0e08..3f916d16b0 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -44,12 +44,10 @@ void tor_event_free(struct event *ev); #define tor_evdns_add_server_port evdns_add_server_port #endif -typedef struct tor_libevent_action_t tor_libevent_action_t; -tor_libevent_action_t *tor_run_in_libevent_loop(void (*cb)(void *arg), - void *arg); -void tor_cancel_libevent_action(tor_libevent_action_t *action); - typedef struct periodic_timer_t periodic_timer_t; + +int tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg); + periodic_timer_t *periodic_timer_new(struct event_base *base, const struct timeval *tv, void (*cb)(periodic_timer_t *timer, void *data), diff --git a/src/common/tortls.c b/src/common/tortls.c index a6947c87d8..b4d81de2f3 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1339,9 +1339,16 @@ tor_tls_got_client_hello(tor_tls_t *tls) tls->excess_renegotiations_callback) { /* We got more than one renegotiation requests. The Tor protocol needs just one renegotiation; more than that probably means - They are trying to DoS us and we have to stop them. */ + They are trying to DoS us and we have to stop them. We can't + close their connection from in here since it's an OpenSSL + callback, so we set a libevent timer that triggers in the next + event loop and closes the connection. */ - tls->excess_renegotiations_callback(tls->callback_arg); + if (tor_run_in_libevent_loop(tls->excess_renegotiations_callback, + tls->callback_arg) < 0) { + log_warn(LD_GENERAL, "Didn't manage to set a renegotiation " + "limiting callback."); + } } /* Now check the cipher list. */ diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 6082be4121..82297c3316 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -492,9 +492,6 @@ connection_or_about_to_close(or_connection_t *or_conn) time_t now = time(NULL); connection_t *conn = TO_CONN(or_conn); - if (or_conn->pending_action) - tor_cancel_libevent_action(or_conn->pending_action); - /* Remember why we're closing this connection. */ if (conn->state != OR_CONN_STATE_OPEN) { /* Inform any pending (not attached) circs that they should @@ -1159,34 +1156,20 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) } } -/*DOCDOC*/ +/** Invoked on the server side using a timer from inside + * tor_tls_got_client_hello() when the server receives excess + * renegotiation attempts; probably indicating a DoS. */ static void -close_connection_libevent_cb(void *_conn) +connection_or_close_connection_cb(void *_conn) { or_connection_t *or_conn = _conn; connection_t *conn = TO_CONN(or_conn); - or_conn->pending_action = NULL; - connection_stop_reading(conn); if (!conn->marked_for_close) connection_mark_for_close(conn); } -/* DOCDOC */ -static void -connection_or_close_connection_cb(void *_conn) -{ - /* We can't close their connection from in here since it's an OpenSSL - callback, so we set a libevent event that triggers in the next event - loop and closes the connection. */ - or_connection_t *or_conn = _conn; - if (or_conn->_base.marked_for_close || or_conn->pending_action) - return; - or_conn->pending_action = - tor_run_in_libevent_loop(close_connection_libevent_cb, or_conn); -} - /** Move forward with the tls handshake. If it finishes, hand * conn to connection_tls_finish_handshake(). * diff --git a/src/or/or.h b/src/or/or.h index eb9f060e50..6ff02ee36c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1276,8 +1276,6 @@ typedef struct or_connection_t { unsigned active_circuit_pqueue_last_recalibrated; struct or_connection_t *next_with_same_id; /**< Next connection with same * identity digest as this one. */ - - tor_libevent_action_t *pending_action; } or_connection_t; /** Subtype of connection_t for an "edge connection" -- that is, an entry (ap) From 75134c6c86e54c10fd9e11c4345aadcdabc0f8fb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 03/16] Revert "indent; add comment" This reverts commit 40a87c4c08be0cdd87a3df283f285b3c2a0c8445. --- src/common/tortls.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index b4d81de2f3..9ac5c34f26 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -585,8 +585,6 @@ tor_tls_create_certificate(crypto_pk_env_t *rsa, const char *cname_sign, unsigned int cert_lifetime) { - /* OpenSSL generates self-signed certificates with random 64-bit serial - * numbers, so let's do that too. */ #define SERIAL_NUMBER_SIZE 8 time_t start_time, end_time; @@ -614,12 +612,12 @@ tor_tls_create_certificate(crypto_pk_env_t *rsa, goto error; { /* our serial number is 8 random bytes. */ - if (crypto_rand((char *)serial_tmp, sizeof(serial_tmp)) < 0) - goto error; - if (!(serial_number = BN_bin2bn(serial_tmp, sizeof(serial_tmp), NULL))) - goto error; - if (!(BN_to_ASN1_INTEGER(serial_number, X509_get_serialNumber(x509)))) - goto error; + if (crypto_rand((char *)serial_tmp, sizeof(serial_tmp)) < 0) + goto error; + if (!(serial_number = BN_bin2bn(serial_tmp, sizeof(serial_tmp), NULL))) + goto error; + if (!(BN_to_ASN1_INTEGER(serial_number, X509_get_serialNumber(x509)))) + goto error; } if (!(name = tor_x509_name_new(cname))) From acc1806eb85b9bb3b6afdb4b6b489b5e23b97d94 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 04/16] Revert "Don't schedule excess_renegotiations_callback unless it's set" This reverts commit 617617e21a2d30a86cea9c8f7043333078f2e8f8. --- src/common/tortls.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 9ac5c34f26..9a77bab09c 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1333,8 +1333,7 @@ tor_tls_got_client_hello(tor_tls_t *tls) } tls->got_renegotiate = 1; - } else if (tls->server_handshake_count > 2 && - tls->excess_renegotiations_callback) { + } else if (tls->server_handshake_count > 2) { /* We got more than one renegotiation requests. The Tor protocol needs just one renegotiation; more than that probably means They are trying to DoS us and we have to stop them. We can't From e83e720c8bf1f97c3df2ed272296f6dbcc4b8c24 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 05/16] Revert "use event_free() wrapper; fix bug 4582" This reverts commit 9a88c0cd32df53116a6bbb6b961650943755061c. --- src/common/compat_libevent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 3a754bef70..c3a4746b37 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -573,7 +573,7 @@ run_runnable_cb(evutil_socket_t s, short what, void *arg) void *cb_arg = r->arg; (void)what; (void)s; - tor_event_free(r->ev); + event_free(r->ev); tor_free(r); cb(cb_arg); From 17880e4c0a9670178d0722aea387c6885a28e69a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 06/16] Revert "Fix some wide lines in tortls.c" This reverts commit e8dde3aabd3e1292d381eb4269c6457548dca6b9. --- src/common/tortls.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 9a77bab09c..459251488c 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1343,8 +1343,7 @@ tor_tls_got_client_hello(tor_tls_t *tls) if (tor_run_in_libevent_loop(tls->excess_renegotiations_callback, tls->callback_arg) < 0) { - log_warn(LD_GENERAL, "Didn't manage to set a renegotiation " - "limiting callback."); + log_warn(LD_GENERAL, "Didn't manage to set a renegotiation limiting callback."); } } @@ -1399,6 +1398,7 @@ tor_tls_state_changed_callback(const SSL *ssl, int type, int val) tor_tls_got_client_hello(tls); } #endif + } /** Replace *ciphers with a new list of SSL ciphersuites: specifically, @@ -1666,8 +1666,8 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) #ifdef V2_HANDSHAKE_SERVER if (tls->got_renegotiate) { if (tls->server_handshake_count != 2) { - log_warn(LD_BUG, "We did not notice renegotiation in a timely " - "fashion (%u)!", tls->server_handshake_count); + log_warn(LD_BUG, "We did not notice renegotiation in a timely fashion (%u)!", + tls->server_handshake_count); } /* Renegotiation happened! */ From 3a17a1a62f242f3aa64891407d3d64aa040d6d02 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 07/16] Revert "Avoid a double-mark in connection_or_close_connection_cb" This reverts commit 633071eb3bcf2c4106e93de28d727594bd23b1db. --- src/or/connection_or.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 82297c3316..44f559c744 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1162,12 +1162,10 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) static void connection_or_close_connection_cb(void *_conn) { - or_connection_t *or_conn = _conn; - connection_t *conn = TO_CONN(or_conn); + or_connection_t *conn = _conn; - connection_stop_reading(conn); - if (!conn->marked_for_close) - connection_mark_for_close(conn); + connection_stop_reading(TO_CONN(conn)); + connection_mark_for_close(TO_CONN(conn)); } /** Move forward with the tls handshake. If it finishes, hand From df1f72329acf5f555618a5309f2621e584c0d763 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 08/16] Revert "Refactor tor_event_base_once to do what we actually want" This reverts commit 7920ea55b8d994268d2b07f27316b0f34d8f27e5. --- src/common/compat_libevent.c | 45 ++++-------------------------------- src/common/compat_libevent.h | 3 ++- src/common/tortls.c | 10 ++++---- src/common/tortls.h | 2 +- src/common/util.c | 4 ++-- src/or/connection_or.c | 4 +++- 6 files changed, 19 insertions(+), 49 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index c3a4746b37..b709b4afd6 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -558,48 +558,13 @@ tor_check_libevent_header_compatibility(void) #endif } -typedef struct runnable_t { - struct event *ev; - void (*cb)(void *arg); - void *arg; -} runnable_t; - -/** Callback for tor_run_in_libevent_loop */ -static void -run_runnable_cb(evutil_socket_t s, short what, void *arg) -{ - runnable_t *r = arg; - void (*cb)(void *) = r->cb; - void *cb_arg = r->arg; - (void)what; - (void)s; - event_free(r->ev); - tor_free(r); - - cb(cb_arg); -} - -/** Cause cb(arg) to run later on this iteration of the libevent loop, or in - * the next iteration of the libevent loop. This is useful for when you're - * deep inside a no-reentrant code and there's some function you want to call - * without worrying about whether it might cause reeentrant invocation. - */ +/** Wrapper around libevent's event_base_once(). Sets a + * timeout-triggered event with no associated file descriptor. */ int -tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg) +tor_event_base_once(void (*cb)(evutil_socket_t, short, void *), + void *arg, struct timeval *timer) { - runnable_t *r = tor_malloc(sizeof(runnable_t)); - r->cb = cb; - r->arg = arg; - r->ev = tor_event_new(tor_libevent_get_base(), -1, EV_TIMEOUT, - run_runnable_cb, r); - if (!r->ev) { - tor_free(r); - return -1; - } - /* Make the event active immediately. */ - event_active(r->ev, EV_TIMEOUT, 1); - - return 0; + return event_base_once(tor_libevent_get_base(), -1, EV_TIMEOUT, cb, arg, timer); } /* diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 3f916d16b0..897eddbbe0 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -46,7 +46,8 @@ void tor_event_free(struct event *ev); typedef struct periodic_timer_t periodic_timer_t; -int tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg); +int tor_event_base_once(void (*cb)(evutil_socket_t, short, void *), + void *arg, struct timeval *timer); periodic_timer_t *periodic_timer_new(struct event_base *base, const struct timeval *tv, diff --git a/src/common/tortls.c b/src/common/tortls.c index 459251488c..2ef2803db3 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -160,7 +160,7 @@ struct tor_tls_t { /** Callback to invoke whenever a client tries to renegotiate more than once. */ - void (*excess_renegotiations_callback)(void *); + void (*excess_renegotiations_callback)(evutil_socket_t, short, void *); /** Argument to pass to negotiated_callback. */ void *callback_arg; @@ -1341,8 +1341,10 @@ tor_tls_got_client_hello(tor_tls_t *tls) callback, so we set a libevent timer that triggers in the next event loop and closes the connection. */ - if (tor_run_in_libevent_loop(tls->excess_renegotiations_callback, - tls->callback_arg) < 0) { + struct timeval zero_seconds_timer = {0,0}; + + if (tor_event_base_once(tls->excess_renegotiations_callback, + tls->callback_arg, &zero_seconds_timer) < 0) { log_warn(LD_GENERAL, "Didn't manage to set a renegotiation limiting callback."); } } @@ -1570,7 +1572,7 @@ tor_tls_set_logged_address(tor_tls_t *tls, const char *address) void tor_tls_set_renegotiate_callbacks(tor_tls_t *tls, void (*cb)(tor_tls_t *, void *arg), - void (*cb2)(void *), + void (*cb2)(evutil_socket_t, short, void *), void *arg) { tls->negotiated_callback = cb; diff --git a/src/common/tortls.h b/src/common/tortls.h index 9f86e37127..5874881989 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -63,7 +63,7 @@ tor_tls_t *tor_tls_new(int sock, int is_server); void tor_tls_set_logged_address(tor_tls_t *tls, const char *address); void tor_tls_set_renegotiate_callbacks(tor_tls_t *tls, void (*cb)(tor_tls_t *, void *arg), - void (*cb2)(void *), + void (*cb2)(evutil_socket_t, short, void *), void *arg); int tor_tls_is_server(tor_tls_t *tls); void tor_tls_free(tor_tls_t *tls); diff --git a/src/common/util.c b/src/common/util.c index 6d488d9963..a59cab0bbe 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3661,8 +3661,8 @@ tor_get_exit_code(const process_handle_t *process_handle, /* Process has not exited */ return PROCESS_EXIT_RUNNING; } else if (retval != process_handle->pid) { - log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", - process_handle->pid, strerror(errno)); + log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle->pid, + strerror(errno)); return PROCESS_EXIT_ERROR; } diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 44f559c744..ff696f8c31 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1160,9 +1160,11 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) * tor_tls_got_client_hello() when the server receives excess * renegotiation attempts; probably indicating a DoS. */ static void -connection_or_close_connection_cb(void *_conn) +connection_or_close_connection_cb(evutil_socket_t fd, short what, void *_conn) { or_connection_t *conn = _conn; + (void) what; + (void) fd; connection_stop_reading(TO_CONN(conn)); connection_mark_for_close(TO_CONN(conn)); From 53f535aeb863204470379b2da4631770fa10b13f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 09/16] Revert "appease check-spaces" This reverts commit f77f9bddb8bf0dd6e9c3e0d94269aa23f459a272. --- src/common/compat_libevent.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index b709b4afd6..fcc119373a 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -243,8 +243,8 @@ tor_libevent_initialize(tor_libevent_cfg *torcfg) * again. */ #if defined(MS_WINDOWS) && defined(USE_BUFFEREVENTS) if (torcfg->disable_iocp == 0) { - log_warn(LD_GENERAL, "Unable to initialize Libevent. Trying again " - "with IOCP disabled."); + log_warn(LD_GENERAL, "Unable to initialize Libevent. Trying again with " + "IOCP disabled."); } else #endif { @@ -254,6 +254,7 @@ tor_libevent_initialize(tor_libevent_cfg *torcfg) torcfg->disable_iocp = 1; goto retry; } + } #else the_event_base = event_init(); From 616b60cef39f78d8a6ebb984096ff0ec09a3021c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 10/16] Revert "Use callback-driven approach to block renegotiations." This reverts commit 406ae1ba5ad529a4d0e710229dab6ed645d42b50. --- src/common/compat_libevent.c | 9 ------ src/common/compat_libevent.h | 3 -- src/common/tortls.c | 57 ++++++++++++++++++------------------ src/common/tortls.h | 4 +-- src/or/connection_or.c | 20 ++----------- 5 files changed, 31 insertions(+), 62 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index fcc119373a..7a28c9bc9b 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -559,15 +559,6 @@ tor_check_libevent_header_compatibility(void) #endif } -/** Wrapper around libevent's event_base_once(). Sets a - * timeout-triggered event with no associated file descriptor. */ -int -tor_event_base_once(void (*cb)(evutil_socket_t, short, void *), - void *arg, struct timeval *timer) -{ - return event_base_once(tor_libevent_get_base(), -1, EV_TIMEOUT, cb, arg, timer); -} - /* If possible, we're going to try to use Libevent's periodic timer support, since it does a pretty good job of making sure that periodic events get diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 897eddbbe0..0247297177 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -46,9 +46,6 @@ void tor_event_free(struct event *ev); typedef struct periodic_timer_t periodic_timer_t; -int tor_event_base_once(void (*cb)(evutil_socket_t, short, void *), - void *arg, struct timeval *timer); - periodic_timer_t *periodic_timer_new(struct event_base *base, const struct timeval *tv, void (*cb)(periodic_timer_t *timer, void *data), diff --git a/src/common/tortls.c b/src/common/tortls.c index 2ef2803db3..62d34f7ff3 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -52,6 +52,7 @@ #include #include #include +#include "compat_libevent.h" #endif #define CRYPTO_PRIVATE /* to import prototypes from crypto.h */ @@ -63,7 +64,6 @@ #include "torlog.h" #include "container.h" #include -#include "compat_libevent.h" /* Enable the "v2" TLS handshake. */ @@ -157,11 +157,6 @@ struct tor_tls_t { /** If set, a callback to invoke whenever the client tries to renegotiate * the handshake. */ void (*negotiated_callback)(tor_tls_t *tls, void *arg); - - /** Callback to invoke whenever a client tries to renegotiate more - than once. */ - void (*excess_renegotiations_callback)(evutil_socket_t, short, void *); - /** Argument to pass to negotiated_callback. */ void *callback_arg; }; @@ -1265,6 +1260,17 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, return NULL; } +/** Return true if the tls object has completed more + * renegotiations than needed for the Tor protocol. */ +static INLINE int +tor_tls_got_excess_renegotiations(tor_tls_t *tls) +{ + /** The Tor v2 server handshake needs a single renegotiation after + the initial SSL handshake. This means that if we ever see more + than 2 handshakes, we raise the flag. */ + return (tls->server_handshake_count > 2) ? 1 : 0; +} + #ifdef V2_HANDSHAKE_SERVER /** Return true iff the cipher list suggested by the client for ssl is * a list that indicates that the client knows how to do the v2 TLS connection @@ -1333,20 +1339,6 @@ tor_tls_got_client_hello(tor_tls_t *tls) } tls->got_renegotiate = 1; - } else if (tls->server_handshake_count > 2) { - /* We got more than one renegotiation requests. The Tor protocol - needs just one renegotiation; more than that probably means - They are trying to DoS us and we have to stop them. We can't - close their connection from in here since it's an OpenSSL - callback, so we set a libevent timer that triggers in the next - event loop and closes the connection. */ - - struct timeval zero_seconds_timer = {0,0}; - - if (tor_event_base_once(tls->excess_renegotiations_callback, - tls->callback_arg, &zero_seconds_timer) < 0) { - log_warn(LD_GENERAL, "Didn't manage to set a renegotiation limiting callback."); - } } /* Now check the cipher list. */ @@ -1563,20 +1555,16 @@ tor_tls_set_logged_address(tor_tls_t *tls, const char *address) tls->address = tor_strdup(address); } -/** Set cb to be called with argument arg whenever - * tls next gets a client-side renegotiate in the middle of a - * read. Set cb2 to be called with argument arg whenever - * tls gets excess renegotiation requests. Do not invoke this - * function until after initial handshaking is done! +/** Set cb to be called with argument arg whenever tls + * next gets a client-side renegotiate in the middle of a read. Do not + * invoke this function until after initial handshaking is done! */ void -tor_tls_set_renegotiate_callbacks(tor_tls_t *tls, +tor_tls_set_renegotiate_callback(tor_tls_t *tls, void (*cb)(tor_tls_t *, void *arg), - void (*cb2)(evutil_socket_t, short, void *), void *arg) { tls->negotiated_callback = cb; - tls->excess_renegotiations_callback = cb2; tls->callback_arg = arg; tls->got_renegotiate = 0; SSL_set_info_callback(tls->ssl, tor_tls_state_changed_callback); @@ -1636,7 +1624,6 @@ tor_tls_free(tor_tls_t *tls) SSL_free(tls->ssl); tls->ssl = NULL; tls->negotiated_callback = NULL; - tls->excess_renegotiations_callback = NULL; if (tls->context) tor_tls_context_decref(tls->context); tor_free(tls->address); @@ -1665,6 +1652,12 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) err = tor_tls_get_error(tls, r, CATCH_ZERO, "reading", LOG_DEBUG, LD_NET); + if (tor_tls_got_excess_renegotiations(tls)) { + log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); + + return TOR_TLS_ERROR_MISC; + } + #ifdef V2_HANDSHAKE_SERVER if (tls->got_renegotiate) { if (tls->server_handshake_count != 2) { @@ -1719,6 +1712,12 @@ tor_tls_write(tor_tls_t *tls, const char *cp, size_t n) r = SSL_write(tls->ssl, cp, (int)n); err = tor_tls_get_error(tls, r, 0, "writing", LOG_INFO, LD_NET); + if (tor_tls_got_excess_renegotiations(tls)) { + log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); + + return TOR_TLS_ERROR_MISC; + } + if (err == TOR_TLS_DONE) { return r; } diff --git a/src/common/tortls.h b/src/common/tortls.h index 5874881989..e558df756c 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -13,7 +13,6 @@ #include "crypto.h" #include "compat.h" -#include "compat_libevent.h" /* Opaque structure to hold a TLS connection. */ typedef struct tor_tls_t tor_tls_t; @@ -61,9 +60,8 @@ int tor_tls_context_init(int is_public_server, unsigned int key_lifetime); tor_tls_t *tor_tls_new(int sock, int is_server); void tor_tls_set_logged_address(tor_tls_t *tls, const char *address); -void tor_tls_set_renegotiate_callbacks(tor_tls_t *tls, +void tor_tls_set_renegotiate_callback(tor_tls_t *tls, void (*cb)(tor_tls_t *, void *arg), - void (*cb2)(evutil_socket_t, short, void *), void *arg); int tor_tls_is_server(tor_tls_t *tls); void tor_tls_free(tor_tls_t *tls); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ff696f8c31..992db9a40c 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1156,20 +1156,6 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) } } -/** Invoked on the server side using a timer from inside - * tor_tls_got_client_hello() when the server receives excess - * renegotiation attempts; probably indicating a DoS. */ -static void -connection_or_close_connection_cb(evutil_socket_t fd, short what, void *_conn) -{ - or_connection_t *conn = _conn; - (void) what; - (void) fd; - - connection_stop_reading(TO_CONN(conn)); - connection_mark_for_close(TO_CONN(conn)); -} - /** Move forward with the tls handshake. If it finishes, hand * conn to connection_tls_finish_handshake(). * @@ -1216,9 +1202,8 @@ connection_tls_continue_handshake(or_connection_t *conn) /* v2/v3 handshake, but not a client. */ log_debug(LD_OR, "Done with initial SSL handshake (server-side). " "Expecting renegotiation or VERSIONS cell"); - tor_tls_set_renegotiate_callbacks(conn->tls, + tor_tls_set_renegotiate_callback(conn->tls, connection_or_tls_renegotiated_cb, - connection_or_close_connection_cb, conn); conn->_base.state = OR_CONN_STATE_TLS_SERVER_RENEGOTIATING; connection_stop_writing(TO_CONN(conn)); @@ -1280,9 +1265,8 @@ connection_or_handle_event_cb(struct bufferevent *bufev, short event, } else if (tor_tls_get_num_server_handshakes(conn->tls) == 1) { /* v2 or v3 handshake, as a server. Only got one handshake, so * wait for the next one. */ - tor_tls_set_renegotiate_callbacks(conn->tls, + tor_tls_set_renegotiate_callback(conn->tls, connection_or_tls_renegotiated_cb, - connection_or_close_connection_cb, conn); conn->_base.state = OR_CONN_STATE_TLS_SERVER_RENEGOTIATING; /* return 0; */ From 45c46129ed7a53844d6bed5ecda9d15dc8a676aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 11/16] Revert "Fix issues pointed out by nickm." This reverts commit e097bffaed72af6b19f7293722021196bb94de1e. --- src/common/tortls.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 62d34f7ff3..65c26cc515 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1323,21 +1323,17 @@ tor_tls_client_is_using_v2_ciphers(const SSL *ssl, const char *address) return 1; } -/** We got an SSL ClientHello message. This might mean that the - * client wants to initiate a renegotiation and appropriate actions - * must be taken. */ +/** We sent the ServerHello part of an SSL handshake. This might mean + * that we completed a renegotiation and appropriate actions must be + * taken. */ static void -tor_tls_got_client_hello(tor_tls_t *tls) +tor_tls_got_server_hello(tor_tls_t *tls) { if (tls->server_handshake_count < 3) ++tls->server_handshake_count; if (tls->server_handshake_count == 2) { - if (!tls->negotiated_callback) { - log_warn(LD_BUG, "Got a renegotiation request but we don't" - " have a renegotiation callback set!"); - } - + tor_assert(tls->negotiated_callback); tls->got_renegotiate = 1; } @@ -1380,8 +1376,8 @@ tor_tls_state_changed_callback(const SSL *ssl, int type, int val) if (type == SSL_CB_ACCEPT_LOOP && ssl->state == SSL3_ST_SW_SRVR_HELLO_A) { - /* Call tor_tls_got_client_hello() for every SSL ClientHello we - receive. */ + /* Call tor_tls_got_server_hello() for every SSL ServerHello we + send. */ tor_tls_t *tls = tor_tls_get_by_ssl(ssl); if (!tls) { @@ -1389,7 +1385,7 @@ tor_tls_state_changed_callback(const SSL *ssl, int type, int val) return; } - tor_tls_got_client_hello(tls); + tor_tls_got_server_hello(tls); } #endif @@ -1660,10 +1656,8 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) #ifdef V2_HANDSHAKE_SERVER if (tls->got_renegotiate) { - if (tls->server_handshake_count != 2) { - log_warn(LD_BUG, "We did not notice renegotiation in a timely fashion (%u)!", - tls->server_handshake_count); - } + tor_assert(tls->server_handshake_count == 2); + /* XXX tor_assert(err == TOR_TLS_WANTREAD); */ /* Renegotiation happened! */ log_info(LD_NET, "Got a TLS renegotiation from %s", ADDR(tls)); From fa74af0cfa834edbfe5d02ec22fe7c53699770a3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:20 -0500 Subject: [PATCH 12/16] Revert "Also handle needless renegotiations in SSL_write()." This reverts commit e2b3527106e0747f652e2f28fa087d9874e0e2ce. --- src/common/tortls.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 65c26cc515..ddb5ea1efc 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1260,17 +1260,6 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, return NULL; } -/** Return true if the tls object has completed more - * renegotiations than needed for the Tor protocol. */ -static INLINE int -tor_tls_got_excess_renegotiations(tor_tls_t *tls) -{ - /** The Tor v2 server handshake needs a single renegotiation after - the initial SSL handshake. This means that if we ever see more - than 2 handshakes, we raise the flag. */ - return (tls->server_handshake_count > 2) ? 1 : 0; -} - #ifdef V2_HANDSHAKE_SERVER /** Return true iff the cipher list suggested by the client for ssl is * a list that indicates that the client knows how to do the v2 TLS connection @@ -1648,12 +1637,6 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) err = tor_tls_get_error(tls, r, CATCH_ZERO, "reading", LOG_DEBUG, LD_NET); - if (tor_tls_got_excess_renegotiations(tls)) { - log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); - - return TOR_TLS_ERROR_MISC; - } - #ifdef V2_HANDSHAKE_SERVER if (tls->got_renegotiate) { tor_assert(tls->server_handshake_count == 2); @@ -1666,6 +1649,14 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) tls->got_renegotiate = 0; return r; + } else if (tls->server_handshake_count > 2) { + /* If we get more than 2 handshakes, it means that our peer is + trying to re-renegotiate. Return an error. */ + tor_assert(tls->server_handshake_count == 3); + + log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); + + return TOR_TLS_ERROR_MISC; } #endif @@ -1705,13 +1696,6 @@ tor_tls_write(tor_tls_t *tls, const char *cp, size_t n) } r = SSL_write(tls->ssl, cp, (int)n); err = tor_tls_get_error(tls, r, 0, "writing", LOG_INFO, LD_NET); - - if (tor_tls_got_excess_renegotiations(tls)) { - log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); - - return TOR_TLS_ERROR_MISC; - } - if (err == TOR_TLS_DONE) { return r; } From 021ff31ba6c303ebf13e12e691ea74ca4b39a693 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:21 -0500 Subject: [PATCH 13/16] Revert "Get rid of tor_tls_block_renegotiation()." This reverts commit 340809dd224b244675496e301d3ba154a6fe68d0. --- src/common/tortls.c | 10 ++++++++++ src/common/tortls.h | 1 + src/or/connection_or.c | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/src/common/tortls.c b/src/common/tortls.c index ddb5ea1efc..9caf9308bf 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1572,6 +1572,16 @@ tor_tls_unblock_renegotiation(tor_tls_t *tls) } } +/** If this version of openssl supports it, turn off renegotiation on + * tls. (Our protocol never requires this for security, but it's nice + * to use belt-and-suspenders here.) + */ +void +tor_tls_block_renegotiation(tor_tls_t *tls) +{ + tls->ssl->s3->flags &= ~SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION; +} + void tor_tls_assert_renegotiation_unblocked(tor_tls_t *tls) { diff --git a/src/common/tortls.h b/src/common/tortls.h index e558df756c..673f18dfe8 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -77,6 +77,7 @@ int tor_tls_handshake(tor_tls_t *tls); int tor_tls_finish_handshake(tor_tls_t *tls); int tor_tls_renegotiate(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); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 992db9a40c..cbe678d6cf 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1149,6 +1149,10 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) or_connection_t *conn = _conn; (void)tls; + /* Don't invoke this again. */ + tor_tls_set_renegotiate_callback(tls, NULL, NULL); + tor_tls_block_renegotiation(tls); + if (connection_tls_finish_handshake(conn) < 0) { /* XXXX_TLS double-check that it's ok to do this from inside read. */ /* XXXX_TLS double-check that this verifies certificates. */ @@ -1537,6 +1541,7 @@ connection_tls_finish_handshake(or_connection_t *conn) connection_or_init_conn_from_address(conn, &conn->_base.addr, conn->_base.port, digest_rcvd, 0); } + tor_tls_block_renegotiation(conn->tls); return connection_or_set_state_open(conn); } else { conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING_V2; From e09dd43ab38e2f1a23010463b1188a1d3631e960 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:21 -0500 Subject: [PATCH 14/16] Revert "Detect and deny excess renegotiations attempts." This reverts commit ecd239e3b577705e0669d47293a2e755cf93cec0. --- src/common/tortls.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 9caf9308bf..c0ff4e172d 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -146,7 +146,7 @@ struct tor_tls_t { /** True iff we should call negotiated_callback when we're done reading. */ unsigned int got_renegotiate:1; /** Incremented every time we start the server side of a handshake. */ - unsigned int server_handshake_count:2; + uint8_t server_handshake_count; size_t wantwrite_n; /**< 0 normally, >0 if we returned wantwrite last * time. */ /** Last values retrieved from BIO_number_read()/write(); see @@ -1318,13 +1318,11 @@ tor_tls_client_is_using_v2_ciphers(const SSL *ssl, const char *address) static void tor_tls_got_server_hello(tor_tls_t *tls) { - if (tls->server_handshake_count < 3) - ++tls->server_handshake_count; - - if (tls->server_handshake_count == 2) { - tor_assert(tls->negotiated_callback); + /* Check whether we're watching for renegotiates. If so, this is one! */ + if (tls->negotiated_callback) tls->got_renegotiate = 1; - } + if (tls->server_handshake_count < 127) /*avoid any overflow possibility*/ + ++tls->server_handshake_count; /* Now check the cipher list. */ if (tor_tls_client_is_using_v2_ciphers(tls->ssl, ADDR(tls))) { @@ -1659,14 +1657,6 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) tls->got_renegotiate = 0; return r; - } else if (tls->server_handshake_count > 2) { - /* If we get more than 2 handshakes, it means that our peer is - trying to re-renegotiate. Return an error. */ - tor_assert(tls->server_handshake_count == 3); - - log_info(LD_NET, "Detected excess renegotiation from %s!", ADDR(tls)); - - return TOR_TLS_ERROR_MISC; } #endif From 9727d21f68d0ef91f9c9002bebde425477bc4c6f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:21 -0500 Subject: [PATCH 15/16] Revert "Detect renegotiation when it actually happens." This reverts commit 4fd79f9def28996552b5739792f428c2514de1f6. --- src/common/tortls.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index c0ff4e172d..e26fd1ff26 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1638,28 +1638,19 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) tor_assert(tls->state == TOR_TLS_ST_OPEN); tor_assert(lenssl, cp, (int)len); - if (r > 0) /* return the number of characters read */ - return r; - - /* If we got here, SSL_read() did not go as expected. */ - - err = tor_tls_get_error(tls, r, CATCH_ZERO, "reading", LOG_DEBUG, LD_NET); - + if (r > 0) { #ifdef V2_HANDSHAKE_SERVER - if (tls->got_renegotiate) { - tor_assert(tls->server_handshake_count == 2); - /* XXX tor_assert(err == TOR_TLS_WANTREAD); */ - - /* Renegotiation happened! */ - log_info(LD_NET, "Got a TLS renegotiation from %s", ADDR(tls)); - if (tls->negotiated_callback) - tls->negotiated_callback(tls, tls->callback_arg); - tls->got_renegotiate = 0; - + if (tls->got_renegotiate) { + /* Renegotiation happened! */ + log_info(LD_NET, "Got a TLS renegotiation from %s", ADDR(tls)); + if (tls->negotiated_callback) + tls->negotiated_callback(tls, tls->callback_arg); + tls->got_renegotiate = 0; + } +#endif return r; } -#endif - + err = tor_tls_get_error(tls, r, CATCH_ZERO, "reading", LOG_DEBUG, LD_NET); if (err == _TOR_TLS_ZERORETURN || err == TOR_TLS_CLOSE) { log_debug(LD_NET,"read returned r=%d; TLS is closed",r); tls->state = TOR_TLS_ST_CLOSED; From 0ebcf345ce6087deeb9f1a1a54b9e6d003822fc3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 Dec 2011 19:49:21 -0500 Subject: [PATCH 16/16] Revert "Refactor the SSL_set_info_callback() callbacks." This reverts commit 69a821ea1c9357acdd5aa1c9e23fd030b01cb5a9. --- src/common/tortls.c | 97 +++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 44 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index e26fd1ff26..e4992efc6d 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1312,29 +1312,55 @@ tor_tls_client_is_using_v2_ciphers(const SSL *ssl, const char *address) return 1; } -/** We sent the ServerHello part of an SSL handshake. This might mean - * that we completed a renegotiation and appropriate actions must be - * taken. */ static void -tor_tls_got_server_hello(tor_tls_t *tls) +tor_tls_debug_state_callback(const SSL *ssl, int type, int val) { - /* Check whether we're watching for renegotiates. If so, this is one! */ - if (tls->negotiated_callback) - tls->got_renegotiate = 1; - if (tls->server_handshake_count < 127) /*avoid any overflow possibility*/ - ++tls->server_handshake_count; + log_debug(LD_HANDSHAKE, "SSL %p is now in state %s [type=%d,val=%d].", + ssl, ssl_state_to_string(ssl->state), type, val); +} + +/** Invoked when we're accepting a connection on ssl, and the connection + * changes state. We use this: + *
  • To alter the state of the handshake partway through, so we + * do not send or request extra certificates in v2 handshakes.
    • + *
  • To detect renegotiation
+ */ +static void +tor_tls_server_info_callback(const SSL *ssl, int type, int val) +{ + tor_tls_t *tls; + (void) val; + + tor_tls_debug_state_callback(ssl, type, val); + + if (type != SSL_CB_ACCEPT_LOOP) + return; + if (ssl->state != SSL3_ST_SW_SRVR_HELLO_A) + return; + + tls = tor_tls_get_by_ssl(ssl); + if (tls) { + /* Check whether we're watching for renegotiates. If so, this is one! */ + if (tls->negotiated_callback) + tls->got_renegotiate = 1; + if (tls->server_handshake_count < 127) /*avoid any overflow possibility*/ + ++tls->server_handshake_count; + } else { + log_warn(LD_BUG, "Couldn't look up the tls for an SSL*. How odd!"); + return; + } /* Now check the cipher list. */ - if (tor_tls_client_is_using_v2_ciphers(tls->ssl, ADDR(tls))) { + if (tor_tls_client_is_using_v2_ciphers(ssl, ADDR(tls))) { /*XXXX_TLS keep this from happening more than once! */ /* Yes, we're casting away the const from ssl. This is very naughty of us. * Let's hope openssl doesn't notice! */ /* Set SSL_MODE_NO_AUTO_CHAIN to keep from sending back any extra certs. */ - SSL_set_mode((SSL*) tls->ssl, SSL_MODE_NO_AUTO_CHAIN); + SSL_set_mode((SSL*) ssl, SSL_MODE_NO_AUTO_CHAIN); /* Don't send a hello request. */ - SSL_set_verify((SSL*) tls->ssl, SSL_VERIFY_NONE, NULL); + SSL_set_verify((SSL*) ssl, SSL_VERIFY_NONE, NULL); if (tls) { tls->wasV2Handshake = 1; @@ -1349,35 +1375,6 @@ tor_tls_got_server_hello(tor_tls_t *tls) } #endif -/** This is a callback function for SSL_set_info_callback() and it - * will be called in every SSL state change. - * It logs the SSL state change, and executes any actions that must be - * taken. */ -static void -tor_tls_state_changed_callback(const SSL *ssl, int type, int val) -{ - log_debug(LD_HANDSHAKE, "SSL %p is now in state %s [type=%d,val=%d].", - ssl, ssl_state_to_string(ssl->state), type, val); - -#ifdef V2_HANDSHAKE_SERVER - if (type == SSL_CB_ACCEPT_LOOP && - ssl->state == SSL3_ST_SW_SRVR_HELLO_A) { - - /* Call tor_tls_got_server_hello() for every SSL ServerHello we - send. */ - - tor_tls_t *tls = tor_tls_get_by_ssl(ssl); - if (!tls) { - log_warn(LD_BUG, "Couldn't look up the tls for an SSL*. How odd!"); - return; - } - - tor_tls_got_server_hello(tls); - } -#endif - -} - /** Replace *ciphers with a new list of SSL ciphersuites: specifically, * a list designed to mimic a common web browser. Some of the ciphers in the * list won't actually be implemented by OpenSSL: that's okay so long as the @@ -1519,8 +1516,14 @@ tor_tls_new(int sock, int isServer) log_warn(LD_NET, "Newly created BIO has read count %lu, write count %lu", result->last_read_count, result->last_write_count); } - - SSL_set_info_callback(result->ssl, tor_tls_state_changed_callback); +#ifdef V2_HANDSHAKE_SERVER + if (isServer) { + SSL_set_info_callback(result->ssl, tor_tls_server_info_callback); + } else +#endif + { + SSL_set_info_callback(result->ssl, tor_tls_debug_state_callback); + } /* Not expected to get called. */ tls_log_errors(NULL, LOG_WARN, LD_NET, "creating tor_tls_t object"); @@ -1550,7 +1553,13 @@ tor_tls_set_renegotiate_callback(tor_tls_t *tls, tls->negotiated_callback = cb; tls->callback_arg = arg; tls->got_renegotiate = 0; - SSL_set_info_callback(tls->ssl, tor_tls_state_changed_callback); +#ifdef V2_HANDSHAKE_SERVER + if (cb) { + SSL_set_info_callback(tls->ssl, tor_tls_server_info_callback); + } else { + SSL_set_info_callback(tls->ssl, tor_tls_debug_state_callback); + } +#endif } /** If this version of openssl requires it, turn on renegotiation on