November 2011 - tor-commits - lists.torproject.org

[tor/master] Get rid of tor_tls_block_renegotiation().
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit 340809dd224b244675496e301d3ba154a6fe68d0 Author: George Kadianakis <desnacked(a)gmail.com> Date: Wed Oct 26 13:16:14 2011 +0200 Get rid of tor_tls_block_renegotiation(). Since we check for naughty renegotiations using tor_tls_t.server_handshake_count we don't need that semi-broken function (at least till there is a way to disable rfc5746 renegotiations too). --- src/common/tortls.c | 10 ---------- src/common/tortls.h | 1 - src/or/connection_or.c | 5 ----- 3 files changed, 0 insertions(+), 16 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 7269785..1150c42 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1540,16 +1540,6 @@ 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 90e76e4..1219b80 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -75,7 +75,6 @@ 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 bcae075..6c56a61 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1139,10 +1139,6 @@ 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. */ @@ -1529,7 +1525,6 @@ 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;

1 0

[tor/master] Also handle needless renegotiations in SSL_write().
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit e2b3527106e0747f652e2f28fa087d9874e0e2ce Author: George Kadianakis <desnacked(a)gmail.com> Date: Wed Oct 26 13:36:30 2011 +0200 Also handle needless renegotiations in SSL_write(). SSL_read(), SSL_write() and SSL_do_handshake() can always progress the SSL protocol instead of their normal operation, this means that we must be checking for needless renegotiations after they return. Introduce tor_tls_got_excess_renegotiations() which makes the tls->server_handshake_count > 2 check for us, and use it in tor_tls_read() and tor_tls_write(). Cases that should not be handled: * SSL_do_handshake() is only called by tor_tls_renegotiate() which is a client-only function. * The SSL_read() in tor_tls_shutdown() does not need to be handled, since SSL_shutdown() will be called if SSL_read() returns an error. --- src/common/tortls.c | 32 ++++++++++++++++++++++++-------- 1 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 1150c42..4235de7 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1228,6 +1228,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 @@ -1605,6 +1616,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) { tor_assert(tls->server_handshake_count == 2); @@ -1617,14 +1634,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 @@ -1664,6 +1673,13 @@ 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; }

1 0

[tor/master] Fix issues pointed out by nickm.
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit e097bffaed72af6b19f7293722021196bb94de1e Author: George Kadianakis <desnacked(a)gmail.com> Date: Thu Nov 3 22:33:50 2011 +0100 Fix issues pointed out by nickm. - Rename tor_tls_got_server_hello() to tor_tls_got_client_hello(). - Replaced some aggressive asserts with LD_BUG logging. They were the innocent "I believe I understand how these callbacks work, and this assert proves it" type of callbacks, and not the "If this statement is not true, computer is exploding." type of callbacks. - Added a changes file. --- changes/bug4312 | 11 +++++++++++ src/common/tortls.c | 26 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/changes/bug4312 b/changes/bug4312 new file mode 100644 index 0000000..f8647d3 --- /dev/null +++ b/changes/bug4312 @@ -0,0 +1,11 @@ + o Security fixes: + + - Block excess renegotiations even if they are RFC5746 compliant. + This mitigates potential SSL Denial of Service attacks that use + SSL renegotiation as a way of forcing the server to perform + unneeded computationally expensive SSL handshakes. Implements + #4312. + + - Fix a bug where tor would not notice excess renegotiation + attempts before it received the first data SSL record. Fixes + part of #4312. diff --git a/src/common/tortls.c b/src/common/tortls.c index 4235de7..111a7f6 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1291,17 +1291,21 @@ 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. */ +/** We got an SSL ClientHello message. This might mean that the + * client wants to initiate a renegotiation and appropriate actions + * must be taken. */ static void -tor_tls_got_server_hello(tor_tls_t *tls) +tor_tls_got_client_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); + if (!tls->negotiated_callback) { + log_warn(LD_BUG, "Got a renegotiation request but we don't" + " have a renegotiation callback set!"); + } + tls->got_renegotiate = 1; } @@ -1344,8 +1348,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_server_hello() for every SSL ServerHello we - send. */ + /* Call tor_tls_got_client_hello() for every SSL ClientHello we + receive. */ tor_tls_t *tls = tor_tls_get_by_ssl(ssl); if (!tls) { @@ -1353,7 +1357,7 @@ tor_tls_state_changed_callback(const SSL *ssl, int type, int val) return; } - tor_tls_got_server_hello(tls); + tor_tls_got_client_hello(tls); } #endif @@ -1624,8 +1628,10 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len) #ifdef V2_HANDSHAKE_SERVER if (tls->got_renegotiate) { - tor_assert(tls->server_handshake_count == 2); - /* XXX tor_assert(err == TOR_TLS_WANTREAD); */ + if (tls->server_handshake_count != 2) { + log_warn(LD_BUG, "We did not notice renegotiation in a timely fashion (%u)!", + tls->server_handshake_count); + } /* Renegotiation happened! */ log_info(LD_NET, "Got a TLS renegotiation from %s", ADDR(tls));

