use a random string as peer-name in mDNS #368
Conversation
There was a problem hiding this comment.
I am not an expert on mDNS. Proposal is fine by me.
Could you bump the spec version @marten-seemann?
//CC @dvc94ch as you have been working a bunch on the Rust mDNS implementation and thus might have opinions.
|
The problem is a different one. Why are peer ids allowed to be longer than 63 characters. Also the name can be segmented using the DNS segmentation algorithm if I recall. rust-libp2p does this. |
AFAIK this is only possible if someone decides to encode peerIDs as CIDs using a base smaller than 36. Perhaps there's a reasonable argument for decoupling here in the event that at some point it became necessary to switch the hash function used for keys to be something longer than sha2-256 that won't fit into a lowercase encoding.
What do you mean? Is there an example or a spec I can look at? |
so you mean a 512 bit hash for pq security?
so according to the spec a label is 63 characters [0]. rust-libp2p [1] works around it by splitting a peer_id longer than 63 characters into multiple labels and joining them as subdomains with a |
There are a variety of reasons I could imagine someone proposing a change in the hash function, but yes e.g. someone wanting a 512 bit one.
Ah, ya. I'm not sure what the tradeoffs are here. It might just work in the mDNS case, but I'm not sure what types of things the tooling around mDNS expects. We have a similar problem in go-ipfs (ipfs/kubo#7318) where the |
|
In any case that seems like an ipfs problem. Not sure why getting TLS certificates from some CA for |
|
Yes. My point was just that there is already evidence that DNS tooling exists that won't be happy with arbitrary |
|
My suggestion would be to leave it as is and design your own protocol once you have the bandwidth. The libp2p-mdns seems shoe horned onto an mdns spec. You can probably build something much simpler for discovering peers and their addresses in local networks using multicast, at least in terms of the packet format. |
Actyx has deployed the rust-libp2p mdns on Android/windows/Linux in the mission critical context of factories. I think it's pretty clear that it works on more than just my machine |
|
Is Actyx using the |
Probably. But we also don't care about mdns compatibility. We only care about discovering other local rust-libp2p nodes, and I presume that's the sentiment of many rust-libp2p users. |
|
I was trying to avoid having to solve the problem of how to map peer IDs to subdomains. All we need for mDNS is a unique identifier shorter than 64 characters, and this PR suggests the (probably) easiest solution to achieve this property. |
|
I guess it's backwards compatible with existing implementations as long as they use peer ids shorter than 64 characters |
There was a problem hiding this comment.
Looks sensible.
- While we don't need TLS wildcard certificates in mDNS context, subdomain splitting might cause issues with existing mDNS libraries.
- The simplified spec proposed here makes it easier to implement and reduces potential issues with naive mdns implementations that do not support subdomains, giving us backward-compatibility for free.
Small suggestions inline, but feel free to merge without them.
discovery/mdns.md
Outdated
| - `name`._p2p._udp.local IN TXT dnsaddr=/ip6/fe80::7573:b0a8:46b0:bfea/tcp/4001/p2p/`id` | ||
| - `name`._p2p._udp.local IN TXT dnsaddr=/ip4/192.168.178.21/tcp/4001/p2p/'id' | ||
| - `name`._p2p._udp.local IN TXT dnsaddr=/ip4/192.168.178.21/tcp/4001/p2p/`id` |
There was a problem hiding this comment.
nit; i assume the name should be renamed to peer-name here for consistency?
discovery/mdns.md
Outdated
|
|
||
| If the encoding of the peer's ID exceeds 63 characters, then the [Split at 63rd character](https://github.com/ipfs/in-web-browsers/issues/89#issue-341357014) workaround can be used. | ||
| As the this field doesn't carry any meaning, it is sufficient to ensure the uniqueness of this identifier. Peers SHOULD generate a random, lower-case alphanumeric string of least 32 characters in length when booting up their node. |
There was a problem hiding this comment.
Perhaps it's better to give people some hints to limit bad names with low entropy?
| As the this field doesn't carry any meaning, it is sufficient to ensure the uniqueness of this identifier. Peers SHOULD generate a random, lower-case alphanumeric string of least 32 characters in length when booting up their node. | |
| As the this field doesn't carry any meaning, it is sufficient to ensure the uniqueness of this identifier. Peers SHOULD generate a random, lower-case alphanumeric string of least 32 characters in length when booting up their node. Peers SHOULD not use their Peer ID here because a future Peer ID could exceed the DNS label limit of 63 characters. When access to reliable randomness is difficult, implementer can derive `peer-name` from Peer ID using a cryptographic hash function that produces digests with lower-case alphanumeric representation shorter than DNS label limit. |
There was a problem hiding this comment.
Thanks. I took the first part of your suggestion. I'm not too worried about peers without access to randomness.
marten-seemann
force-pushed
the
mdns-random-peer-name
branch
from
38df6b6
to
08b9a70
Compare
Friendly ping :) |
With libp2p/specs#368 the definition of the _peer name_ changed in the mDNS specification. > peer-name is the case-insensitive unique identifier of the peer, and is less > than 64 characters. > > As the this field doesn't carry any meaning, it is sufficient to ensure the > uniqueness of this identifier. Peers SHOULD generate a random, lower-case > alphanumeric string of least 32 characters in length when booting up their > node. Peers SHOULD NOT use their Peer ID here because a future Peer ID could > exceed the DNS label limit of 63 characters. https://github.com/libp2p/specs/blob/master/discovery/mdns.md This commit adjusts `libp2p-mdns` accordingly. Also see libp2p/go-libp2p#1222 for the corresponding change on the Golang side. Co-authored-by: Max Inden <mail@max-inden.de>
…p#2311) With libp2p/specs#368 the definition of the _peer name_ changed in the mDNS specification. > peer-name is the case-insensitive unique identifier of the peer, and is less > than 64 characters. > > As the this field doesn't carry any meaning, it is sufficient to ensure the > uniqueness of this identifier. Peers SHOULD generate a random, lower-case > alphanumeric string of least 32 characters in length when booting up their > node. Peers SHOULD NOT use their Peer ID here because a future Peer ID could > exceed the DNS label limit of 63 characters. https://github.com/libp2p/specs/blob/master/discovery/mdns.md This commit adjusts `libp2p-mdns` accordingly. Also see libp2p/go-libp2p#1222 for the corresponding change on the Golang side. Co-authored-by: Max Inden <mail@max-inden.de>
With libp2p/specs#368 the definition of the _peer name_ changed in the mDNS specification. > peer-name is the case-insensitive unique identifier of the peer, and is less > than 64 characters. > > As the this field doesn't carry any meaning, it is sufficient to ensure the > uniqueness of this identifier. Peers SHOULD generate a random, lower-case > alphanumeric string of least 32 characters in length when booting up their > node. Peers SHOULD NOT use their Peer ID here because a future Peer ID could > exceed the DNS label limit of 63 characters. https://github.com/libp2p/specs/blob/master/discovery/mdns.md This commit adjusts `libp2p-mdns` accordingly. Also see libp2p/go-libp2p#1222 for the corresponding change on the Golang side. Co-authored-by: Max Inden <mail@max-inden.de>
We don't use the
<peer-name>for anything, we only should make sure that there's no collision in the network (I think). This was part of the initial design requirements: libp2p/libp2p#28 (Proposal section).Instead of figuring out how to best encode a peer ID such that it fits into 63 characters, a problem we've struggled with for a very long time, we can just admit that we're only filling this field because the RFC requires us to do so, and use random garbage.