Skip to content

Commit 12972bd

Browse files
hoytechhoytech
committed
new index: replaceDeletion
- The previous method of detecting the deletion of PREs involved an arbitrarily long scan in the write path. This could have potentially been a DoS vector, since users can create an arbitrarily large number of deletion events for their own events.
1 parent 453bbee commit 12972bd

File tree

8 files changed

+92
-53
lines changed

8 files changed

+92
-53
lines changed

‎golpe‎

Submodule golpe updated 1 file

‎golpe.yaml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ features:
1010
templar: true
1111

1212
includes: |
13+
#include "golpe.h"
14+
#include "global.h"
1315
#include "PackedEvent.h"
1416
#include "EventUtils.h"
1517
@@ -47,6 +49,8 @@ tables:
4749
multi: true
4850
replace: # pubkey, d-tag, kind
4951
multi: true
52+
replaceDeletion: # hash(a-tag), created_at
53+
multi: true
5054

5155
indexPrelude: |
5256
PackedEventView packed(v.buf);
@@ -69,11 +73,11 @@ tables:
6973
}
7074
} else if (tagName == 'a') {
7175
if (packed.kind() == 5) {
72-
try {
76+
try { // parseATag can fail
7377
auto [kind, pubkey, dTag] = parseATag(tagVal);
7478
7579
if (isParamReplaceableKind(kind) && pubkey == packed.pubkey()) {
76-
replace.push_back(makeKey_StringUint64(pubkey + dTag, kind));
80+
replaceDeletion.push_back(makeKey_StringUint64(sha256(tagVal).sv(), packed.created_at()));
7781
}
7882
} catch(...) {
7983
}

‎src/Bytes32.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include <cstring>
4+
#include <string>
45
#include <string_view>
56

67
#include "golpe.h"
@@ -14,6 +15,10 @@ struct Bytes32 {
1415
memcpy(buf, s.data(), 32);
1516
}
1617

18+
std::string str() const {
19+
return std::string((char*)buf, 32);
20+
}
21+
1722
std::string_view sv() const {
1823
return std::string_view((char*)buf, 32);
1924
}

‎src/EventUtils.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
#include <string_view>
44
#include <string>
55
#include <tuple>
6-
#include <stdexcept>
6+
7+
#include "golpe.h"
78

89

910
inline bool isReplaceableKind(uint64_t kind) {
@@ -30,18 +31,18 @@ inline bool isEphemeralKind(uint64_t kind) {
3031

3132
inline std::tuple<uint64_t, std::string, std::string> parseATag(std::string_view input) {
3233
size_t firstColon = input.find(':');
33-
if (firstColon == std::string::npos) throw std::invalid_argument("parse error");
34+
if (firstColon == std::string::npos) throw herr("parse error");
3435

3536
std::string_view kindStr = input.substr(0, firstColon);
3637
size_t pos;
3738
uint64_t kind = std::stoull(std::string(kindStr), &pos);
38-
if (pos != kindStr.size()) throw std::invalid_argument("parse error");
39+
if (pos != kindStr.size()) throw herr("parse error");
3940

4041
size_t secondColon = input.find(':', firstColon + 1);
41-
if (secondColon == std::string::npos) throw std::invalid_argument("parse error");
42+
if (secondColon == std::string::npos) throw herr("parse error");
4243

4344
std::string_view pubkeyStr = input.substr(firstColon + 1, secondColon - firstColon - 1);
44-
if (pubkeyStr.size() != 64) throw std::invalid_argument("parse error");
45+
if (pubkeyStr.size() != 64) throw herr("parse error");
4546

4647
std::string dTag = std::string(input.substr(secondColon + 1));
4748

‎src/events.cpp‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#include <openssl/sha.h>
21
#include <negentropy.h>
32

43
#include "events.h"
@@ -23,7 +22,7 @@ std::string nostrJsonToPackedEvent(const tao::json::value &v) {
2322
uint64_t expiration = 0;
2423

2524
if (isReplaceableKind(kind)) {
26-
// Prepend virtual d-tag
25+
// Prepend virtual d-tag. Any later d-tags will be ignored during indexing
2726
tagBuilder.add('d', "");
2827
}
2928

@@ -58,7 +57,7 @@ std::string nostrJsonToPackedEvent(const tao::json::value &v) {
5857
}
5958

6059
if (isParamReplaceableKind(kind)) {
61-
// Append virtual d-tag
60+
// Append virtual d-tag. Will be overidden by any previous d-tags.
6261
tagBuilder.add('d', "");
6362
}
6463

@@ -84,10 +83,7 @@ Bytes32 nostrHash(const tao::json::value &origJson) {
8483

8584
std::string encoded = tao::json::to_string(arr);
8685

87-
unsigned char hash[SHA256_DIGEST_LENGTH];
88-
SHA256(reinterpret_cast<unsigned char*>(encoded.data()), encoded.size(), hash);
89-
90-
return Bytes32(std::string_view(reinterpret_cast<char*>(hash), SHA256_DIGEST_LENGTH));
86+
return sha256(encoded);
9187
}
9288

9389
bool verifySig(secp256k1_context* ctx, std::string_view sig, std::string_view hash, std::string_view pubkey) {
@@ -280,50 +276,52 @@ void writeEvents(lmdb::txn &txn, NegentropyFilterCache &neFilterCache, std::vect
280276
continue;
281277
}
282278

283-
{
279+
if (isReplaceableKind(packed.kind()) || isParamReplaceableKind(packed.kind())) {
284280
std::optional<std::string> replace;
285281

286-
if (isReplaceableKind(packed.kind()) || isParamReplaceableKind(packed.kind())) {
287-
packed.foreachTag([&](char tagName, std::string_view tagVal){
288-
if (tagName != 'd') return true;
289-
replace = std::string(tagVal);
290-
return false;
291-
});
292-
}
282+
packed.foreachTag([&](char tagName, std::string_view tagVal){
283+
if (tagName != 'd') return true;
284+
replace = std::string(tagVal);
285+
return false;
286+
});
293287

294288
if (replace) {
295289
auto searchStr = std::string(packed.pubkey()) + *replace;
296290
auto searchKey = makeKey_StringUint64(searchStr, packed.kind());
297291

292+
// Check if there is a newer event in the DB, or an older event to replace
293+
298294
env.generic_foreachFull(txn, env.dbi_Event__replace, searchKey, lmdb::to_sv<uint64_t>(MAX_U64), [&](auto k, auto v) {
299295
if (k != searchKey) return false;
300296

301297
auto otherEv = lookupEventByLevId(txn, lmdb::from_sv<uint64_t>(v));
302298
auto otherPacked = PackedEventView(otherEv.buf);
303299

304300
if (isEventABeforeEventB(packed, otherPacked)) {
305-
ev.status = otherPacked.kind() == 5 ? EventWriteStatus::Deleted : EventWriteStatus::Replaced;
306-
return false; // found more recent replacement event, no need to scan more
301+
ev.status = EventWriteStatus::Replaced;
302+
} else {
303+
if (logLevel >= 1) LI << "Deleting event (d-tag). id=" << to_hex(otherPacked.id());
304+
levIdsToDelete.push_back(otherEv.primaryKeyId);
307305
}
308306

309-
return true;
307+
return false;
310308
}, true);
311309

312-
// If event is accepted (pending write), then do a second pass to remove/unindex outdated events
310+
// If param-replaceable event is still accepted (pending write), check if there is a more recent deletion
313311

314-
if (ev.status == EventWriteStatus::Pending) {
315-
env.generic_foreachFull(txn, env.dbi_Event__replace, searchKey, lmdb::to_sv<uint64_t>(MAX_U64), [&](auto k, auto v) {
316-
if (k != searchKey) return false;
312+
if (isParamReplaceableKind(packed.kind()) && ev.status == EventWriteStatus::Pending) {
313+
auto searchStr = sha256(std::to_string(packed.kind()) + ":" + to_hex(packed.pubkey()) + ":" + *replace).str();
314+
auto searchKey = makeKey_StringUint64(searchStr, MAX_U64);
317315

318-
auto otherEv = lookupEventByLevId(txn, lmdb::from_sv<uint64_t>(v));
319-
auto otherPacked = PackedEventView(otherEv.buf);
316+
env.generic_foreachFull(txn, env.dbi_Event__replaceDeletion, searchKey, lmdb::to_sv<uint64_t>(MAX_U64), [&](auto k, auto v) {
317+
ParsedKey_StringUint64 parsedKey(k);
318+
if (parsedKey.s != searchStr) return false;
320319

321-
if (otherPacked.kind() != 5) {
322-
if (logLevel >= 1) LI << "Deleting event (d-tag). id=" << to_hex(otherPacked.id());
323-
levIdsToDelete.push_back(otherEv.primaryKeyId);
320+
if (parsedKey.n >= packed.created_at()) {
321+
ev.status = EventWriteStatus::Deleted;
324322
}
325323

326-
return true;
324+
return false;
327325
}, true);
328326
}
329327
}
@@ -351,14 +349,12 @@ void writeEvents(lmdb::txn &txn, NegentropyFilterCache &neFilterCache, std::vect
351349
auto otherEv = lookupEventByLevId(txn, lmdb::from_sv<uint64_t>(v));
352350
auto otherPacked = PackedEventView(otherEv.buf);
353351

354-
if (isEventABeforeEventB(otherPacked, packed)) {
355-
if (otherPacked.kind() != 5) {
356-
if (logLevel >= 1) LI << "Deleting replaceable event (kind 5). id=" << to_hex(otherPacked.id());
357-
levIdsToDelete.push_back(otherEv.primaryKeyId);
358-
}
352+
if (otherPacked.created_at() <= packed.created_at()) {
353+
if (logLevel >= 1) LI << "Deleting replaceable event (kind 5). id=" << to_hex(otherPacked.id());
354+
levIdsToDelete.push_back(otherEv.primaryKeyId);
359355
}
360356

361-
return true;
357+
return false;
362358
}, true);
363359
}
364360
} catch(...) {

‎src/global.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ using namespace phmap;
77

88

99
#include "constants.h"
10+
#include "Bytes32.h"
1011

1112

13+
Bytes32 sha256(std::string_view inp);
1214
std::string renderIP(std::string_view ipBytes);
1315
std::string renderSize(uint64_t si);
1416
std::string renderPercent(double p);

‎src/misc.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,20 @@
99
#include <algorithm>
1010
#include <string>
1111

12+
#include <openssl/sha.h>
13+
1214
#include "golpe.h"
1315

1416

17+
18+
19+
Bytes32 sha256(std::string_view inp) {
20+
unsigned char hash[SHA256_DIGEST_LENGTH];
21+
SHA256(reinterpret_cast<const unsigned char*>(inp.data()), inp.size(), hash);
22+
23+
return Bytes32(std::string_view(reinterpret_cast<const char*>(hash), SHA256_DIGEST_LENGTH));
24+
}
25+
1526
std::string renderIP(std::string_view ipBytes) {
1627
char buf[128];
1728

‎test/writeTest.pl‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -164,28 +164,48 @@
164164

165165

166166

167+
167168
doTest({
168-
desc => "Deletion by a-tag",
169+
desc => "Deletion by a-tag, same event blocked",
169170
events => [
170171
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "hello"},
171-
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "blah"},
172-
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "whatever"},
173-
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30001 --created-at 5000 --tag d "whatever"},
174-
qq{--sec $ids->[1]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "whatever"},
175-
qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 --tag a "30000:$ids->[0]->{pub}:hello" --tag a "30000:$ids->[0]->{pub}:whatever" --tag a "30000:$ids->[0]->{pub}:stuff"},
172+
qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 --tag a "30000:$ids->[0]->{pub}:hello"},
173+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "hello"}, # blocked
176174
],
177-
verify => [ 1, 3, 4, 5, ],
175+
verify => [ 1, ],
178176
});
179177

180178
doTest({
181-
desc => "Deletion by a-tag, can re-add later event",
179+
desc => "Deletion by a-tag, earlier event blocked",
182180
events => [
183181
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "hello"},
184182
qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 --tag a "30000:$ids->[0]->{pub}:hello"},
185-
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5500 --tag d "hello"}, ## before deletion: can't add
186-
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 6500 --tag d "hello"}, ## after deletion: can add
183+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5500 --tag d "hello"}, # blocked
187184
],
188-
verify => [ 1, 3, ],
185+
verify => [ 1, ],
186+
});
187+
188+
doTest({
189+
desc => "Deletion by a-tag, later event OK",
190+
events => [
191+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "hello"},
192+
qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 --tag a "30000:$ids->[0]->{pub}:hello"},
193+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 6500 --tag d "hello"}, ## after deletion: OK
194+
],
195+
verify => [ 1, 2, ],
196+
});
197+
198+
doTest({
199+
desc => "Deletion by a-tag, multiple tags",
200+
events => [
201+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "hello"},
202+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "blah"},
203+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "whatever"},
204+
qq{--sec $ids->[0]->{sec} --content "hi" --kind 30001 --created-at 5000 --tag d "whatever"},
205+
qq{--sec $ids->[1]->{sec} --content "hi" --kind 30000 --created-at 5000 --tag d "whatever"},
206+
qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 --tag a "30000:$ids->[0]->{pub}:hello" --tag a "30000:$ids->[0]->{pub}:whatever" --tag a "30000:$ids->[0]->{pub}:stuff"},
207+
],
208+
verify => [ 1, 3, 4, 5, ],
189209
});
190210

191211
doTest({

0 commit comments

Comments
 (0)