Skip to content

Commit cdfb775

Browse files
laanwjlaanwj
committed
Merge #8914: Kill insecure_random and associated global state
5eaaa83 Kill insecure_random and associated global state (Wladimir J. van der Laan)
2 parents 0306978 + 5eaaa83 commit cdfb775

28 files changed

+92
-66
lines changed

‎src/addrman.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
358358
int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT);
359359
int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
360360
while (vvTried[nKBucket][nKBucketPos] == -1) {
361-
nKBucket = (nKBucket + insecure_rand()) % ADDRMAN_TRIED_BUCKET_COUNT;
362-
nKBucketPos = (nKBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
361+
nKBucket = (nKBucket + insecure_rand.rand32()) % ADDRMAN_TRIED_BUCKET_COUNT;
362+
nKBucketPos = (nKBucketPos + insecure_rand.rand32()) % ADDRMAN_BUCKET_SIZE;
363363
}
364364
int nId = vvTried[nKBucket][nKBucketPos];
365365
assert(mapInfo.count(nId) == 1);
@@ -375,8 +375,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
375375
int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
376376
int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
377377
while (vvNew[nUBucket][nUBucketPos] == -1) {
378-
nUBucket = (nUBucket + insecure_rand()) % ADDRMAN_NEW_BUCKET_COUNT;
379-
nUBucketPos = (nUBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
378+
nUBucket = (nUBucket + insecure_rand.rand32()) % ADDRMAN_NEW_BUCKET_COUNT;
379+
nUBucketPos = (nUBucketPos + insecure_rand.rand32()) % ADDRMAN_BUCKET_SIZE;
380380
}
381381
int nId = vvNew[nUBucket][nUBucketPos];
382382
assert(mapInfo.count(nId) == 1);

‎src/addrman.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ class CAddrMan
211211
//! secret key to randomize bucket select with
212212
uint256 nKey;
213213

214+
//! Source of random numbers for randomization in inner loops
215+
FastRandomContext insecure_rand;
216+
214217
//! Find an entry.
215218
CAddrInfo* Find(const CNetAddr& addr, int *pnId = NULL);
216219

‎src/main.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4762,6 +4762,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma
47624762
uint64_t hashAddr = addr.GetHash();
47634763
std::multimap<uint64_t, CNode*> mapMix;
47644764
const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60));
4765+
FastRandomContext insecure_rand;
47654766

47664767
auto sortfunc = [&mapMix, &hasher](CNode* pnode) {
47674768
if (pnode->nVersion >= CADDR_TIME_VERSION) {
@@ -4770,9 +4771,9 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma
47704771
}
47714772
};
47724773

4773-
auto pushfunc = [&addr, &mapMix, &nRelayNodes] {
4774+
auto pushfunc = [&addr, &mapMix, &nRelayNodes, &insecure_rand] {
47744775
for (auto mi = mapMix.begin(); mi != mapMix.end() && nRelayNodes-- > 0; ++mi)
4775-
mi->second->PushAddress(addr);
4776+
mi->second->PushAddress(addr, insecure_rand);
47764777
};
47774778

47784779
connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc));
@@ -5082,14 +5083,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
50825083
if (fListen && !IsInitialBlockDownload())
50835084
{
50845085
CAddress addr = GetLocalAddress(&pfrom->addr, pfrom->GetLocalServices());
5086+
FastRandomContext insecure_rand;
50855087
if (addr.IsRoutable())
50865088
{
50875089
LogPrint("net", "ProcessMessages: advertising address %s\n", addr.ToString());
5088-
pfrom->PushAddress(addr);
5090+
pfrom->PushAddress(addr, insecure_rand);
50895091
} else if (IsPeerAddrLocalGood(pfrom)) {
50905092
addr.SetIP(pfrom->addrLocal);
50915093
LogPrint("net", "ProcessMessages: advertising address %s\n", addr.ToString());
5092-
pfrom->PushAddress(addr);
5094+
pfrom->PushAddress(addr, insecure_rand);
50935095
}
50945096
}
50955097

@@ -6012,8 +6014,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
60126014

60136015
pfrom->vAddrToSend.clear();
60146016
vector<CAddress> vAddr = connman.GetAddresses();
6017+
FastRandomContext insecure_rand;
60156018
BOOST_FOREACH(const CAddress &addr, vAddr)
6016-
pfrom->PushAddress(addr);
6019+
pfrom->PushAddress(addr, insecure_rand);
60176020
}
60186021

60196022

@@ -6846,7 +6849,7 @@ bool SendMessages(CNode* pto, CConnman& connman)
68466849
// until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY.
68476850
else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->nextSendTimeFeeFilter &&
68486851
(currentFilter < 3 * pto->lastSentFeeFilter / 4 || currentFilter > 4 * pto->lastSentFeeFilter / 3)) {
6849-
pto->nextSendTimeFeeFilter = timeNow + (insecure_rand() % MAX_FEEFILTER_CHANGE_DELAY) * 1000000;
6852+
pto->nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000;
68506853
}
68516854
}
68526855
}

‎src/net.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ void AdvertiseLocal(CNode *pnode)
187187
if (addrLocal.IsRoutable())
188188
{
189189
LogPrint("net", "AdvertiseLocal: advertising address %s\n", addrLocal.ToString());
190-
pnode->PushAddress(addrLocal);
190+
FastRandomContext insecure_rand;
191+
pnode->PushAddress(addrLocal, insecure_rand);
191192
}
192193
}
193194
}

