[tor-bugs] #16943 [Tor]: Implement prop250 (Random Number Generation During Tor Voting)

[Tor Bug Tracker & Wiki]

#16943: Implement prop250 (Random Number Generation During Tor Voting)
-------------------------+------------------------------------
 Reporter:  asn          |          Owner:  dgoulet
     Type:  enhancement  |         Status:  needs_revision
 Priority:  High         |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor          |        Version:
 Severity:  Normal       |     Resolution:
 Keywords:  tor-hs       |  Actual Points:
Parent ID:  #8244        |         Points:  large
  Sponsor:  SponsorR     |
-------------------------+------------------------------------
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:40 dgoulet]:
 > Replying to [comment:38 teor]:
 > > I also get an assertion failure:
 > > {{{
 > > shared-random/utils: [forking]
 > >   FAIL /Users/twilsonb/tor/tor-sr/src/test/test_shared_random.c:957:
 assert(commit_is_authoritative(&commit, fp) == 0): 1 vs 0
 > >   [utils FAILED]
 > > }}}
 > I wonder if one of those options somehow removes the `memset(0)` just
 above the assert which would explain why 1 is returned instead of 0 else I
 can't see why this would return 1!

 The `memcpy(fp, commit.rsa_identity_fpr, sizeof(fp));` copies the
 uninitialised `commit.rsa_identity_fpr` into `fp`. Therefore, clang can
 assume that `commit.rsa_identity_fpr` is all zero (or, alternately,
 `commit.rsa_identity_fpr` uses memory that is initialised zero).
 Therefore, you get the same result for both `commit_is_authoritative`
 calls.

 > > I see undefined behaviour under 32 bit, possibly in tor_gmtime (which
 isn't mentioned below, but is in the tor backtrace):
 > > {{{
 > > 2   libsystem_c.dylib               0x9d7f5c34 abort + 156
 > > 3   test                            0x0057e5bf crash_handler + 383
 > > 4   libsystem_platform.dylib        0x990fb01b _sigtramp + 43
 > > 5   ???                             0xffffffff 0 + 4294967295
 > > 6   test                            0x0057e440 0x1000 + 5755968
 > > 7   test                            0x007d20eb parse_iso_time_ + 1739
 (util.c:1713)
 > > 8   test                            0x007d2134 parse_iso_time + 52
 (util.c:1723)
 > > 9   test                            0x00702157 config_assign_value +
 7239 (confparse.c:346)
 > > 10  test                            0x006fbba7 config_assign_line +
 4663 (confparse.c:529)
 > > 11  test                            0x006fa435 config_assign + 2133
 (confparse.c:828)
 > > 12  test                            0x009be851
 disk_state_load_from_disk_impl + 417 (shared_random_state.c:651)
 > > 13  test                            0x0073b9f5
 test_state_load_from_disk + 1525 (test_shared_random.c:617)
 > > }}}
 >
 > I'm not sure what to do about this one. I wonder if this actually also
 happens with our current state file since this is plain ISO TIME parsing.
 Nothing different between this state file and the one we have in terms of
 parsing timestamp...

 You pass 2666 as the year. Any years after 2037 overflow a 32 bit time_t.

 See also #18383 for a related issue in the dirauth code that was causing
 some assertion failures.

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