Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add timestamp and ephemeral for opt-in dos validator #1713

Merged
merged 4 commits into from
May 5, 2023
Merged

chore: add timestamp and ephemeral for opt-in dos validator #1713

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59bf092
chore: add timestamp and ephemeral for opt-in dos validator
alrevuelta May 4, 2023
6b512c4
chore: add timestamp validation in dos validator
alrevuelta May 4, 2023
3092a51
Dont import time, reuse from waku_core
alrevuelta May 5, 2023
da7814a
substract since its int64
alrevuelta May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions apps/wakunode2/wakunode2_validator_signed.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@ else:
{.push raises: [].}

import
std/[math,times],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should try to avoid importing std/times (especially when we use chronos.timer constructs in the same module). In this case we need it for absolute system time, but waku_core/time provides the wrappers for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 3092a51

chronicles,
chronos,
metrics,
stew/byteutils,
stew/endians2,
libp2p/protocols/pubsub/gossipsub,
libp2p/protocols/pubsub/rpc/messages,
libp2p/protocols/pubsub/errors,
nimcrypto/sha2,
secp256k1

const MessageWindowInSec = 5*60 # +- 5 minutes

import
../../waku/v2/waku_relay/protocol,
../../waku/v2/waku_core
Expand All @@ -29,9 +33,26 @@ proc msgHash*(pubSubTopic: string, msg: WakuMessage): array[32, byte] =
ctx.update(pubsubTopic.toBytes())
ctx.update(msg.payload)
ctx.update(msg.contentTopic.toBytes())
ctx.update(msg.timestamp.uint64.toBytes(Endianness.littleEndian))
ctx.update(if msg.ephemeral: @[1.byte] else: @[0.byte])

return ctx.finish()

proc withinTimeWindow*(msg: WakuMessage): bool =
# Returns true if the message timestamp is:
# abs(now - msg.timestamp) < MessageWindowInSec
let ts = msg.timestamp
let now = getNanosecondTime(getTime().toUnixFloat())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waku_core (exporting time) already provides getNowInNanosecondTime() so you don't have to import std/times

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes right, i keep forgeting.

fixed 3092a51

let window = getNanosecondTime(MessageWindowInSec)

if now > ts:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open for other ways.

cant do abs(now-ts) since im working with uint64 and if I cast to signed, I will lose precision. So its the way I found.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe https://nim-lang.org/docs/times.html#abs%2CDuration from times can be used

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, mabe it is a completely stupid question, but would losing precision matter? Is the concern that someone could make the difference between TS and now overflow and thus the result of abs() be < window?

Can we check for overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ou between is indeed what I needed, but since in nwaku we generally use our own Timestamp (and not te Duration) I think this may be simpler. Less castings, etc.

Sorry, mabe it is a completely stupid question, but would losing precision matter? Is the concern that someone could make the difference between TS and now overflow and thus the result of abs() be < window?

mm havent done the maths if matters or not. Difference between ts and now will never overflow, since it will be always a smaller value. But since both values are uint64 I need to do ts-now or now-ts to ensure the result is not negative. Hence why i compare.

Mathematically is just abs(now-ts) but in practice thats not possible afaik without converting to int64 with a possible lose in precission.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, but Timestamp is an alias for int64 not uint64, unless I'm missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch indeed. Totally assumed it was uint64, since otherwise it would be a waste of bytes (assuming negative timestamps doesnt make sense). my bad.

fixed, way easier da7814a

some quick napkin math @vpavlin int64 overflow with 9223372036854775807, since its nano seconds thats 9223372036 . 854775807 and 9223372036 in unix timestamp its Fri Apr 11 2262 23:47:16 GMT+0000 so we are covered.

thanks all!

if now - ts < window:
return true
else:
if ts - now < window:
return true
return false

proc addSignedTopicValidator*(w: WakuRelay, topic: PubsubTopic, publicTopicKey: SkPublicKey) =
debug "adding validator to signed topic", topic=topic, publicTopicKey=publicTopicKey

Expand All @@ -40,11 +61,13 @@ proc addSignedTopicValidator*(w: WakuRelay, topic: PubsubTopic, publicTopicKey:
var outcome = errors.ValidationResult.Reject

if msg.isOk():
let msgHash = SkMessage(topic.msgHash(msg.get))
let recoveredSignature = SkSignature.fromRaw(msg.get.meta)
if recoveredSignature.isOk():
if recoveredSignature.get.verify(msgHash, publicTopicKey):
outcome = errors.ValidationResult.Accept
if msg.get.timestamp != 0:
if msg.get.withinTimeWindow():
let msgHash = SkMessage(topic.msgHash(msg.get))
let recoveredSignature = SkSignature.fromRaw(msg.get.meta)
if recoveredSignature.isOk():
if recoveredSignature.get.verify(msgHash, publicTopicKey):
outcome = errors.ValidationResult.Accept

waku_msg_validator_signed_outcome.inc(labelValues = [$outcome])
return outcome
Expand Down
49 changes: 38 additions & 11 deletions tests/wakunode2/test_validators.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ suite "WakuNode2 - Validators":
# Stop all nodes
await allFutures(nodes.mapIt(it.stop()))

asyncTest "Spam protected topic rejects non-signed and wrongly-signed messages":
asyncTest "Spam protected topic rejects non-signed/wrongly-signed/no-timestamp messages":
# Create 5 nodes
let nodes = toSeq(0..<5).mapIt(newTestWakuNode(generateSecp256k1Key(), ValidIpAddress.init("0.0.0.0"), Port(0)))

Expand Down Expand Up @@ -156,23 +156,49 @@ suite "WakuNode2 - Validators":
version: 2, timestamp: now(), ephemeral: true)
await nodes[i].publish(spamProtectedTopic, unsignedMessage)

