Revert "Use callback-driven approach to block renegotiations."

This reverts commit 406ae1ba5ad529a4d0e710229dab6ed645d42b50.

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 <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 */
@@ -63,7 +64,6 @@
 #include "torlog.h"
 #include "container.h"
 #include <string.h>
-#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 <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
@@ -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 <b>cb</b> to be called with argument <b>arg</b> whenever
- * <b>tls</b> next gets a client-side renegotiate in the middle of a
- * read. Set <b>cb2</b> to be called with argument <b>arg</b> whenever
- * <b>tls</b> gets excess renegotiation requests.  Do not invoke this
- * function until <em>after</em> initial handshaking is done!
+/** Set <b>cb</b> to be called with argument <b>arg</b> whenever <b>tls</b>
+ * next gets a client-side renegotiate in the middle of a read.  Do not
+ * invoke this function until <em>after</em> 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
  * <b>conn</b> 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; */