1 0

[tor/master] Use callback-driven approach to block renegotiations.
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit 406ae1ba5ad529a4d0e710229dab6ed645d42b50 Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Nov 13 14:47:11 2011 +0100 Use callback-driven approach to block renegotiations. Also use this new approach in the bufferevents-enabled case. --- 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, 62 insertions(+), 31 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 3201738..ea7f5d1 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -518,6 +518,15 @@ 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 0247297..897eddb 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -46,6 +46,9 @@ 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 111a7f6..94aa4ae 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -52,7 +52,6 @@ #include <event2/bufferevent_ssl.h> #include <event2/buffer.h> #include <event2/event.h> -#include "compat_libevent.h" #endif #define CRYPTO_PRIVATE /* to import prototypes from crypto.h */ @@ -64,6 +63,7 @@ #include "torlog.h" #include "container.h" #include <string.h> +#include "compat_libevent.h" /* Enable the "v2" TLS handshake. */ @@ -157,6 +157,11 @@ 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; }; @@ -1228,17 +1233,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 @@ -1307,6 +1301,20 @@ 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. */ @@ -1523,16 +1531,20 @@ 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. 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. 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! */ void -tor_tls_set_renegotiate_callback(tor_tls_t *tls, +tor_tls_set_renegotiate_callbacks(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); @@ -1592,6 +1604,7 @@ 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); @@ -1620,12 +1633,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) { if (tls->server_handshake_count != 2) { @@ -1680,12 +1687,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; } diff --git a/src/common/tortls.h b/src/common/tortls.h index 1219b80..f032acd 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -13,6 +13,7 @@ #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; @@ -60,8 +61,9 @@ 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_callback(tor_tls_t *tls, +void tor_tls_set_renegotiate_callbacks(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 6c56a61..0b39ad5 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1146,6 +1146,20 @@ 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(). * @@ -1192,8 +1206,9 @@ 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_callback(conn->tls, + 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)); @@ -1255,8 +1270,9 @@ 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_callback(conn->tls, + 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; */

1 0

[tor/master] Merge remote-tracking branch 'asn/bug4312'
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit e5f2f10844dc2445f8cf2f0dc8ec92bc4ce65d63 Merge: b1bd30c 406ae1b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 25 17:00:47 2011 -0500 Merge remote-tracking branch 'asn/bug4312' changes/bug4312 | 11 +++ src/common/compat_libevent.c | 9 ++ src/common/compat_libevent.h | 3 + src/common/tortls.c | 169 ++++++++++++++++++++++++------------------ src/common/tortls.h | 5 +- src/or/connection_or.c | 25 +++++-- 6 files changed, 140 insertions(+), 82 deletions(-)

1 0

