[tor-bugs] #6465 [Tor Relay]: Build abstraction layer around TLS

[Tor Bug Tracker & Wiki]

#6465: Build abstraction layer around TLS
-----------------------+----------------------------------------------------
 Reporter:  andrea     |          Owner:  andrea            
     Type:  project    |         Status:  needs_review      
 Priority:  major      |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor Relay  |        Version:  Tor: unspecified  
 Keywords:             |         Parent:                    
   Points:             |   Actualpoints:                    
-----------------------+----------------------------------------------------

Comment(by andrea):

 Responses to part 3 (points in potential need of further discussion):

 > Weird thing: connection_or.c in your branch is 2322 lines long. In
 master,
 > it's 2290 lines long. I would have expected it to get shorter as code
 moved
 > out.

 Yeah, but connection_or.c didn't lose that much; the biggest loser was
 command.c.  Looking at the diff on that file, it looks like the
 connection_or_send_destroy(), connection_or_set_circid_type(),
 connection_or_mark_bad_for_new_circs(), connection_or_get_for_extend() and
 connection_or_is_better() functions went away, but a lot of functions got
 a little longer because of additional channel-related logic, and it gained
 some glue logic it needed to let things that called it elsehwere pass
 through to channels (connection_or_get_num_circuits(),
 connection_or_change_state(), connection_or_notify_error(),
 connection_or_close_normally(), connection_or_close_for_error(),
 connection_or_client_used()), and a laundry list of other functions gained
 a few extra lines here and there.  The latter outweighed the former a bit.

 > The new log_debug in connection_mark_for_close makes me wonder: can we
 be
 > checking for this case and warning/asserting if it hits? If the new rule
 is
 > "never call connection_mark_for_close on an or_conn directly", that's
 going
 > to be fragile.

 If you do that it'll assert in the channel code because you'll be closing
 it
 when it isn't in the CLOSING state.  I added that debug message to track
 down
 where things were doing that.

 > General stuff: I'm perpetually terrified of breaking the handshake in a
 way
 > to allow us to count as authenticated, or to process cells we shouldn't,
 or
 > to send data we shouldn't, without actually completing the TLS handshake
 and
 > verifying the other party with the Tor handshake. I'm also perpetually
 afraid
 > of breaking the v2 or v1 TLS handshakes and not noticing because I only
 > tested master against master.

 Hmm, good point.  Got any suggestions on how to test stuff like this?

 > Where did that big block of code in connection_or_set_state_open go?

 It's in channel_do_open_actions() now.

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