-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(mdns): filter addresses to reduce packet size #3434
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ import ( | |
|
|
||
| logging "github.com/libp2p/go-libp2p/gologshim" | ||
| ma "github.com/multiformats/go-multiaddr" | ||
| manet "github.com/multiformats/go-multiaddr/net" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -109,6 +108,68 @@ func (s *mdnsService) getIPs(addrs []ma.Multiaddr) ([]string, error) { | |
| return ips, nil | ||
| } | ||
|
|
||
| // containsUnsuitableProtocol returns true if the multiaddr includes protocols | ||
| // that are not suitable for mDNS advertisement: | ||
| // - Circuit relay (requires intermediary, not direct LAN connectivity) | ||
| // - Browser transports: WebTransport, WebRTC, WebSocket (browsers don't use mDNS) | ||
| func containsUnsuitableProtocol(addr ma.Multiaddr) bool { | ||
| found := false | ||
| ma.ForEach(addr, func(c ma.Component) bool { | ||
| switch c.Protocol().Code { | ||
| case ma.P_CIRCUIT, | ||
| ma.P_WEBTRANSPORT, | ||
| ma.P_WEBRTC, | ||
| ma.P_WEBRTC_DIRECT, | ||
| ma.P_P2P_WEBRTC_DIRECT, | ||
| ma.P_WS, | ||
| ma.P_WSS: | ||
| found = true | ||
| return false | ||
| } | ||
| return true | ||
| }) | ||
| return found | ||
| } | ||
|
|
||
| // isSuitableForMDNS returns true for multiaddrs that should be advertised | ||
| // via mDNS. | ||
| // | ||
| // For an address to be suitable: | ||
| // 1. It must start with /ip4, /ip6, or a .local DNS name. The .local TLD is | ||
| // reserved for mDNS (RFC 6762) and resolved via multicast, not unicast DNS. | ||
| // Non-.local DNS names are filtered out as they require external DNS. | ||
| // 2. It must not use circuit relay or browser-only transports (WebTransport, | ||
| // WebRTC, WebSocket) because these are not useful for direct LAN discovery. | ||
| // | ||
| // Filtering reduces mDNS packet size, helping stay within the 1500-byte MTU | ||
| // limit per RFC 6762. See: https://github.com/libp2p/go-libp2p/issues/3415 | ||
| func isSuitableForMDNS(addr ma.Multiaddr) bool { | ||
| if addr == nil { | ||
| return false | ||
| } | ||
|
|
||
| first, _ := ma.SplitFirst(addr) | ||
| if first == nil { | ||
| return false | ||
| } | ||
|
|
||
| // Check the addressing scheme | ||
| switch first.Protocol().Code { | ||
| case ma.P_IP4, ma.P_IP6: | ||
| // Direct IP addresses are always suitable for LAN discovery | ||
| case ma.P_DNS, ma.P_DNS4, ma.P_DNS6, ma.P_DNSADDR: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does DNSADDR make sense here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added it mostly for completeness, but yes, technically someone could have TXT record on |
||
| // DNS names are only suitable if they're in the .local TLD, | ||
| // which is resolved via mDNS (RFC 6762), not unicast DNS. | ||
| if !strings.HasSuffix(strings.ToLower(first.Value()), ".local") { | ||
| return false | ||
| } | ||
| default: | ||
| return false | ||
| } | ||
|
|
||
| return !containsUnsuitableProtocol(addr) | ||
| } | ||
|
|
||
| func (s *mdnsService) startServer() error { | ||
| interfaceAddrs, err := s.host.Network().InterfaceListenAddresses() | ||
| if err != nil { | ||
|
|
@@ -121,9 +182,10 @@ func (s *mdnsService) startServer() error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| // Build TXT records for addresses suitable for mDNS advertisement. | ||
| var txts []string | ||
| for _, addr := range addrs { | ||
| if manet.IsThinWaist(addr) { // don't announce circuit addresses | ||
| if isSuitableForMDNS(addr) { | ||
| txts = append(txts, dnsaddrPrefix+addr.String()) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A slight correction in this comment. The RFC states the packet "MUST NOT exceed 9000 bytes". Below 1500 bytes is recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 1aabf13