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

Support hostname as ICE candidate #512

Open
kantlivelong opened this issue Jan 4, 2023 · 9 comments
Open

Support hostname as ICE candidate #512

kantlivelong opened this issue Jan 4, 2023 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@kantlivelong
Copy link

Summary

From what I've come to understand this library does not support using a hostname as an ICE candidate. Is this something that can be added?

Motivation

Obvious gains of using hostnames over IPs 😄

Describe alternatives you've considered

The only alternative is using a list of static IPs. Works but isn't great.

Additional context

AlexxIT/go2rtc#146

@kantlivelong kantlivelong changed the title Support hostname as ICE canidate Support hostname as ICE candidate Jan 4, 2023
@stv0g
Copy link
Member

stv0g commented Jan 9, 2023

I would be surprised if hostnames are valid as ICE candidates..
Is this part of the RFCs?

@kantlivelong
Copy link
Author

kantlivelong commented Jan 11, 2023

@stv0g Good question!

I'm in unfamiliar territory when it comes to WebRTC/ICE but after some digging I found RFC 8839 §5.1-3 which states the following:

<connection-address>: is taken from RFC 4566 [RFC4566]. It is the IP address of the candidate, allowing for IPv4 addresses, IPv6 addresses, and fully qualified domain names (FQDNs).

Whether that is the right RFC I am not sure.

@stv0g
Copy link
Member

stv0g commented Jan 12, 2023

Hi @kantlivelong,

RFC8839 should be the correct and most recent RFC in this regard.

However, the RFC tells a bit more about FQDNs:

An agent generating local candidates MUST NOT use FQDN addresses.
An agent processing remote candidates MUST ignore "candidate" lines that include candidates with FQDNs or IP address versions that are not supported or recognized.
The procedures for generation and handling of FQDN candidates, as well as, how agents indicate support for such procedures, need to be specified in an extension specification.

From what know there is currently no extension specification which defines the handling of FQDN candidates.

There is also a related issue in the pion/webrtc repo: pion/webrtc#2300

Here are some more related links:

@stv0g stv0g added enhancement New feature or request good first issue Good for newcomers labels Apr 12, 2023
@streamer45
Copy link

@stv0g This sounds like a decent improvement to cover some advanced deployments (e.g. multiple network partitions). Do you know what would be the rough expectation implementation wise?

@Sean-Der
Copy link
Member

@streamer45 trivial!

I would follow the same pattern as mDNS. When you get a hostname attempt to resolve it a thread, and then add it on success.

here is mDNS

@streamer45
Copy link

Thanks @Sean-Der, so that would be the client side implementation right? From the remote side of things I suppose there wouldn't be much to do other than allowing these hostnames to be advertised to clients?

@Sean-Der
Copy link
Member

Oh yes! This is just for the client side.

That might just work today with NAT1To1 in the SettingEngine. What happens if you do that today?

@streamer45
Copy link

Honestly I haven't tried but reading some of the related discussion it seemed that ice.NewCandidateHost would actually fail if given a hostname. Anyway that should be an easy fix.

@streamer45
Copy link

@Sean-Der As far as NAT1To1IPs it looks like it wouldn't pass this check:

extIP, isExtIPv4, err := validateIPString(ipPair[0])

As far as NewCandidateHost we also try to parse it as an IP unless it has the expected mDNS suffix:

ip := net.ParseIP(config.Address)

The potentially tricky detail here would be how to determine the network type given we don't have an address and I don't believe we should resolve it either given the resolution is meant to happen on the client side and could be totally different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Development

No branches or pull requests

4 participants