[tor-bugs] #4312 [Tor Relay]: Rate limit renegotiations

[Tor Bug Tracker & Wiki]

#4312: Rate limit renegotiations
-----------------------+----------------------------------------------------
 Reporter:  asn        |          Owner:              
     Type:  defect     |         Status:  needs_review
 Priority:  normal     |      Milestone:              
Component:  Tor Relay  |        Version:              
 Keywords:             |         Parent:              
   Points:             |   Actualpoints:              
-----------------------+----------------------------------------------------

Comment(by nickm):

 Replying to [comment:8 asn]:
 > Replying to [comment:7 nickm]:
 > > Looks ok.
 > >
 > > For the bufferevents case: The same callback-based approach works for
 detecting excessive renegotiation, but we run into problems in knowing
 what to do about it.  We want to run some "kill this connection" callback
 soon, but not immediately (because running that kind of thing from inside
 the SSL callback from inside the bufferevent code is a recipe for Lots Of
 The Wrong Kind Fun.)  Probably, we could just use event_base_once code to
 invoke a trampoline function to call the appropriate user-level callback
 to call  bufferevent_disable() and connection_mark_for_close() on the
 connection.  Does that make sense?  I think it would work.
 >
 > In the non-bufferevents case, the OpenSSL callbacks increment the
 ClientHello-seen count, but they are not killing the connection. The
 connection is killed after SSL_{read,write}() functions are done.
 >
 > The problem with setting a "kill the connection" callback from within
 the OpenSSL callbacks is that we only have access to the `tor_tls_t` in
 there. I assume that calling a libevent event with the `tor_tls_t`
 attached and doing a linear search over all the connections to find the
 `tor_tls_t` bearer is super stupid, and we must find something better. If
 you have something in mind, I indeed believe a callback-based approach is
 the cleanest (we can also do that in the non-bufferevents case as well,
 and stop catching excess renegotiations around the code.)

 Right.  I'd be thinking of something like adding a function pointer and an
 void* to the tor_tls_t such that if the renegotiation count is exceeded,
 the function pointer gets called with the void* and the tor_tls_t as its
 arguments.  (Like how the renegotiation callback works now) When we set up
 a TLS connection to use bufferevents, we'd pass a callback function took
 the or_connection_t as an argument, and did  that did
 bufferevent_disable() on its bufferevent and connection_mark_for_close()
 on the connection.

 At least, that's what I'm thinking here.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/4312#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online