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

Qualify IPv6 link-local addresses with scope_id #315

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 20, 2020

When a service is advertised on an IPv6 address where
the scope is link local, i.e. fe80::/64 (see RFC 4007)
the resolved IPv6 address must be extended with the
scope_id that identifies through the "%" symbol the
local interface to be used when routing to that address.
A new API is provided to return qualified addresses
to avoid breaking compatibility on the existing
parsed_addresses().

This patch:

  • Extracts the scope_id from an IPv6 link-local address
    and return it in the retrieved information in ServiceInfo.
  • Adjusts the browser test to be able to show IPv6 link-local
    addresses by using the new method parsed_scoped_addresses().
  • Uses scope_id in unicast replies to IPv6 link-local addresses,
    and adds some unit tests to verify the new interface provided.

Co-authored-by: Lokesh Prajapati lokesh.prajapati@ncipher.com

Closes #314

@coveralls
Copy link

coveralls commented Oct 20, 2020

Coverage Status

Coverage decreased (-0.9%) to 92.446% when pulling 620564f on nCipherSecurity:master into 4da1612 on jstasiak:master.

@vazhnov
Copy link

vazhnov commented Oct 21, 2020

I see an error in automatic Travis build with Python 3.5 only:

E     File "/home/travis/build/jstasiak/python-zeroconf/zeroconf/__init__.py", line 1899
E       return [f"{addr}%{self.interface_index}" for addr in ll_addrs] + other_addrs
E                                              ^
E   SyntaxError: invalid syntax

@jstasiak
Copy link
Collaborator

Thank you! I'll need to think about this, I'm not a fan of a separate API for this (parsed_scoped_addresses(), scoped_id being separate from address variables etc.) but we need to support this in some way.

@ghost
Copy link
Author

ghost commented Oct 22, 2020

Is there a way to run locally coveralls to check the coverage reports? I would like to understand if there is anyway to improve coverage in the proposed patch as well.

@jstasiak
Copy link
Collaborator

You can use our test_coverage make target to use the standard coverage module and print the output.

@ghost ghost force-pushed the master branch 2 times, most recently from 1ce5dab to acdffcc Compare October 22, 2020 15:20
When a service is advertised on an IPv6 address where
the scope is link local, i.e. fe80::/64 (see RFC 4007)
the resolved IPv6 address must be extended with the
scope_id that identifies through the "%" symbol the
local interface to be used when routing to that address.
A new API is provided to return qualified addresses
to avoid breaking compatibility on the existing
arsed_addresses().

This patch:
 * Extracts the scope_id from an IPv6 link-local address
   and return it in the retrieved information in ServiceInfo.
 * Adjusts the browser test to be able to show IPv6 link-local
   addresses by using the new method parsed_scoped_addresses().
 * Uses scope_id in unicast replies to IPv6 link-local addresses,
   and adds some unit tests to verify the new interface provided.

Co-authored-by: Lokesh Prajapati <lokesh.prajapati@ncipher.com>
@yoyko
Copy link

yoyko commented Mar 30, 2021

I ran into a problem with this implementation: when a device reports both ipv4 and ipv6 addresses over ipv4 (well, usually it reports both addresses over both ipv4 and ipv6 ;) and python-zeroconf listens only on ipv4 (which is the default), then scope_id won't be added:

addr: 10.147.0.20, port: 5353, v6: []
+ TestService._test._tcp.local.: ['10.147.0.20', 'fe80::2086:3656:3eb9:4d45']

Additionally, if the device reports both addresses, but only over ipv4 (yeah, I know, this is a weird behavior and could be considered a bug...), setting python-zeroconf to listen on all actually makes it worse, becase with dualstack (IPV6_V6ONLY==false), ipv4-in-ipv6 addresses from socket.recvfrom have scope-id set to 0, which then gets set on the ipv6 address:

addr: ::ffff:10.147.0.20, port: 5353, v6: [0, 0]
+ Test._test._tcp.local.: ['fe80::2086:3656:3eb9:4d45%0', '10.147.0.20']

Test program:

from zeroconf import Zeroconf, ServiceBrowser, IPVersion
import sys
import time

class L:
    def add_service(self, z, t, n):
        info = z.get_service_info(t, n)
        print(f"+ {n}: {info.parsed_scoped_addresses(IPVersion.All)}")

ip_version = None
if len(sys.argv) > 2 and sys.argv[2] == "all":
    ip_version = IPVersion.All
z = Zeroconf(ip_version=ip_version)
l = L()
b = ServiceBrowser(z, sys.argv[1], l)

time.sleep(2)

The first message is from:

diff --git a/zeroconf/__init__.py b/zeroconf/__init__.py
index c633d5f..b074533 100644
--- a/zeroconf/__init__.py
+++ b/zeroconf/__init__.py
@@ -1410,6 +1410,7 @@ class Listener(QuietLogger):
             self.log_exception_warning('Error reading from socket %d', socket_.fileno())
             return
 
+        print(f"addr: {addr}, port: {port}, v6: {_v6}")
         if _v6:
             log.debug('IPv6 scope_id %d associated to the receiving interface', _v6[1])
 

The first test was run against

avahi-publish-service  TestService _test._tcp 1234

@bdraco
Copy link
Member

bdraco commented May 3, 2021

Closing this since the OP is now a ghost.

@bdraco bdraco closed this May 3, 2021
@ibygrave
Copy link

Could this be reopened? The OP was working with me, and I have taken over this issue. I've opened a new PR for it.

@bdraco bdraco reopened this May 22, 2021
@bdraco
Copy link
Member

bdraco commented May 22, 2021

New PR is #343

@bdraco bdraco closed this May 22, 2021
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

Successfully merging this pull request may close these issues.

Provide scope_id when resolving IPv6 Link Local addresses
6 participants