[tor-bugs] #28636 [Core Tor/Tor]: Address WTF-PAD comments by Nick (2018-11-27 over IRC)

[Tor Bug Tracker & Wiki]

#28636: Address WTF-PAD comments by Nick (2018-11-27 over IRC)
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  (none)
     Type:  defect                               |         Status:  new
 Priority:  High                                 |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  wtf-pad, tor-relay, tor-cell,        |  Actual Points:
  padding, 041-proposed                          |
Parent ID:  #28632                               |         Points:  3
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor2
-------------------------------------------------+-------------------------

Comment (by asn):

 OK I'm trying to map out what needs to happen for this ticket, because
 there is lots of info in here:

 a) Need to see how circuitmapping should interact with the dormant
 subsystem. I think I can figure this out, maybe after speaking with Mike
 about the ideal behavior here.

 b) Need to add type-checking to the downcasting macros in prob_distr.c .
 Some of it was done in d82a8a7f9d268728b2447b2dbbaa346140784f9b but I'm
 not sure if this is good enough. Might need to think of the right API here
 to come up with the best plan. **Nick let me know if you have any ideas
 here, but please don't spend too much time, becaues I can probably figure
 it out too.**

 I was thinking of perhaps ditching the generic `dist` abstraction and only
 using the underlying functions (in a switch statement when needed (e.g. in
 the unittests)) so that we don't have type safety issues. Or otherwise
 adding some sort of magic field and introducing initialization functions
 for all the distributions, and then checking the magic field in the
 downcasting macro.

 c) `<+nickm> Also there are all these generic dist_ops functions that you
 could use ... but the API seems to encourage you not to use them. having
 wrapper functions instead that call dist->dist_ops.func(dist, ...) would
 be neat`

   I'm not sure what this comment calls for. **Can you please tell me some
 more about it?** Riastradh seems to expand on this on comment:5 but I
 cannot quite get it.

 d) `<+nickm> oh, one list thing: I think src/lib/math is not allowed to
 include lib/crypt_ops, since that would introduce a circularity. Better
 make sure that it doesn't`

   Is this circular dependency still a problem? i see that `src/lib/math/`
 includes `lib/crypt_ops` but I don't see the other way around.** Is this
 for future proofing this, or is there currently a problem?** Also, would
 we be OK with the suggestion that Riastradh gives in comment:5 where we
 split prob_distr.c into the computational part, and the sampling part
 which calls `crypto_rand()`?

 e) I should do the renaming that mike suggests in comment:10.

 f) not sure if I will have time to do the API change that comment:13
 recommends. Will see abou this.

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