identify: specify max message size

[lidel]

This PR proposes specifying max size of Identify message to unify behavior across implementations.

Limit of 4096 bytes is based on current value in rust-libp2p, but we can pick something else. (I was not able to quickly find any limit in go-libp2p)

[Menduist]

LGTM, notes that this implies a limit on the SPR too

[mxinden]

Unless there are any objections, I will move forward here next week.

@lidel could you please bump the specification revision and accept @marten-seemann's suggestion above?

[lidel]

@mxinden done in https://github.com/libp2p/specs/pull/492/commits/e4cb42cc0237c96f5e01ec689b4d3cc516803a3b :)

[Nashatyrev]

Just for the reference here is Jvm-libp2p commit addressing the issue: https://github.com/libp2p/jvm-libp2p/commit/5e23467be5ed2dd6d960fe74414568a14fde5622 We set the Identify message size limit to 1Mb there.

May be 4Kb is too restrictive for general use? There could be applications with a lot of supported protocols which wouldn't fit to Identify.protocols field

From my perspective it's important to bound the size with any reasonable value. So may be 1Mb would be as safe as 4Kb.

[Nashatyrev]

E.g. Ethereum has just 6 RPC methods, but each of them

• is identified by ~64 symbol protocol name
• has several versions (distinct protocol for each version)
• during hardforks the total number of protocols might increase x2 (as the protocol name incorporates fork identifier)

So even now the protocols field could be as large as 2Kb. After adding 2-3 new method versions it could exceed 4Kb

I see that 4Kb is kind of 'recommendation' but just a bit worried that this limit could be hardcoded to some Libp2p implementation and could just stop working at some point. (upd): like it's currently done in Rust :)

[Winterhuman]

Maybe the maximum could be 16KiB to mimic the IPNS record max size? If IPNS is safe using 16KiB then it seems it should be a safe value in general

[Nashatyrev]

@Nashatyrev unless you feel strongly about bumping this value, I suggest we proceed here.

I'm totally fine with this 👍

[marten-seemann]

I disagree that setting a small message size is the way to go. This has already led us to implement a chunked encoding (https://github.com/libp2p/go-libp2p/pull/958/files). I would have hoped that the days of chunked encodings were behind us...

Regarding DoS defense, there's little advantage to splitting up a large message into multiple smaller ones. Yes, you get incremental verifiability, but this hardly counts as an advantage when we're dealing with chunks of a few kB.

Really, if we actually think that the maximum of data we can accept without risking exposing ourselves to a DoS attack is that close to sizes that are actually useful for the protocol, there's something wrong with the protocol (I do think that Identify is a bloated protocol, but that's a different story). We need to acknowledge: Shipping lists of supported protocols and lists of multiaddrs takes up space. A DoS protection limit should be set such that well-behaved implementations don't have to worry about running into this limit. DoS limits exist to deter malicious nodes, not to make the life of well-behaved nodes miserable.

Looking at memory consumption of typical a libp2p connection, a size limit of 4 kB seems extreme stingy. We allow stream muxers to consume up to 15 MB in both go-libp2p and rust-libp2p. A memory limit of 128 - 256 kB per Identify wouldn't be crazy, if the number of concurrent Identify streams is limited.