Opened 9 months ago

Closed 8 months ago

#29732 closed enhancement (fixed)

Add full-fledged deterministic PRNG support for testing.

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: dgoulet-merge
Cc: asn Actual Points: 1
Parent ID: Points:
Reviewer: ahf Sponsor:

Description

In several places, the tests override crypto_rand() or crypto_fast_prng() for one reason or another. We should provide a standard way for the tests to do this.

Child Tickets

Change History (9)

comment:1 Changed 9 months ago by nickm

I have a branch called "ticket29732", but the CI won't pass until we merge #29668

comment:2 Changed 8 months ago by nickm

New version at ticket29732_v2 with PR at https://github.com/torproject/tor/pull/909 .

comment:3 Changed 8 months ago by nickm

Status: assignedneeds_review

comment:4 Changed 8 months ago by nickm

Actual Points: 1

comment:5 Changed 8 months ago by asn

Reviewer: catalyst

comment:6 Changed 8 months ago by ahf

Reviewer: catalystahf

Taking this per network-team meeting discussion.

comment:7 Changed 8 months ago by ahf

Status: needs_reviewmerge_ready

I think this code looks good and can be merged. I've been focusing on checking to ensure that the API cannot be used wrongly in a non-testing setup and it seems like the important functions here are all protected using the TOR_UNIT_TESTS guard. If the reseeding mechanism, outside of the unit test setup, is disabled, Tor will crash with an unreached assertion in "production", which is what we want I think.

comment:8 Changed 8 months ago by nickm

Keywords: dgoulet-merge added

comment:9 Changed 8 months ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

Merged! Great feature to have! Agree with ahf assessment, this can't be used in production afaict!

Note: See TracTickets for help on using tickets.