Opened 3 years ago

Last modified 21 months ago

#17800 new defect

Tor Unit Tests should TT_FORK before initialising global PRNG state

Reported by: teor Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Minor Keywords: easy tor-tests prng init mostly-harmless
Cc: Actual Points:
Parent ID: Points: medium
Reviewer: Sponsor:

Description

Tor unit tests are meant to TT_FORK before changing any global process state that can't be reverted.

In #17789, we discovered that one or more tests initialise the OpenSSL CSPRNG without forking first. (This can't be undone.)

While this isn't likely to cause any issues, (#17789 was marked as wontfix for other reasons), it would be nice to fix it eventually for correctness.

One way to do this is:

  • initialise a static variable to 0.
  • set it to 1 just after fork()ing via TT_FORK.
  • tor_assert() that it's 1 in functions that irreversibly modify global test process state, if TOR_UNIT_TESTS is defined:
    • crypto_early_init(), which seeds the OpenSSL CSPRNG,
    • (I'm sure many other init functions do similar things)
  • mark the tests that assert as TT_FORK

Marked as Low/Minor because this doesn't currently cause any issues, but if tests start failing due to interactions with previous global state, we should look at it again.

Child Tickets

Change History (3)

comment:1 Changed 3 years ago by nickm

Points: medium

comment:2 Changed 22 months ago by nickm

Milestone: Tor: very long termTor: unspecified

Batch modify: these tickets seem to be things that wouldn't actually be a big redesign or a big amount of work, so they belong "Unspecified", not "Very Long Term"

comment:3 Changed 21 months ago by nickm

Keywords: tor-tests prng init mostly-harmless added
Note: See TracTickets for help on using tickets.