feat: dos protected topic relay msgs based on meta field

[alrevuelta]

Partially addresses #1612

Summary:

• Add new pubsub validator using the meta field for validation. This field can include an optional signature of the message injected from the application, making the relayer only propagate/accept messages with a valid signature included. Note that this is not part of the protocol and only configured from the application.
• TODO: Wait for RFC/BCP to merge: feat(rfc): add opt-in signed message dos protection vacp2p/rfc#588

[LNSD]

Are you planning to use the meta field for DoS protection? There is a discussion related to that here: vacp2p/rfc#573 (comment)

The idea is to use a generalized version of the proof field for DoS purposes instead of the meta field, which should be an application-specific field.

[LNSD]

Additionally, the "formalization" of the proof field was also discussed in this PR: waku-org/waku-proto#13 (comment)

In case we should need specific data to be associated with a message for relay layer DoS protections, I'd suggest introducing a new field for this.

This is the case for RLN. The RLN relay validator precise a proof Waku message attribute. Maybe that field could be generalized/standardised in a particular manner so it can be used for different purposes.

[alrevuelta]

@LNSD Thanks for the input. I actually haven't thought about it, was using meta since I initially thought it was a good idea, but goes against I would suggest using meta only for data the app layer wants to hand down and I have to do a couple of tricks to make it work (eg removing meta from the hash calculation).

So I was initially using meta because imho this message signing shouldn't be part of the protocol, and I saw it as some kinf of application level thingy (I see the key as an application level thing) but ofc mixed with the message hash, which clearly belong to the protocol.

So I'm fine with both of your suggestions:

• Using proof or a generalized version of it. A variable size array with a max and just try to decode it into an rlnProof or a a signature. Following this
• Adding a new field, msgSignature. However I'm not sure having a field for this would be nice, since it kind of expresses an intention, and we don't want messages to be signed.

So perhaps I would lean towards a generalized version of proof, but not 100% sure of the implications. cc @rymnc

[jm-clius]

I think it's important that (apart from the new field) the actual validation functionality gets removed from the protocol itself as much as possible. This imply some mechanism to pass validation-like functionality from the configured app (wakunode2 in this case) down to where relay is mounted, but I don't think the protocol should be aware of how app-level validations work (or even the configured public key). I know there may be some complexities that I'm missing here in getting this concept to work, but I would strive for a simple mechanism that works in this way.

The implementation itself should be much simpler if we don't try to do signing ourselves and don't have to deal with public keys.

[alrevuelta]

@jm-clius thanks for the comments, will try to move it to the app layer. Note that I added message signing for completeness, since it feels weird that the node can validate signed messages but not publish them.

[jm-clius]

Note that I added message signing for completeness, since it feels weird that the node can validate signed messages but not publish them.

I think the concept here is that from the nodes perspective it simply receives messages from the app (e.g. via the API) and publishes as-is. I also don't think the "node" should therefore do the validation - just be configurable to accept validation vectors from the app of which it is agnostic, therefore allowing app-defined validation.

[alrevuelta]

@jm-clius removed signing of messages and moved everything to the app.

[jm-clius]

Thanks for addressing the app-node separation concerns. I think this will work and keeps the node clean of application-level knowledge. One thing to note is that this still require the inverse (node/protocol-level knowledge on the application, e.g. the app "knowing" about node.wakuRelay and relay validators). This may just be an OK tradeoff to keep things simple for now?

If we need a more general approach I'd suggest something like defining the concept of an app-level validation function, which the node gets constructed with. When relay then gets constructed and mounted, it "translates" these app-defined validations to relay validators and adds them to its set. This would allow the app to remain ignorant of protocol/relay concepts and the protocol/relay to remain ignorant of app concepts (as it is in your implementation).

This idea would require a bit more structure so is perhaps overkill for now, but perhaps useful if we need general app-level validation in future.

The most important concern left then, I think, is to address the naming of the field we add to the WakuMessage. I don't think it should be a signature, as the protocol should be unaware of what this field contains. What do you think about appdata (or any better suggestions)?

[LNSD]

Please, @alrevuelta, could you answer my questions in the associated issue?

[alrevuelta]

The most important concern left then, I think, is to address the naming of the field we add to the WakuMessage. I don't think it should be a signature, as the protocol should be unaware of what this field contains. What do you think about appdata (or any better suggestions)?

Very open for namings. Have the feeling that appdata would be interpreted as the same as the existing meta defined as # Application specific metadata.. Perhaps it can be named validityProof and reused by also rln or future validators.

This idea would require a bit more structure so is perhaps overkill for now, but perhaps useful if we need general app-level validation in future.

Since the future of this feature is not very clear and I'm also figuring things out along the way, I would prefer to prioritize validating it + moving forward with scoring + trimming connections. Actually I wouldn't mind having a dedicated branch for this complete feature, where functionality >> convenctions, processes, patterns, etc. This would allow to iterate faster and first validate the idea before spending time in things that may be discarded. Once the idea is validated with a bunch of learnings along the way, it can be materialized in production ready code. wdyt?

[alrevuelta]

As agreed in a meeting, this PR will use the new meta field to store the signuture instead of creating a new field. Once the new meta is in place, this PR will use it.

[alrevuelta]

Using now the meta field + new app hash function as discussed, tested and ready to rumble.
can I get a review @jm-clius @LNSD ?
will follow up with metrics + cli config in upcoming PRs.

[jm-clius]

Approving to not block and require re-review, but comment below should be addressed first - think it's just something that went wrong when merging

[jm-clius]

I'm pretty sure this is a faulty merge conflict resolution. This should not be part of the except CatchableError block and the error-less except in L396 should be except CatchableError.

[alrevuelta]

indeed, very good catch thanks.

[alrevuelta]

@jm-clius addressed your last comment thanks!

can see other 5 commnets made at the same time but in old code, weird. i assume you resolved them and there is noting to address there, since it was already addressed before.