[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #4549 [Tor Bridge]: Implement user-defined certificate strings through torrc (part of the proposal 179 efforts)

#4549: Implement user-defined certificate strings through torrc (part of the
proposal 179 efforts)
------------------------+---------------------------------------------------
 Reporter:  asn         |          Owner:                    
     Type:  defect      |         Status:  needs_review      
 Priority:  normal      |      Milestone:  Tor: 0.2.3.x-final
Component:  Tor Bridge  |        Version:                    
 Keywords:              |         Parent:                    
   Points:              |   Actualpoints:                    
------------------------+---------------------------------------------------

Comment(by nickm):

 This is based on the same messed-up master as #4548 was, and is all mixed
 up with #4548 changes.  Next time I'd prefer have something where it's
 easy to tell which actual commits I'm supposed to review.  As near as I
 can tell here, it's supposed to be everything in bug4549 starting with
 fb8b31c3fb26d03.

 Notes on review:

  * wrt the tor_tls_set_custom_cert_strings interface: There is a great
 quote from Alan Perlis: "If you have a procedure with ten parameters, you
 probably missed some."   These arguments probably want to turn into a
 single argument of type tor_cert_cfg_t, to be defined in tortls.h
  * Customizeable start and end time and serial number: these are probably
 a mistake.  Serial number needs to be different for every generated cert
 with the same issuer; start time and end time seem like they're begging
 you to have your Tor not work because you made your own cert expire.
 Instead let's go with certs that expire after a year or something; that
 seems to be the standard lifetime.
  * tor_X509_name_new now has tons of copy-and-paste code.  That's not so
 good; let's use a function or a macro to make it shorter.
  * There's no need to use BN_pseudo_rand to get a random number; just use
 Tor's crypto_rand_int or whatever it's called.
  * The documentation for tor_decorate_cert makes no sense to me.  How does
 stuff "float around" ?  What does it mean to "decorate" a cert?  Which
 certs get decorated?  Why is this a separate function?
  * Looks like the covert-channel-in-the-serialnumber junk is still in here
 too; it'll need to come out.

 Questions/ideas:

  * It looks like you apply the issuer name stuff to be the name of EVERY
 issuer, and the subject name to be subject name on EVERY cert?  That isn't
 right, if so.  This stuff only wants to apply to the initial presented-in-
 cleartext cert, which now wants to be different from the cert chain
 presented after in the v2 or v3 handshake.  The cert chain probably wants
 to stay unchanged.
  * How have you tested this?
  * Maybe we should have the ability to store these and the corresponding
 link keys in the datadir.  If so, that should probably get done along with
 the "load a custom cert from disk" work.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/4549#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs