[SERVER-72366] Pass random seed to `srand` for unit tests Created: 22/Dec/22  Updated: 03/Jan/23  Resolved: 03/Jan/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Pierlauro Sciarelli Assignee: Billy Donahue
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates SERVER-43838 unit test: pseudorandom test seed option Backlog
Related
related to SERVER-72373 Ban `rand` Closed
Assigned Teams:
Service Arch
Operating System: ALL
Participants:

 Description   

In order to generate random-like numbers, srand is usually initialized to some distinctive runtime value. For example, in mongod main and mongos_main the seed is always based on the current timestamp XOR some random address.

In unit tests the generator is not seeded, so it's very difficult to generate real random sequences. There are a lot of unit tests relying on `std::rand()` (aka `rand()`) expecting to get nondeterministic behavior but that's not the case as shown by this unit test executed 3 times:

TEST_F(TmpSuite, StdRandNotRandomAtAll) {
    for (int i = 0; i < 5; i++) {
        logd("XOXO Generated random {}", rand());
    }
}

$ ./db_s_shard_server_test --filter StdRandNotRandomAtAll | grep XOXO
{"t":{"$date":"2022-12-22T12:12:25.263Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1804289383"}
{"t":{"$date":"2022-12-22T12:12:25.263Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 846930886"}
{"t":{"$date":"2022-12-22T12:12:25.263Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1681692777"}
{"t":{"$date":"2022-12-22T12:12:25.263Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1714636915"}
{"t":{"$date":"2022-12-22T12:12:25.263Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1957747793"}
 
$ ./db_s_shard_server_test --filter StdRandNotRandomAtAll | grep XOXO
{"t":{"$date":"2022-12-22T12:12:29.389Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1804289383"}
{"t":{"$date":"2022-12-22T12:12:29.389Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 846930886"}
{"t":{"$date":"2022-12-22T12:12:29.389Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1681692777"}
{"t":{"$date":"2022-12-22T12:12:29.389Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1714636915"}
{"t":{"$date":"2022-12-22T12:12:29.389Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1957747793"}
 
$ ./db_s_shard_server_test --filter StdRandNotRandomAtAll | grep XOXO
{"t":{"$date":"2022-12-22T12:12:37.409Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1804289383"}
{"t":{"$date":"2022-12-22T12:12:37.409Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 846930886"}
{"t":{"$date":"2022-12-22T12:12:37.409Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1681692777"}
{"t":{"$date":"2022-12-22T12:12:37.409Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1714636915"}
{"t":{"$date":"2022-12-22T12:12:37.409Z"},"s":"I",  "c":"-",        "id":0,       "ctx":"main","msg":"XOXO Generated random 1957747793"}



 Comments   
Comment by Billy Donahue [ 03/Jan/23 ]

This will be obsoleted by SERVER-72373, which bans std::rand altogether.

But even PseudoRandom-based generators need injectable seeds.
The request to do this is already captured by SERVER-43838, so marking the remaining concerns on this ticket as a duplicate of that one.

Comment by Billy Donahue [ 23/Dec/22 ]

antonio.fuschetto@mongodb.com I haven't heard about security defects from <random>.
are you talking about the C++ <random> API or the C <stdlib.h> rand API?

Comment by Antonio Fuschetto [ 23/Dec/22 ]

I've extensively used Synopsys Coverity in the past (for another company) and I recall that there was a default rule such that the use of the standard functions to generate random numbers causes critical security defects. Coverity's suggestion was to replace them with the Boost implementation. I don't know if this suggestion is still valid today, this implementation is probably included in the C+11, C14 or C+17 standard libraries today.

Comment by Billy Donahue [ 22/Dec/22 ]

Unit tests are currently relying on the default srand seed of 1. I don't think we should change this.

The existing behavior has a benefit of making repeatable unit tests.
The repeatability can be seen as a problem or a benefit.

If we want to change this we should do it in a structured way. We have platform/random.h as a place to hang such coordinating code.

It's also worth saying that std::rand shouldn't be used at all, and the C++11 <random> facilities are better.
std::rand is a poor generator, and it's also non-reentrant (it's mutating global state).

Every call to std::rand in server code or in tests is a bug that should be fixed.
The question is what exactly to replace them with.

Generated at Thu Feb 08 06:21:35 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.