[tor/master] Refactor tor_event_base_once to do what we actually want
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit 7920ea55b8d994268d2b07f27316b0f34d8f27e5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 25 17:18:03 2011 -0500 Refactor tor_event_base_once to do what we actually want This version avoids the timeout system entirely, gives a nicer interface, and lets us manage allocation explicitly. --- 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, 49 insertions(+), 19 deletions(-) diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index b709b4a..c3a4746 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -558,13 +558,48 @@ tor_check_libevent_header_compatibility(void) #endif } -/** Wrapper around libevent's event_base_once(). Sets a - * timeout-triggered event with no associated file descriptor. */ +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. + */ int -tor_event_base_once(void (*cb)(evutil_socket_t, short, void *), - void *arg, struct timeval *timer) +tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg) { - return event_base_once(tor_libevent_get_base(), -1, EV_TIMEOUT, cb, arg, 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; } /* diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 897eddb..3f916d1 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -46,8 +46,7 @@ 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); +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, diff --git a/src/common/tortls.c b/src/common/tortls.c index 59e1bbc..88efa05 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)(evutil_socket_t, short, void *); + void (*excess_renegotiations_callback)(void *); /** Argument to pass to negotiated_callback. */ void *callback_arg; @@ -1326,10 +1326,8 @@ 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. */ - struct timeval zero_seconds_timer = {0,0}; - - if (tor_event_base_once(tls->excess_renegotiations_callback, - tls->callback_arg, &zero_seconds_timer) < 0) { + 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."); } } @@ -1557,7 +1555,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)(evutil_socket_t, short, void *), + void (*cb2)(void *), void *arg) { tls->negotiated_callback = cb; diff --git a/src/common/tortls.h b/src/common/tortls.h index 5874881..9f86e37 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)(evutil_socket_t, short, void *), + void (*cb2)(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 efda6c0..ecd3b59 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3647,8 +3647,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 c095716..cbdfa44 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1157,11 +1157,9 @@ 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(evutil_socket_t fd, short what, void *_conn) +connection_or_close_connection_cb(void *_conn) { or_connection_t *conn = _conn; - (void) what; - (void) fd; connection_stop_reading(TO_CONN(conn)); connection_mark_for_close(TO_CONN(conn));

1 0

[tor/master] Avoid a double-mark in connection_or_close_connection_cb
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit 633071eb3bcf2c4106e93de28d727594bd23b1db Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 25 17:21:11 2011 -0500 Avoid a double-mark in connection_or_close_connection_cb --- src/or/connection_or.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index cbdfa44..e868b7b 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1159,10 +1159,12 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) static void connection_or_close_connection_cb(void *_conn) { - or_connection_t *conn = _conn; + or_connection_t *or_conn = _conn; + connection_t *conn = TO_CONN(or_conn); - connection_stop_reading(TO_CONN(conn)); - connection_mark_for_close(TO_CONN(conn)); + connection_stop_reading(conn); + if (!conn->marked_for_close) + connection_mark_for_close(conn); } /** Move forward with the tls handshake. If it finishes, hand

1 0

[tor/master] Fix some wide lines in tortls.c
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit e8dde3aabd3e1292d381eb4269c6457548dca6b9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 25 17:22:46 2011 -0500 Fix some wide lines in tortls.c --- src/common/tortls.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 88efa05..0571b34 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1328,7 +1328,8 @@ 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."); } } @@ -1383,7 +1384,6 @@ 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, @@ -1651,8 +1651,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! */

1 0

