Skip to content

Commit 301acc6

Browse files
tomt1664tomt1664
committed
Merge #1604: [master] sigcache: harden range proof cache keys and add -norangeproofcache option
53167b1 init: add -norangeproofcache startup option to disable range proof cache (Byron Hambly) 5e5f8e6 sigcache: add proof cache key collision-resistance tests (Byron Hambly) b0a2752 sigcache: harden proof cache keys with length-prefixed hashing (Byron Hambly) Pull request description: target is **master** branch, cherry-picked commits from #1600 Switch the range-proof and surjection-proof cache hashers from raw CSHA256 concatenation to CHashWriter . This length-prefixes every field, so distinct argument tuples with byte-identical raw concatenations no longer collide to the same cache key. Both caches retain per-process salted midstates with distinct domain separators ('r' / 's'). Also adds a -norangeproofcache startup option to disable the range proof cache without recompiling. ACKs for top commit: tomt1664: ACK 53167b1 tested locally Tree-SHA512: 35412700e9773437b67b5ef350e752201856afab5e04e850ee6f9abe2aa4dc5127f5e9c13278c66236193f3279f34ec437f603ade6ba63e8149abd6f364c0246
2 parents c7e856f + 53167b1 commit 301acc6

5 files changed

Lines changed: 270 additions & 33 deletions

File tree

‎src/init.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
636636
argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
637637
argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
638638
argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_VALIDATION_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
639+
argsman.AddArg("-rangeproofcache", strprintf("Enable the range proof validation cache (default: %u). Use -norangeproofcache to disable.", 1), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
639640
argsman.AddArg("-maxtipage=<n>",
640641
strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)",
641642
Ticks<std::chrono::seconds>(DEFAULT_MAX_TIP_AGE)),

‎src/script/sigcache.cpp‎

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55

66
#include <script/sigcache.h>
77

8+
#include <common/args.h>
89
#include <crypto/sha256.h>
10+
#include <hash.h>
911
#include <logging.h>
1012
#include <pubkey.h>
1113
#include <random.h>
@@ -20,10 +22,8 @@
2022
SignatureCache::SignatureCache(const size_t max_size_bytes)
2123
{
2224
uint256 nonce = GetRandHash();
23-
// We want the nonce to be 64 bytes long to force the hasher to process
24-
// this chunk, which makes later hash computations more efficient. We
25-
// just write our 32-byte entropy, and then pad with 'E' for ECDSA and
26-
// 'S' for Schnorr (followed by 0 bytes).
25+
// Use 64-byte, type-specific salted midstates so later hash computations
26+
// can start after the first SHA256 chunk.
2727
static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
2828
static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
2929
static constexpr unsigned char PADDING_RANGE_PROOF[32] = {'r'};
@@ -32,10 +32,8 @@ SignatureCache::SignatureCache(const size_t max_size_bytes)
3232
m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
3333
m_salted_hasher_schnorr.Write(nonce.begin(), 32);
3434
m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
35-
m_salted_hasher_range_proof.Write(nonce.begin(), 32);
36-
m_salted_hasher_range_proof.Write(PADDING_RANGE_PROOF, 32);
37-
m_salted_hasher_surjection_proof.Write(nonce.begin(), 32);
38-
m_salted_hasher_surjection_proof.Write(PADDING_SURJECTION_PROOF, 32);
35+
m_salted_hasher_range_proof << nonce << PADDING_RANGE_PROOF;
36+
m_salted_hasher_surjection_proof << nonce << PADDING_SURJECTION_PROOF;
3937

4038
const auto [num_elems, approx_size_bytes] = setValid.setup_bytes(max_size_bytes);
4139
LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
@@ -55,13 +53,41 @@ void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Sp
5553
}
5654

