[tor-bugs] #3202 [Pluggable transport]: shared secret support in obfs2

[Tor Bug Tracker & Wiki]

#3202: shared secret support in obfs2
---------------------------------+------------------------------------------
 Reporter:  asn                  |          Owner:  asn     
     Type:  defect               |         Status:  assigned
 Priority:  normal               |      Milestone:          
Component:  Pluggable transport  |        Version:          
 Keywords:                       |         Parent:          
   Points:                       |   Actualpoints:          
---------------------------------+------------------------------------------
Changes (by asn):

  * status:  needs_review => assigned


Comment:

 Thanks for the comments!
 I improved the code in my shared_secret branch.

 Replying to [comment:3 nickm]:
 > I don't think I agree with having the shared secret be NUL-terminated in
 the protocol level. It's fine to only take C strings from the command
 line, but why limit the code by making it unable to handle something the
 protocol allows fine?
 >
 > Also, it seems silly to just take the thing from the command line and
 shove it into the "secret" part of the protocol directly, truncating its
 later bytes.  Instead we should hash it so that every byte of input
 counts, and the input doesn't need to be a given length.  Maybe we should
 even do an iterated hash approach to slow down password-guessing attempts.
 >

 You are right.
 In my latest commits, I stopped treating the shared secret as a C-string
 through network.c and obfs2.c.
 The thing is that command line arguments are C-strings anyway. I assume
 that 'in the future' there will be another way to add shared secrets
 (config file?) for hardcore binary people. 'till then we are fine with a
 shared secret of arbitrary size, aren't we?

 About iterated hashing, I see no reason to not do it.

 This whole thing will also require spec overwrite.
 I'll prepare a patch for the iterated hashing and the spec soon.

 > (Also, the time to change stuff is now: since our hash function is 32
 bytes, let's make the shared secret get to be 32 bytes long.)
 >

 True. Done!

 > Also, why not copy the secret into the state?  Keeping a pointer around
 to it and requiring that it never change or get freed seems like an iffy
 interface choice.
 >

 True!
 I ended up copying the secret to the protocol_params_t struct, which I
 also documented and I think it fits the role quite nice. I also created a
 function that frees it when the listener dies.

 > Also the only code I see here sets lsn->have_shared_secret = 1
 unconditionally.  That's not right, is it?  I'm guessing that this is
 something else that you had for testing that you mean to turn off later?
 >

 True!

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