Subscription id too long

[etemiz]

I started a relayer wss://nos.lol

https://snort.social is having problems: [wss://nos.lol] NOTICE: ERROR: bad req: subscription id too long

https://astral.ninja seems to work better.

[hoytech]

Thanks for letting me know! Currently the subIds are restricted to 39 characters. This limit is an optimisation so that the subId can be stored inline in the subscription object I just checked snort and it's sending 45 characters.

I pushed an update to master that increases the limit to 63, please pull and rebuild.

BTW, thank you for your early beta testing! Just a warning: I have a change I'm working on that will require a DB rebuild. It saves 16 bytes per event and 16 bytes for every e/p tag. When I roll this out you'll have to strfry export and strfry import the DB.

[etemiz]

This worked well, thank you!

[pjv]

@hoytech running my shiny new strfry instance (with sub id max-length now at 63) and looking at logs I periodically still see this:

[Ingester 1 ]INFO| sending error to [1318]: bad req: subscription id too long

Taking a look at NIP-01 which is the only place that defines them at all, it says this:

<subscription_id> is a random string that should be used to represent a subscription.

I think either strfry needs to accept arbitrarily long subscription id’s or better, submit a PR to NIP-01 that defines the max length of a <subscription_id> so all clients and relays can safely optimize.

cc @fiatjaf

[hoytech]

The beta branch increases this limit to 71. How long are you seeing? (You can turn on verbose logging with relay.logging.dumpInReqs.

It's a shame that clients send such long subIds, since these are echoed back on every EVENT, so it adds up to a lot of wasted bandwidth.

There's a minor efficiency advantage to using a static sized buffer for this, but if really needed I can make it a configurable length.

[fiatjaf]

It's better to chase the clients and ask them to shorten their ids.

[pjv]

@hoytech I turned on verbose logging for reqs and saw a 100 char sub id. It has an 81 char random hash, then an underscore and then the url of my relay.

what about sha256-ing the incoming sub id string before storing - would that be too big a performance hit?

It's better to chase the clients and ask them to shorten their ids.

Really? Why not specify the max length of a subscription ID so relays can legitimately reject a potential bad actor vector?

[hoytech]

Unfortunately we can't hash the subIds because we need to echo it back to the client.

[pjv]

Unfortunately we can't hash the subIds because we need to echo it back to the client.

ah so.

Leaving the subscription_id so loosely defined in the protocol feels to me like a vulnerability to all kinds of potential bad client mischief.

[pjv]

FWIW, I did an nslookup on an IP address for a client that tried to connect and received the subId too long error and the IP resolved to api.nostr.watch.

[pjv]

Ya, so a little code spelunking in the nostr-watch github turns this up in many places:

subid = crypto.randomBytes(40).toString('hex')

…which yields an 81 character random string.

So @fiatjaf we should at this point ask @dskvr to make those subid’s shorter?

edit: also @hoytech : nostream is sending this to clients: "max_subid_length":256 in its NIP-11 info

see also: nostr-protocol/nips#240

[hoytech]

Thanks for looking into this! Good to know where that's coming from.

Using crypto.randomBytes() for this is unnecessary, and I think indicates we need some better education for client authors.

The max_subid_length NIP-11 response is interesting but IMO also unnecessary. If I were making a client I would just use sequential per-connection integers for my subscription IDs and not worry about this limit at all (as long as it's at least, say, 10).

[fiatjaf]

Nice job finding the culprit.