Malicious Nostr Relays

[TheAwiteb]

Do we have a mechanism to handle malicious relays? Since clients sometimes need to connect to unknown relays, for example, when sending events to a user's READ relays or fetching from their WRITE relays, how does the crate handle cases where a relay returns unrelated or unwanted content?

For instance, if I request user A's notes from a relay, but it returns NSFW notes that aren't from A, will the crate still pass those events to the client for display?

Additionally, when fetching events with a filter, I expect all returned events to match that filter. Does the crate verify this?

If not (based on my view on the source code, it doesn't), could we implement such a check, or would that be overcomplicating things?

cc: @yukibtc @DanConwayDev @dluvian @mikedilger
I'm curious to hear your thoughts on this.

[mikedilger]

I think software ought to check if the events from the relay match the filter that was sent. I believe I am doing that elsewhere.

For the other cases of bad behavior by a relay, I think you just have to have ways of noticing this and then blocking that relay.

[TheAwiteb]

For the other cases of bad behavior by a relay, I think you just have to have ways of noticing this and then blocking that relay.

Like what? I can’t think of anything else except that the events don't match the filter and what @yukibtc mentioned below.

[yukibtc]

Do we have a mechanism to handle malicious relays?

At the moment only these stuff is checked:

• the size of the received message (discard if too big, depending on the configured limit)
• if the event is expired
• the ID and signature (if the event was never verified before)

Any additional check can be implemented using the admission policy trait.

Additionally, when fetching events with a filter, I expect all returned events to match that filter

Yeah, @dluvian suggested this time ago but at the moment is not currently implemented. Some stuff must be adjusted before:

• currently only long-lived subscriptions are stored in a subscription map, so to be able to check if events match the subscriptions we have to save also the auto-closing ones.
• the NIP-50 maybe needs to be avoided by Filter::match_event (or made configurable), since it seems that the query of NIP-50 could be arbitrary in comparison to the others (query by id, query by author and kind, ...), to avoid the ban of non-malicious relays. Currently to check if an event matches NIP-50, I just check if the event's content contains the word given in the search field of the filter.

[yukibtc]

Just to recap, if the client enables Options::check_subscription_filter, which is enabled by default, the pooler will ban the relay upon finding an event that doesn’t match the filter. Additionally, Filter::match_event should ignore NIP-50.

Exactly. Also if it's received an event from a subscription that doesn't exist should be banned IMO.

[TheAwiteb]

The margin of error would be just for ban the relay, the non-matching events don't have to be included in the result/notifications. So, in the CLI case, the non-matching event will still not be received.

What I mean is, the CLI will connect to the relay just once. If the relay acts maliciously, it won’t get banned because it doesn’t hit the ban threshold. So, the CLI won’t receive RelayStatus::banned from those malicious relays. But it’s important to get that status because that’s how it can reject connecting to them later using AdmitPolicy::admit_connection.

[TheAwiteb]

To keep the changes more focused, I'll split this into three PRs: the first for Filter::match_event, the second for banning relays that send non-existent subscriptions, and the third for banning relays that return events that don't match the filter.

[TheAwiteb]

Also if it's received an event from a subscription that doesn't exist should be banned IMO.

What are your thoughts on simply ignoring the event and not broadcasting it as a notification?

I recently debugged an issue where the relay gets banned after an auto-closing subscription. It seems the relay responds after the timeout, which the subscription was deleted, or an event arrives after EOSE, and the pool considers it unrelated since the subscription is deleted after EOSE.

[yukibtc]

Also if it's received an event from a subscription that doesn't exist should be banned IMO.

What are your thoughts on simply ignoring the event and not broadcasting it as a notification?

I recently debugged an issue where the relay gets banned after an auto-closing subscription. It seems the relay responds after the timeout, which the subscription was deleted, or an event arrives after EOSE, and the pool considers it unrelated since the subscription is deleted after EOSE.

Agree

[TheAwiteb]

Resolved at #976, #979 and #981