Merge pull request #5500 from c-po/ppp-interface-ipv6-ll

ppp: T9060: do not derive a link-local address from a synthetic MAC

4 files changed, 104 insertions, 8 deletions

diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 73f7544dd..270e3db08 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -94,6 +94,11 @@ class Interface(Control):
         'eternal': '',
     }
 
+    # T9060: Interfaces negotiating their IPv6 interface identifier with the
+    # remote end - PPP links via IPV6CP (RFC 5072) - must not receive an
+    # additional EUI-64 derived link-local address unknown to the peer.
+    _ipv6_default_link_local = True
+
     _command_get = {
         'admin_state': {
             'shellcmd': 'ip -json link show dev {ifname}',
@@ -983,7 +988,8 @@ class Interface(Control):
         with the given prefix to the interface.
         """
         # T2863: only add a link-local IPv6 address if the interface returns
-        # a MAC address. This is not the case on e.g. WireGuard interfaces.
+        # a MAC address. Interface types without one either provide a
+        # synthetic MAC or opt out via _ipv6_default_link_local.
         mac = self.get_mac()
         if mac:
             eui64 = mac2eui64(mac, prefix)
@@ -2011,7 +2017,8 @@ class Interface(Control):
             self.del_ipv6_eui64_address(addr)
 
         # Manage IPv6 link-local addresses
-        if dict_search('ipv6.address.no_default_link_local', config) != None:
+        if (not self._ipv6_default_link_local or
+                dict_search('ipv6.address.no_default_link_local', config) != None):
             self.del_ipv6_eui64_address(link_local_prefix)
         else:
             self.add_ipv6_eui64_address(link_local_prefix)
diff --git a/python/vyos/ifconfig/pppoe.py b/python/vyos/ifconfig/pppoe.py
index 327c41719..eb217e12d 100644
--- a/python/vyos/ifconfig/pppoe.py
+++ b/python/vyos/ifconfig/pppoe.py
@@ -14,9 +14,11 @@
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 from vyos.ifconfig.interface import Interface
+from vyos.template import is_ipv6
 from vyos.utils.assertion import assert_range
 from vyos.utils.dict import dict_search
 from vyos.utils.network import get_interface_config
+from vyos.utils.network import mac2eui64
 
 @Interface.register
 class PPPoEIf(Interface):
@@ -28,6 +30,10 @@ class PPPoEIf(Interface):
         },
     }
 
+    # T9060: the IPv6 interface identifier of a PPP link is negotiated with
+    # the peer via IPV6CP (RFC 5072) - see Interface._ipv6_default_link_local
+    _ipv6_default_link_local = False
+
     _sysfs_get = {
         **Interface._sysfs_get,**{
             'accept_ra_defrtr': {
@@ -80,6 +86,17 @@ class PPPoEIf(Interface):
         # we cannot create this interface as it is managed outside
         pass
 
+    def del_ipv6_eui64_address(self, prefix):
+        """
+        del_addr() is a NOOP as the addresses are managed by pppd. The EUI-64
+        link-local address was added by VyOS itself, so use the generic
+        implementation to clean it off an already established session.
+        """
+        if is_ipv6(prefix):
+            eui64 = mac2eui64(self.get_mac(), prefix)
+            prefixlen = prefix.split('/')[1]
+            super().del_addr(f'{eui64}/{prefixlen}')
+
     def get_mac(self):
         """ Get a synthetic MAC address. """
         return self.get_mac_synthetic()
diff --git a/python/vyos/ifconfig/sstpc.py b/python/vyos/ifconfig/sstpc.py
index a84a44265..71c965784 100644
--- a/python/vyos/ifconfig/sstpc.py
+++ b/python/vyos/ifconfig/sstpc.py
@@ -26,6 +26,10 @@ class SSTPCIf(Interface):
         },
     }
 
+    # T9060: the IPv6 interface identifier of a PPP link is negotiated with
+    # the peer via IPV6CP (RFC 5072) - see Interface._ipv6_default_link_local
+    _ipv6_default_link_local = False
+
     def _create(self):
         # we cannot create this interface as it is managed outside
         pass
diff --git a/smoketest/scripts/cli/test_interfaces_pppoe.py b/smoketest/scripts/cli/test_interfaces_pppoe.py
index 2c240d20e..dfdd3c21f 100755
--- a/smoketest/scripts/cli/test_interfaces_pppoe.py
+++ b/smoketest/scripts/cli/test_interfaces_pppoe.py
@@ -24,8 +24,12 @@ from ipaddress import IPv6Network
 from base_vyostest_shim import VyOSUnitTestSHIM
 
 from vyos.configsession import ConfigSessionError
+from vyos.ifconfig import PPPoEIf
+from vyos.ifconfig.interface import link_local_prefix
 from vyos.utils.dict import dict_search_recursive
+from vyos.utils.misc import wait_for
 from vyos.utils.network import get_interface_address
+from vyos.utils.network import mac2eui64
 from vyos.xml_ref import default_value
 
 config_file: str = '/etc/ppp/peers/{}'
@@ -73,6 +77,19 @@ def wait_for_interface(interface: str, timeout=connect_timeout) -> bool:
             return False
     return True
 
+def get_interface_addresses(interface, family) -> list:
+    """ Return the list of addresses of a given family assigned to interface """
+    tmp = get_interface_address(interface)
+    if not tmp or 'addr_info' not in tmp:
+        return []
+    return [addr['local'] for addr in tmp['addr_info']
+            if 'family' in addr and addr['family'] == family]
+
+def has_global_ipv6_address(interface) -> bool:
+    """ Check if the interface got a non link-local IPv6 address assigned """
+    return any(not IPv6Address(addr).is_link_local
+               for addr in get_interface_addresses(interface, 'inet6'))
+
 # add a classmethod to setup a temporaray PPPoE server for "proper" validation
 class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
     @classmethod
@@ -137,14 +154,20 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
 
     def _verify_interface_address(self, interface):
         # Verify that the assigned IPv4/IPv6 addresses from the BRAS (PPPoE
-        # server) are from the assigned pools
-        for address in get_interface_address(interface):
-            if 'family' in address and address['family'] == 'inet':
+        # server) are from the assigned pools - 'local' is our own address
+        tmp = get_interface_address(interface)
+        self.assertIn('addr_info', tmp)
+
+        for addr_info in tmp['addr_info']:
+            if 'family' not in addr_info:
+                continue
+
+            if addr_info['family'] == 'inet':
                 # The PPPoE assigned IPv4 address must be from our pool
-                self.assertIn(IPv4Address(address['address']), IPv4Network(ipv4_pool))
-            elif 'family' in address and address['family'] == 'inet6':
+                self.assertIn(IPv4Address(addr_info['local']), IPv4Network(ipv4_pool))
+            elif addr_info['family'] == 'inet6':
                 # The PPPoE assigned IPv6 address must be from our pool
-                ipv6 = IPv6Address(address['address'])
+                ipv6 = IPv6Address(addr_info['local'])
                 if not ipv6.is_link_local:
                     self.assertIn(ipv6, IPv6Network(ipv6_pool))
 
@@ -327,6 +350,51 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
 
             self.cli_delete(['interfaces', 'dummy', delegate_if])
 
+    def test_pppoe_ipv6_link_local(self):
+        # T9060: the IPv6 interface identifier of a PPP link is negotiated
+        # with the peer via IPV6CP (RFC 5072). A second, EUI-64 derived
+        # link-local address is unknown to the BRAS, and RFC 6724 source
+        # address selection may pick it for e.g. DHCPv6-PD.
+        for interface in self._interfaces:
+            (user, passwd) = self.u_p_dict[interface]
+
+            self.cli_set(base_path + [interface, 'authentication', 'username', user])
+            self.cli_set(base_path + [interface, 'authentication', 'password', passwd])
+            self.cli_set(base_path + [interface, 'no-peer-dns'])
+            self.cli_set(base_path + [interface, 'source-interface', self._source_interface])
+            # Only with the "ipv6" node present pppd is told to negotiate
+            # IPV6CP - without it the peer config contains "noipv6"
+            self.cli_set(base_path + [interface, 'ipv6', 'address', 'autoconf'])
+
+        # commit changes
+        self.cli_commit()
+
+        for interface in self._interfaces:
+            self.assertTrue(wait_for_interface(interface),
+                            msg=f'Interface {interface} not found after {connect_timeout} seconds!')
+
+            # The link-local address is assigned before the router
+            # solicitation is sent out - once a global IPv6 address is present
+            # we know we are not testing too early
+            self.assertTrue(wait_for(has_global_ipv6_address, interface,
+                                     interval=0.250, timeout=connect_timeout),
+                            msg=f'Interface {interface} got no global IPv6 address!')
+
+            link_local = [addr for addr in get_interface_addresses(interface, 'inet6')
+                          if IPv6Address(addr).is_link_local]
+
+            # There must be exactly one link-local address - the one negotiated
+            # via IPV6CP
+            self.assertEqual(len(link_local), 1)
+
+            # ... and it must not be the one derived from the synthetic MAC
+            # address of the interface
+            eui64 = mac2eui64(PPPoEIf(interface).get_mac(), link_local_prefix)
+            self.assertNotIn(eui64, link_local)
+
+            # Validate and verify assigned IP addresses
+            self._verify_interface_address(interface)
+
     def test_pppoe_options(self):
         # Verify access-concentrator and service-name CLI options