Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ServiceName configurable in mdns #2898

Closed
lynic opened this issue Sep 13, 2022 · 14 comments
Closed

Make ServiceName configurable in mdns #2898

lynic opened this issue Sep 13, 2022 · 14 comments

Comments

@lynic
Copy link

lynic commented Sep 13, 2022

Description

In go implementation of mdns, servicename is configurable https://github.com/libp2p/go-libp2p/blob/1ad0a50be62b1e00388b9d85fc997d66e8910c9d/p2p/discovery/mdns/mdns.go#L53

Motivation

I would like to separate peers into different group in a local network, using mdns with different servicename will make it possible to group peers into different swarm.

Current Implementation

ServiceName is not configurable at MdnsConfig https://github.com/libp2p/rust-libp2p/blob/master/protocols/mdns/src/lib.rs#L59
It's hardcoded. https://github.com/libp2p/rust-libp2p/blob/master/protocols/mdns/src/lib.rs#L47

Are you planning to do it yourself in a pull request?

No, I'm newbie to libp2p and rust world.

@thomaseizinger
Copy link
Contributor

Hmm, this is in accordance with the spec: https://github.com/libp2p/specs/blob/master/discovery/mdns.md#definitions.

After searching the go-libp2p repository, I can't find any mention of that specified service name actually so it seems the go implementation is not spec confirm here.

I'd suggest opening an issue over at libp2p/specs to start a discussion on what we want here. For interoperability, we need some kind of identifier in the spec but the spec could allow for an override and only have the current value be the default.

Tagging author and interest group of mDNS spec: @richardschneider @yusefnapora @raulk @daviddias @jacobheun

@lynic
Copy link
Author

lynic commented Sep 15, 2022

In this example from go, seems it use a user input service_name for mdns https://github.com/libp2p/go-libp2p/blob/eff72c4afafc766a24dd306145ebd4721fe852df/examples/chat-with-mdns/main.go#L116

@thomaseizinger
Copy link
Contributor

In this example from go, seems it use a user input service_name for mdns libp2p/go-libp2p@eff72c4/examples/chat-with-mdns/main.go#L116

Correct :)

My point still stands, this is not in accordance with the spec. I'd like to have that resolved there before we change our code in here.

@stormshield-pj50
Copy link
Contributor

Hi, I recently had the same needs with libp2p-rust, that means mDNS partitioning by configuring service name. I have a patch ready for this. I can open a PR with the code if you like once the specifications are made clear.

@thomaseizinger
Copy link
Contributor

I don't know much about your usecase but here are some general thoughts:

I don't think it is a good idea to only partition a network using different mDNS service names.

A resilient app needs to be prepared to handle incoming connections from nodes regardless of how they found out about their connection details, meaning you might as well discover more peers via mDNS and only retain the connections to those nodes that you care about.

For example, you can use the identify protocol to communicate different capabilities between nodes and form your partitions that way once you have established a connection.

@stormshield-pj50
Copy link
Contributor

Our use case is having several libp2p vpn independent instances talking on the same local network, we don't want those independent vpns to be able to see each other. We agree we can filter at a higher level but prefer doing it earlier as possible in order to avoid security issues.

@thomaseizinger
Copy link
Contributor

Our use case is having several libp2p vpn independent instances talking on the same local network, we don't want those independent vpns to be able to see each other. We agree we can filter at a higher level but prefer doing it earlier as possible in order to avoid security issues.

mDNS is a discovery mechanism and does not offer any security protections. I can just as easily enumerate all IPs in the local network and try to connect to other nodes without discovering them via mDNS.

If you need to shield certain groups of nodes from each other, those nodes will need to implement a connection management policy where they only allow incoming connections from certain PeerIds for example. With that in place, it no longer matters how many nodes you discover through mDNS as only a specified set of nodes can establish a connection.

@thomaseizinger
Copy link
Contributor

We now have the ability to implement arbitrary connection management so I think the other usecases for configurable mDNS name is resolved.

@TimTinkers
Copy link

@thomaseizinger I have recently stumbled into this issue writing a new application using rust-libp2p. I've written an mDNS application that was unnecessarily attempting to communicate with a local kubo client. I did resolve the matter with Identify and explicit disconnection of the kubo peer, but there is definitely an elegance to simply not discovering peers that I don't want to discover in the first place instead of having to connect to them and then close that connection later.

@stormshield-frb
Copy link
Contributor

As @stormshield-pj50 mentioned some time ago, we had the same problem. We have resolved it in our fork with a patch allowing us to define the following fields constants:

const SERVICE_NAME: &[u8] = b"_p2p._udp.local";
/// `SERVICE_NAME` as a Fully Qualified Domain Name.
const SERVICE_NAME_FQDN: &str = "_p2p._udp.local.";
/// The meta query for looking up the `SERVICE_NAME`.
const META_QUERY_SERVICE: &[u8] = b"_services._dns-sd._udp.local";
/// `META_QUERY_SERVICE` as a Fully Qualified Domain Name.
const META_QUERY_SERVICE_FQDN: &str = "_services._dns-sd._udp.local.";

@thomaseizinger
Copy link
Contributor

thomaseizinger commented May 16, 2024

@thomaseizinger I have recently stumbled into this issue writing a new application using rust-libp2p. I've written an mDNS application that was unnecessarily attempting to communicate with a local kubo client. I did resolve the matter with Identify and explicit disconnection of the kubo peer, but there is definitely an elegance to simply not discovering peers that I don't want to discover in the first place instead of having to connect to them and then close that connection later.

I agree with you. I am not against it, just saying it should be resolved on the spec level first.

@fei-ke
Copy link

fei-ke commented Jun 16, 2024

https://github.com/libp2p/specs/blob/master/discovery/mdns.md#definitions

If a private network is in use, then the service-name contains the base-16 encoding of the network's fingerprint as in _p2p-X._udp.local. This prevents public and private networks from discovering each other's peers.

The spec mentions that when using a private network, you can contains the network's fingerprint in the service name to prevent the private network and the public network from discovering each other.

In the Go implementation, I use a service name like this: _p2p-dced2aa3b9ea8def145b7f80271517b6._udp

@thomaseizinger
Copy link
Contributor

If a private network is in use, then the service-name contains the base-16 encoding of the network's fingerprint as in _p2p-X._udp.local. This prevents public and private networks from discovering each other's peers.

The spec mentions that when using a private network, you can contains the network's fingerprint in the service name to prevent the private network and the public network from discovering each other.

I no longer maintain rust-libp2p but this sounds like a feature that could be implemented although it is different from configurable service names so it would warrant to open a new issue for it.

@jxs
Copy link
Member

jxs commented Jul 2, 2024

libp2p/specs#620 was opened to continue this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants