[tor-commits] [tor/master] Also handle needless renegotiations in SSL_write().

[nickm at torproject.org]

commit e2b3527106e0747f652e2f28fa087d9874e0e2ce
Author: George Kadianakis <desnacked at 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;
   }