5755
// ELEMENTS:
58-
void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const {
59-
CSHA256 hasher = m_salted_hasher_range_proof;
60-
hasher.Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Write(asset_commitment.data(), asset_commitment.size()).Write(scriptPubKey.data(), scriptPubKey.size()).Finalize(entry.begin());
56+
void SignatureCache::ComputeEntryRangeProof(uint256& entry,
57+
const std::vector<unsigned char>& proof,
58+
const std::vector<unsigned char>& commitment,
59+
const std::vector<unsigned char>& asset_commitment,
60+
const CScript& script_pub_key) const
61+
{
62+
HashWriter hasher = m_salted_hasher_range_proof;
63+
// We commit to both commitments and the scriptPubKey because these are
64+
// committed to by the rangeproof itself; a change in any of them would
65+
// invalidate the proof. Since these are exactly the arguments to
66+
// CachingRangeProofChecker::VerifyRangeProof (below), there is no
67+
// additional data that could affect the rangeproof's validity.
68+
// Serialization length-prefixes every field, including the variable-length
69+
// proof and script, so distinct argument tuples cannot share an encoding.
70+
hasher << proof << commitment << asset_commitment << script_pub_key;
71+
entry = hasher.GetSHA256();
6172
}
62-
void SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const {
63-
CSHA256 hasher = m_salted_hasher_surjection_proof;
64-
hasher.Write(hash.begin(), 32).Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Finalize(entry.begin());
73+
74+
void SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const
75+
{
76+
HashWriter hasher = m_salted_hasher_surjection_proof;
77+
// We hash all arguments passed to CachingSurjectionProofChecker::VerifySurjectionProof,
78+
// to ensure that any change in the way that the verification function is called will
79+
// trigger a cache miss and explicit verification. However, we note that the `wtxid`
80+
// (hash) commits to all the other data such that we could technically hash only it.
81+
// We retain the other data as a defense against future refactorings.
82+
//
83+
// Serialize vTags as a flat byte vector (each secp256k1_generator is 64 bytes).
84+
std::vector<unsigned char> vTagsBytes;
85+
vTagsBytes.reserve(vTags.size() * 64);
86+
for (const auto& tag : vTags) {
87+
vTagsBytes.insert(vTagsBytes.end(), std::begin(tag.data), std::end(tag.data));
88+
}
89+
hasher << hash << proof << commitment << vTagsBytes;
90+
entry = hasher.GetSHA256();
6591
}
6692

6793
bool SignatureCache::Get(const uint256& entry, const bool erase)
@@ -109,6 +135,10 @@ namespace {
109135
// To be called once in AppInit2/TestingSetup to initialize the rangeproof cache
110136
bool InitRangeproofCache(size_t max_size_bytes)
111137
{
138+
if (!gArgs.GetBoolArg("-rangeproofcache", true)) {
139+
LogPrintf("Range proof cache disabled via -norangeproofcache\n");
140+
return true;
141+
}
112142
auto setup_results = rangeProofCache.setup_bytes(max_size_bytes);
113143
if (!setup_results) return false;
114144
const auto [num_elems, approx_size_bytes] = *setup_results;
@@ -130,11 +160,18 @@ bool InitSurjectionproofCache(size_t max_size_bytes)
130160

131161
bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>& vchRangeProof, const std::vector<unsigned char>& vchValueCommitment, const std::vector<unsigned char>& vchAssetCommitment, const CScript& scriptPubKey, const secp256k1_context* secp256k1_ctx_verify_amounts) const
132162
{
163+
// ELEMENTS: NOTE FOR FUTURE EDITORS: every argument to this function that
164+
// carries data (i.e. everything except the secp256k1 context, which is
165+
// stateless) MUST be included in ComputeEntryRangeProof. Omitting any
166+
// argument risks returning a cached positive result for a proof that was
167+
// verified with different inputs.
133168
uint256 entry;
134-
rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey);
135-
136-
if (rangeProofCache.Get(entry, !store)) {
137-
return true;
169+
const bool useCache = gArgs.GetBoolArg("-rangeproofcache", true);
170+
if (useCache) {
171+
rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey);
172+
if (rangeProofCache.Get(entry, !store)) {
173+
return true;
174+
}
138175
}
139176

140177
if (vchRangeProof.size() == 0) {
@@ -163,7 +200,7 @@ bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>
163200
return false;
164201
}
165202

166-
if (store) {
203+
if (useCache && store) {
167204
rangeProofCache.Set(entry);
168205
}
169206

@@ -182,7 +219,7 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr
182219
// wtxid commits to all data including surj targets
183220
// we need to specify the proof and output asset point to be unique
184221
uint256 entry;
185-
surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector<unsigned char>(std::begin(gen.data), std::end(gen.data)));
222+
surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector<unsigned char>(std::begin(gen.data), std::end(gen.data)), vTags);
186223

187224
if (surjectionProofCache.Get(entry, !store)) {
188225
return true;
@@ -199,5 +236,24 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr
199236
return true;
200237
}
201238

239+
// Test-only hooks (see sigcache.h). Forward to the anonymous-namespace caches.
240+
void TestComputeEntryRangeProof(uint256& entry,
241+
const std::vector<unsigned char>& proof,
242+
const std::vector<unsigned char>& commitment,
243+
const std::vector<unsigned char>& asset_commitment,
244+
const CScript& script_pub_key)
245+
{
246+
rangeProofCache.ComputeEntryRangeProof(entry, proof, commitment, asset_commitment, script_pub_key);
247+
}
248+
249+
void TestComputeEntrySurjectionProof(uint256& entry,
250+
const uint256& hash,
251+
const std::vector<unsigned char>& proof,
252+
const std::vector<unsigned char>& commitment,
253+
const std::vector<secp256k1_generator>& vTags)
254+
{
255+
surjectionProofCache.ComputeEntrySurjectionProof(entry, hash, proof, commitment, vTags);
256+
}
257+
202258
// END ELEMENTS
203259
//

‎src/script/sigcache.h‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <consensus/amount.h>
1010
#include <crypto/sha256.h>
1111
#include <cuckoocache.h>
12+
#include <hash.h>
1213
#include <script/interpreter.h>
1314
#include <random.h>
1415
#include <span.h>
@@ -43,11 +44,11 @@ static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES +
4344
class SignatureCache
4445
{
4546
private:
46-
//! Entries are SHA256(nonce || 'E' or 'S' || 31 zero bytes || signature hash || public key || signature):
47+
//! Salted SHA256 midstates, domain-separated by signature or proof type.
4748
CSHA256 m_salted_hasher_ecdsa;
4849
CSHA256 m_salted_hasher_schnorr;
49-
CSHA256 m_salted_hasher_range_proof;
50-
CSHA256 m_salted_hasher_surjection_proof;
50+
HashWriter m_salted_hasher_range_proof;
51+
HashWriter m_salted_hasher_surjection_proof;
5152
typedef CuckooCache::cache<uint256, SignatureCacheHasher> map_type;
5253
map_type setValid;
5354
std::shared_mutex cs_sigcache;
@@ -56,10 +57,8 @@ class SignatureCache
5657
SignatureCache()
5758
{
5859
uint256 nonce = GetRandHash();
59-
// We want the nonce to be 64 bytes long to force the hasher to process
60-
// this chunk, which makes later hash computations more efficient. We
61-
// just write our 32-byte entropy, and then pad with 'E' for ECDSA and
62-
// 'S' for Schnorr (followed by 0 bytes).
60+
// Use 64-byte, type-specific salted midstates so later hash computations
61+
// can start after the first SHA256 chunk.
6362
static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
6463
static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
6564
static constexpr unsigned char PADDING_RANGE_PROOF[32] = {'r'};
@@ -68,10 +67,8 @@ class SignatureCache
6867
m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
6968
m_salted_hasher_schnorr.Write(nonce.begin(), 32);
7069
m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
71-
m_salted_hasher_range_proof.Write(nonce.begin(), 32);
72-
m_salted_hasher_range_proof.Write(PADDING_RANGE_PROOF, 32);
73-
m_salted_hasher_surjection_proof.Write(nonce.begin(), 32);
74-
m_salted_hasher_surjection_proof.Write(PADDING_SURJECTION_PROOF, 32);
70+
m_salted_hasher_range_proof << nonce << PADDING_RANGE_PROOF;
71+
m_salted_hasher_surjection_proof << nonce << PADDING_SURJECTION_PROOF;
7572
}
7673

7774
SignatureCache(size_t max_size_bytes);
@@ -84,9 +81,13 @@ class SignatureCache
8481
void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const;
8582

8683
// ELEMENTS:
87-
void ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const;
84+
void ComputeEntryRangeProof(uint256& entry,
85+
const std::vector<unsigned char>& proof,
86+
const std::vector<unsigned char>& commitment,
87+
const std::vector<unsigned char>& asset_commitment,
88+
const CScript& script_pub_key) const;
8889

89-
void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const;
90+
void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const;
9091

9192
bool Get(const uint256& entry, const bool erase);
9293

@@ -145,6 +146,20 @@ class CachingSurjectionProofChecker
145146
[[nodiscard]] bool InitRangeproofCache(size_t max_size_bytes);
146147
[[nodiscard]] bool InitSurjectionproofCache(size_t max_size_bytes);
147148

149+
// Test-only hooks: expose the (anonymous-namespace) cache-entry computation so
150+
// unit tests can verify collision-resistance and domain separation. These are
151+
// NOT part of the consensus/validation API and are only used by unit tests.
152+
void TestComputeEntryRangeProof(uint256& entry,
153+
const std::vector<unsigned char>& proof,
154+
const std::vector<unsigned char>& commitment,
155+
const std::vector<unsigned char>& asset_commitment,
156+
const CScript& script_pub_key);
157+
void TestComputeEntrySurjectionProof(uint256& entry,
158+
const uint256& hash,
159+
const std::vector<unsigned char>& proof,
160+
const std::vector<unsigned char>& commitment,
161+
const std::vector<secp256k1_generator>& vTags);
162+
148163
// END ELEMENTS
149164
//
150165

‎src/test/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ add_executable(test_elements
9696
serfloat_tests.cpp
9797
serialize_tests.cpp
9898
settings_tests.cpp
99+
sigcache_tests.cpp
99100
sighash_tests.cpp
100101
sigopcount_tests.cpp
101102
skiplist_tests.cpp

0 commit comments

Comments
 (0)