[tor/master] Prepare util.[ch] to use the new process_handle_t API.
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit f12a40d8603a7e3e215652fe3bb54e2e1566646c Author: George Kadianakis <desnacked(a)gmail.com> Date: Mon Oct 24 15:55:53 2011 +0200 Prepare util.[ch] to use the new process_handle_t API. Also, create tor_process_destroy() which destroys a process_handle_t. --- src/common/util.c | 64 +++++++++++++++++++++++++++++++++++++++++----------- src/common/util.h | 12 ++++++++- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 79e09e4..6a0893b 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3159,25 +3159,29 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, /* Maximum number of file descriptors, if we cannot get it via sysconf() */ #define DEFAULT_MAX_FD 256 -/** Terminate process running at PID pid. +/** Terminate the process of process_handle. * Code borrowed from Python's os.kill. */ int -tor_terminate_process(pid_t pid) +tor_terminate_process(process_handle_t *process_handle) { #ifdef MS_WINDOWS - HANDLE handle; - /* If the signal is outside of what GenerateConsoleCtrlEvent can use, - attempt to open and terminate the process. */ - handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid); - if (!handle) - return -1; + if (tor_get_exit_code(process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) { + HANDLE handle; + /* If the signal is outside of what GenerateConsoleCtrlEvent can use, + attempt to open and terminate the process. */ + handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, process_handle->pid.dwProcessId); + if (!handle) + return -1; - if (!TerminateProcess(handle, 0)) + if (!TerminateProcess(handle, 0)) + return -1; + else + return 0; + } else { /* process was not running */ return -1; - else - return 0; + } #else /* Unix */ - return kill(pid, SIGTERM); + return kill(process_handle->pid, SIGTERM); #endif } @@ -3213,7 +3217,11 @@ tor_terminate_process(pid_t pid) */ int tor_spawn_background(const char *const filename, const char **argv, +#ifdef MS_WINDOWS + LPVOID envp, +#else const char **envp, +#endif process_handle_t *process_handle) { #ifdef MS_WINDOWS @@ -3293,7 +3301,7 @@ tor_spawn_background(const char *const filename, const char **argv, /*(TODO: set CREATE_NEW CONSOLE/PROCESS_GROUP to make GetExitCodeProcess() * work?) */ 0, // creation flags - NULL, // use parent's environment + envp, // use parent's environment NULL, // use parent's current directory &siStartInfo, // STARTUPINFO pointer &(process_handle->pid)); // receives PROCESS_INFORMATION @@ -3352,7 +3360,7 @@ tor_spawn_background(const char *const filename, const char **argv, } retval = pipe(stderr_pipe); - if (-1 == retval) { + if (-1 == retval) { /* if it fails here, it doesn't close the stdout_pipe */ log_warn(LD_GENERAL, "Failed to set up pipe for stderr communication with child process: %s", strerror(errno)); @@ -3495,6 +3503,34 @@ tor_spawn_background(const char *const filename, const char **argv, #endif // MS_WINDOWS } +void +tor_process_destroy(process_handle_t *process_handle, + int also_terminate_process) +{ + if (also_terminate_process) + if (tor_terminate_process(process_handle) < 0) { + log_notice(LD_GENERAL, "Failed to terminate process with PID " +#ifdef MS_WINDOWS + "%u\n", process_handle->pid.dwProcessId +#else + "%d\n", (int) process_handle->pid +#endif + ); + } + + process_handle->status = PROCESS_STATUS_NOTRUNNING; + +#ifdef MS_WINDOWS + CloseHandle(process_handle->stdout_pipe); + CloseHandle(process_handle->stderr_pipe); +#else + fclose(process_handle->stdout_handle); + fclose(process_handle->stderr_handle); +#endif + + tor_free(process_handle); +} + /** Get the exit code of a process specified by process_handle and store * it in exit_code, if set to a non-NULL value. If block is set * to true, the call will block until the process has exited. Otherwise if diff --git a/src/common/util.h b/src/common/util.h index c5a52d1..b9f3a5b 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -360,10 +360,14 @@ void write_pidfile(char *filename); void tor_check_port_forwarding(const char *filename, int dir_port, int or_port, time_t now); -int tor_terminate_process(pid_t pid); typedef struct process_handle_s process_handle_t; int tor_spawn_background(const char *const filename, const char **argv, - const char **envp, process_handle_t *process_handle); +#ifdef MS_WINDOWS + LPVOID envp, +#else + const char **envp, +#endif + process_handle_t *process_handle); #define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code " @@ -413,6 +417,10 @@ ssize_t tor_read_all_from_process_stderr( const process_handle_t *process_handle, char *buf, size_t count); char *tor_join_win_cmdline(const char *argv[]); +int tor_terminate_process(process_handle_t *process_handle); +void tor_process_destroy(process_handle_t *process_handle, + int also_terminate_process); + #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */

1 0

[tor/master] Port managed proxy launching code to the new subprocess API.
by nickm＠torproject.org 25 Nov '11

25 Nov '11

