commit e2b3527106e0747f652e2f28fa087d9874e0e2ce Author: George Kadianakis <desnacked@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 <b>tls</b> 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 <b>ssl</b> 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; }