# Each node sends 10 messages that dont contain timestamp (total = 50)
for i in 0..<5:
for j in 0..<10:
let unsignedMessage = WakuMessage(
payload: urandom(1*(10^3)), contentTopic: spamProtectedTopic,
version: 2, timestamp: 0, ephemeral: true)
await nodes[i].publish(spamProtectedTopic, unsignedMessage)

# Each node sends 10 messages way BEFORE than the current timestmap (total = 50)
for i in 0..<5:
for j in 0..<10:
let beforeTimestamp = now() - getNanosecondTime(6*60)
let unsignedMessage = WakuMessage(
payload: urandom(1*(10^3)), contentTopic: spamProtectedTopic,
version: 2, timestamp: beforeTimestamp, ephemeral: true)
await nodes[i].publish(spamProtectedTopic, unsignedMessage)

# Each node sends 10 messages way LATER than the current timestmap (total = 50)
for i in 0..<5:
for j in 0..<10:
let afterTimestamp = now() - getNanosecondTime(6*60)
let unsignedMessage = WakuMessage(
payload: urandom(1*(10^3)), contentTopic: spamProtectedTopic,
version: 2, timestamp: afterTimestamp, ephemeral: true)
await nodes[i].publish(spamProtectedTopic, unsignedMessage)

# Wait for gossip
await sleepAsync(2.seconds)

# Since we have a full mesh with 5 nodes and each one publishes 50+50 msgs
# there are 500 messages being sent.
# 100 are received ok in the handler (first hop)
# 400 are are wrong so rejected (rejected not relayed)
# Since we have a full mesh with 5 nodes and each one publishes 50+50+50+50+50 msgs
# there are 1250 messages being sent.
# 250 are received ok in the handler (first hop)
# 1000 are are wrong so rejected (rejected not relayed)
check:
msgReceived == 100
msgReceived == 250

var msgRejected = 0
for i in 0..<5:
for k, v in nodes[i].wakuRelay.peerStats.mpairs:
msgRejected += v.topicInfos[spamProtectedTopic].invalidMessageDeliveries.int

check:
msgRejected == 400
msgRejected == 1000

await allFutures(nodes.mapIt(it.stop()))

Expand Down Expand Up @@ -273,10 +299,12 @@ suite "WakuNode2 - Validators":
let contentTopic = "content-topic"
let pubsubTopic = "pubsub-topic"
let payload = "1A12E077D0E89F9CAC11FBBB6A676C86120B5AD3E248B1F180E98F15EE43D2DFCF62F00C92737B2FF6F59B3ABA02773314B991C41DC19ADB0AD8C17C8E26757B"
let timestamp = 1683208172339052800
let ephemeral = true

# expected values
let expectedMsgAppHash = "0914369D6D0C13783A8E86409FE42C68D8E8296456B9A9468C845006BCE5B9B2"
let expectedSignature = "B139487797A242291E0DD3F689777E559FB749D565D55FF202C18E24F21312A555043437B4F808BB0D21D542D703873DC712D76A3BAF1C5C8FF754210D894AD4"
let expectedMsgAppHash = "662F8C20A335F170BD60ABC1F02AD66F0C6A6EE285DA2A53C95259E7937C0AE9"
let expectedSignature = "127FA211B2514F0E974A055392946DC1A14052182A6ABEFB8A6CD7C51DA1BF2E40595D28EF1A9488797C297EED3AAC45430005FB3A7F037BDD9FC4BD99F59E63"

let secretKey = SkSecretKey.fromHex(privateKey).expect("valid key")

Expand All @@ -286,12 +314,11 @@ suite "WakuNode2 - Validators":

var msg = WakuMessage(
payload: payload.fromHex(), contentTopic: contentTopic,
version: 2, timestamp: now(), ephemeral: true)
version: 2, timestamp: timestamp, ephemeral: ephemeral)

let msgAppHash = pubsubTopic.msgHash(msg)
let signature = secretKey.sign(SkMessage(msgAppHash)).toRaw()

check:
msgAppHash.toHex() == expectedMsgAppHash
signature.toHex() == expectedSignature

2 changes: 1 addition & 1 deletion waku/v2/waku_core/message/message.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type WakuMessage* = object
# Number to discriminate different types of payload encryption.
# Compatibility with Whisper/WakuV1.
version*: uint32
# Sender generated timestamp. Deprecated. Superseded by `meta` attribute.
# Sender generated timestamp.
timestamp*: Timestamp
# The ephemeral attribute indicates signifies the transient nature of the
# message (if the message should be stored).
Expand Down