commit 45307ff98038b2bed7263194c8d5cf56d87f4de4 Author: George Kadianakis <desnacked(a)gmail.com> Date: Mon Oct 17 22:46:44 2011 +0200 Port managed proxy launching code to the new subprocess API. --- src/common/util.h | 6 +- src/or/transports.c | 227 +++++++++++++++++++++++++++++++++++++++++---------- src/or/transports.h | 4 +- 3 files changed, 187 insertions(+), 50 deletions(-) diff --git a/src/common/util.h b/src/common/util.h index 77ed1ca..c5a52d1 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -371,9 +371,6 @@ int tor_spawn_background(const char *const filename, const char **argv, HANDLE load_windows_system_library(const TCHAR *library_name); #endif -#ifdef UTIL_PRIVATE -/* Prototypes for private functions only used by util.c (and unit tests) */ - /* Values of process_handle_t.status. PROCESS_STATUS_NOTRUNNING must be * 0 because tor_check_port_forwarding depends on this being the initial * statue of the static instance of process_handle_t */ @@ -416,6 +413,9 @@ ssize_t tor_read_all_from_process_stderr( const process_handle_t *process_handle, char *buf, size_t count); char *tor_join_win_cmdline(const char *argv[]); +#ifdef UTIL_PRIVATE +/* Prototypes for private functions only used by util.c (and unit tests) */ + void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno); diff --git a/src/or/transports.c b/src/or/transports.c index 3c533cc..318cb23 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -13,8 +13,14 @@ #include "transports.h" #include "util.h" +#ifdef MS_WINDOWS +static void set_managed_proxy_environment(const managed_proxy_t *mp); +static void reset_environment(const managed_proxy_t *mp); +#else static void set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp); +#endif + static INLINE int proxy_configuration_finished(const managed_proxy_t *mp); static void managed_proxy_destroy(managed_proxy_t *mp); @@ -229,12 +235,15 @@ proxy_prepare_for_restart(managed_proxy_t *mp) transport_t *t_tmp = NULL; tor_assert(mp->conf_state == PT_PROTO_COMPLETED); - tor_assert(mp->pid); /* kill the old obfsproxy process */ - tor_terminate_process(mp->pid); - mp->pid = 0; - fclose(mp->_stdout); +#ifdef MS_WINDOWS + tor_terminate_process(mp->process_handle->pid.dwProcessId); +#else + tor_terminate_process(mp->process_handle->pid); +#endif + + memset(mp->process_handle, 0, sizeof(process_handle_t)); /* destroy all its old transports. we no longer use them. */ SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) { @@ -256,52 +265,54 @@ proxy_prepare_for_restart(managed_proxy_t *mp) static int launch_managed_proxy(managed_proxy_t *mp) { - (void) mp; - (void) set_managed_proxy_environment; - return -1; -#if 0 - /* XXXX023 we must reenable this code for managed proxies to work. - * "All it needs" is revision to work with the new tor_spawn_background - * API. */ +#ifdef MS_WINDOWS + /* In Windows, we set the managed proxy environment on the tor + process, and then we spawn a managed proxy with tor's + environment. After the managed proxy is launched, we reset the + environment to what it was previously. */ + + set_managed_proxy_environment(mp); + + /* Passing NULL as lpApplicationName makes Windows search for the .exe */ + tor_spawn_background(NULL, (const char **)mp->argv, NULL, + mp->process_handle); + + reset_environment(mp); + +#else + /* In Linux, we create an environment for the managed proxy process + in the envp variable. We then pass it to tor_spawn_background() + so that it can be used with execve(). */ + char **envp=NULL; - int pid; - process_handle_t proc; - FILE *stdout_read = NULL; - int stdout_pipe=-1, stderr_pipe=-1; /* prepare the environment variables for the managed proxy */ set_managed_proxy_environment(&envp, mp); - pid = tor_spawn_background(mp->argv[0], (const char **)mp->argv, - (const char **)envp, &proc); - if (pid < 0) { - log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", - mp->argv[0]); - return -1; - } + tor_spawn_background(mp->argv[0], (const char **)mp->argv, + (const char **)envp, mp->process_handle); /* free the memory allocated by set_managed_proxy_environment(). */ free_execve_args(envp); - /* Set stdout/stderr pipes to be non-blocking */ -#ifdef _WIN32 - { - u_long nonblocking = 1; - ioctlsocket(stdout_pipe, FIONBIO, &nonblocking); - } -#else - fcntl(stdout_pipe, F_SETFL, O_NONBLOCK); #endif - /* Open the buffered IO streams */ - stdout_read = fdopen(stdout_pipe, "r"); + if (mp->process_handle->status == PROCESS_STATUS_ERROR) { + log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", + mp->argv[0]); + return -1; + } - log_info(LD_CONFIG, "Managed proxy has spawned at PID %d.", pid); +#ifdef MS_WINDOWS + log_info(LD_CONFIG, "Managed proxy at '%s' has spawned.", + mp->argv[0]); +#else + log_info(LD_CONFIG, "Managed proxy at '%s' has spawned with pid %d.", + mp->argv[0], mp->process_handle->pid); +#endif mp->conf_state = PT_PROTO_LAUNCHED; - mp->_stdout = stdout_read; - mp->pid = pid; -#endif + return 0; } @@ -343,6 +354,61 @@ pt_configure_remaining_proxies(void) } SMARTLIST_FOREACH_END(mp); } +#ifdef MS_WINDOWS + +/** Attempt to continue configuring managed proxy mp. */ +static void +configure_proxy(managed_proxy_t *mp) +{ + int pos; + char stdout_buf[200]; + smartlist_t *lines = NULL; + + /* if we haven't launched the proxy yet, do it now */ + if (mp->conf_state == PT_PROTO_INFANT) { + if (launch_managed_proxy(mp) < 0) { /* launch fail */ + mp->conf_state = PT_PROTO_BROKEN; + handle_finished_proxy(mp); + } + return; + } + + tor_assert(mp->conf_state != PT_PROTO_INFANT); + + pos = tor_read_all_handle(mp->process_handle->stdout_pipe, + stdout_buf, sizeof(stdout_buf) - 1, NULL); + if (pos < 0) { + log_notice(LD_GENERAL, "Failed to read data from managed proxy"); + mp->conf_state = PT_PROTO_BROKEN; + goto done; + } + + if (pos == 0) /* proxy has nothing interesting to say. */ + return; + + /* End with a null even if there isn't a \r\n at the end */ + /* TODO: What if this is a partial line? */ + stdout_buf[pos] = '\0'; + + /* Split up the buffer */ + lines = smartlist_create(); + tor_split_lines(lines, stdout_buf, pos); + + /* Handle lines. */ + SMARTLIST_FOREACH(lines, const char *, line, + handle_proxy_line(line, mp)); + + done: + /* if the proxy finished configuring, exit the loop. */ + if (proxy_configuration_finished(mp)) + handle_finished_proxy(mp); + + if (lines) + smartlist_free(lines); +} + +#else /* Unix version: */ + /** Attempt to continue configuring managed proxy mp. */ static void configure_proxy(managed_proxy_t *mp) @@ -352,15 +418,18 @@ configure_proxy(managed_proxy_t *mp) /* if we haven't launched the proxy yet, do it now */ if (mp->conf_state == PT_PROTO_INFANT) { - launch_managed_proxy(mp); + if (launch_managed_proxy(mp) < 0) { /* launch fail */ + mp->conf_state = PT_PROTO_BROKEN; + handle_finished_proxy(mp); + } return; } tor_assert(mp->conf_state != PT_PROTO_INFANT); while (1) { - r = get_string_from_pipe(mp->_stdout, stdout_buf, - sizeof(stdout_buf) - 1); + r = get_string_from_pipe(mp->process_handle->stdout_handle, + stdout_buf, sizeof(stdout_buf) - 1); if (r == IO_STREAM_OKAY) { /* got a line; handle it! */ handle_proxy_line((const char *)stdout_buf, mp); @@ -382,6 +451,8 @@ configure_proxy(managed_proxy_t *mp) } } +#endif /* MS_WINDOWS */ + /** Register server managed proxy mp transports to state */ static void register_server_proxy(managed_proxy_t *mp) @@ -470,15 +541,19 @@ managed_proxy_destroy(managed_proxy_t *mp) /* remove it from the list of managed proxies */ smartlist_remove(managed_proxy_list, mp); - /* close its stdout stream */ - if (mp->_stdout) - fclose(mp->_stdout); - /* free the argv */ free_execve_args(mp->argv); - if (mp->pid) - tor_terminate_process(mp->pid); + /* If proxy is launched, terminate it. */ + if (mp->conf_state != PT_PROTO_INFANT) { +#ifdef MS_WINDOWS + tor_terminate_process(mp->process_handle->pid.dwProcessId); +#else + tor_terminate_process(mp->process_handle->pid); +#endif + } + + tor_free(mp->process_handle); tor_free(mp); } @@ -858,6 +933,64 @@ get_bindaddr_for_proxy(const managed_proxy_t *mp) return bindaddr; } +#ifdef MS_WINDOWS +/** Set the managed proxy environment for the managed proxy + * mp. */ +static void +set_managed_proxy_environment(const managed_proxy_t *mp) +{ + const or_options_t *options = get_options(); + char *state_loc=NULL; + char *transports_to_launch=NULL; + char *bindaddr=NULL; + char *orport=NULL; + + state_loc = get_datadir_fname("pt_state/"); /* XXX temp */ + transports_to_launch = + smartlist_join_strings(mp->transports_to_launch, ",", 0, NULL); + + SetEnvironmentVariable("TOR_PT_STATE_LOCATION", state_loc); + + SetEnvironmentVariable("TOR_PT_MANAGED_TRANSPORT_VER", "1"); + + if (mp->is_server) { + tor_asprintf(&orport, "127.0.0.1:%d", options->ORPort); + SetEnvironmentVariable("TOR_PT_ORPORT", orport); + + bindaddr = get_bindaddr_for_proxy(mp); + SetEnvironmentVariable("TOR_PT_SERVER_BINDADDR", bindaddr); + + SetEnvironmentVariable("TOR_PT_SERVER_TRANSPORTS", transports_to_launch); + SetEnvironmentVariable("TOR_PT_EXTENDED_SERVER_PORT", "127.0.0.1:4200"); + } else { + SetEnvironmentVariable("TOR_PT_CLIENT_TRANSPORTS", transports_to_launch); + } + + tor_free(state_loc); + tor_free(transports_to_launch); + tor_free(bindaddr); + tor_free(orport); +} + +/** Unsets the managed proxy environment variables. */ +static void +reset_environment(const managed_proxy_t *mp) +{ + SetEnvironmentVariable("TOR_PT_STATE_LOCATION", NULL); + SetEnvironmentVariable("TOR_PT_MANAGED_TRANSPORT_VER", NULL); + + if (mp->is_server) { + SetEnvironmentVariable("TOR_PT_ORPORT", NULL); + SetEnvironmentVariable("TOR_PT_SERVER_BINDADDR", NULL); + SetEnvironmentVariable("TOR_PT_SERVER_TRANSPORTS", NULL); + SetEnvironmentVariable("TOR_PT_EXTENDED_SERVER_PORT", NULL); + } else { + SetEnvironmentVariable("TOR_PT_CLIENT_TRANSPORTS", NULL); + } +} + +#else /* Unix version: */ + /** Prepare the envp of managed proxy mp */ static void set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) @@ -901,6 +1034,8 @@ set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) tor_free(bindaddr); } +#endif + /** Create and return a new managed proxy for transport using * proxy_argv. If is_server is true, it's a server * managed proxy. */ @@ -918,6 +1053,8 @@ managed_proxy_create(const smartlist_t *transport_list, SMARTLIST_FOREACH(transport_list, const char *, transport, add_transport_to_proxy(transport, mp)); + mp->process_handle = tor_malloc_zero(sizeof(process_handle_t)); + /* register the managed proxy */ if (!managed_proxy_list) managed_proxy_list = smartlist_create(); diff --git a/src/or/transports.h b/src/or/transports.h index 4a93387..fee9c77 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -47,8 +47,8 @@ typedef struct { int is_server; /* is it a server proxy? */ - FILE *_stdout; /* a stream to its stdout - (closed in managed_proxy_destroy()) */ + /* A pointer to the process handle of this managed proxy. */ + process_handle_t *process_handle; int pid; /* The Process ID this managed proxy is using. */

1 0