‎src/net.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,14 +735,14 @@ class CNode
735735
addrKnown.insert(_addr.GetKey());
736736
}
737737

738-
void PushAddress(const CAddress& _addr)
738+
void PushAddress(const CAddress& _addr, FastRandomContext &insecure_rand)
739739
{
740740
// Known checking here is only to save space from duplicates.
741741
// SendMessages will filter it again for knowns that were added
742742
// after addresses were pushed.
743743
if (_addr.IsValid() && !addrKnown.contains(_addr.GetKey())) {
744744
if (vAddrToSend.size() >= MAX_ADDR_TO_SEND) {
745-
vAddrToSend[insecure_rand() % vAddrToSend.size()] = _addr;
745+
vAddrToSend[insecure_rand.rand32() % vAddrToSend.size()] = _addr;
746746
} else {
747747
vAddrToSend.push_back(_addr);
748748
}

‎src/netbase.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,8 @@ static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDe
596596
// do socks negotiation
597597
if (proxy.randomize_credentials) {
598598
ProxyCredentials random_auth;
599-
random_auth.username = strprintf("%i", insecure_rand());
600-
random_auth.password = strprintf("%i", insecure_rand());
599+
static std::atomic_int counter;
600+
random_auth.username = random_auth.password = strprintf("%i", counter++);
601601
if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket))
602602
return false;
603603
} else {

‎src/policy/fees.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
594594
CAmount FeeFilterRounder::round(CAmount currentMinFee)
595595
{
596596
std::set<double>::iterator it = feeset.lower_bound(currentMinFee);
597-
if ((it != feeset.begin() && insecure_rand() % 3 != 0) || it == feeset.end()) {
597+
if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) {
598598
it--;
599599
}
600600
return *it;

‎src/policy/fees.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include "amount.h"
99
#include "uint256.h"
10+
#include "random.h"
1011

1112
#include <map>
1213
#include <string>
@@ -298,5 +299,6 @@ class FeeFilterRounder
298299

299300
private:
300301
std::set<double> feeset;
302+
FastRandomContext insecure_rand;
301303
};
302304
#endif /*BITCOIN_POLICYESTIMATOR_H */

‎src/random.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,22 +178,21 @@ uint256 GetRandHash()
178178
return hash;
179179
}
180180

181-
uint32_t insecure_rand_Rz = 11;
182-
uint32_t insecure_rand_Rw = 11;
183-
void seed_insecure_rand(bool fDeterministic)
181+
FastRandomContext::FastRandomContext(bool fDeterministic)
184182
{
185183
// The seed values have some unlikely fixed points which we avoid.
186184
if (fDeterministic) {
187-
insecure_rand_Rz = insecure_rand_Rw = 11;
185+
Rz = Rw = 11;
188186
} else {
189187
uint32_t tmp;
190188
do {
191189
GetRandBytes((unsigned char*)&tmp, 4);
192190
} while (tmp == 0 || tmp == 0x9068ffffU);
193-
insecure_rand_Rz = tmp;
191+
Rz = tmp;
194192
do {
195193
GetRandBytes((unsigned char*)&tmp, 4);
196194
} while (tmp == 0 || tmp == 0x464fffffU);
197-
insecure_rand_Rw = tmp;
195+
Rw = tmp;
198196
}
199197
}
198+

‎src/random.h

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,22 @@ uint256 GetRandHash();
2828
void GetStrongRandBytes(unsigned char* buf, int num);
2929

3030
/**
31-
* Seed insecure_rand using the random pool.
32-
* @param Deterministic Use a deterministic seed
31+
* Fast randomness source. This is seeded once with secure random data, but
32+
* is completely deterministic and insecure after that.
33+
* This class is not thread-safe.
3334
*/
34-
void seed_insecure_rand(bool fDeterministic = false);
35-
36-
/**
37-
* MWC RNG of George Marsaglia
38-
* This is intended to be fast. It has a period of 2^59.3, though the
39-
* least significant 16 bits only have a period of about 2^30.1.
40-
*
41-
* @return random value
42-
*/
43-
extern uint32_t insecure_rand_Rz;
44-
extern uint32_t insecure_rand_Rw;
45-
static inline uint32_t insecure_rand(void)
46-
{
47-
insecure_rand_Rz = 36969 * (insecure_rand_Rz & 65535) + (insecure_rand_Rz >> 16);
48-
insecure_rand_Rw = 18000 * (insecure_rand_Rw & 65535) + (insecure_rand_Rw >> 16);
49-
return (insecure_rand_Rw << 16) + insecure_rand_Rz;
50-
}
35+
class FastRandomContext {
36+
public:
37+
explicit FastRandomContext(bool fDeterministic=false);
38+
39+
uint32_t rand32() {
40+
Rz = 36969 * (Rz & 65535) + (Rz >> 16);
41+
Rw = 18000 * (Rw & 65535) + (Rw >> 16);
42+
return (Rw << 16) + Rz;
43+
}
44+
45+
uint32_t Rz;
46+
uint32_t Rw;
47+
};
5148

5249
#endif // BITCOIN_RANDOM_H

0 commit comments

Comments
 (0)