dhcp: improve BPF filters for vlans

pyroute2/dhcp/dhcp4socket.py

@@ -12,6 +12,7 @@
 from pyroute2.compat import ETHERTYPE_IP
 from pyroute2.dhcp.dhcp4msg import dhcp4msg
 from pyroute2.dhcp.messages import ReceivedDHCPMessage, SentDHCPMessage
+from pyroute2.ext.bpf import BPF
 from pyroute2.ext.rawsocket import AsyncRawSocket
 from pyroute2.protocols import ethmsg, ip4msg, udp4_pseudo_header, udpmsg
 
@@ -20,22 +21,40 @@
 
 UDP_HEADER_SIZE = 8
 IPV4_HEADER_SIZE = 20
+SKF_AD_OFF = -0x1000
+SKF_AD_VLAN_TAG_PRESENT = 48
 
 
 def listen_udp_port(port: int = 68) -> list[list[int]]:
-    # pre-scripted BPF code that matches UDP port
+    '''BPF filter that matches Ethernet + IPv4 + UDP on the given port.
+
+    Packets tagged on a vlan are also dropped, see
+    https://lore.kernel.org/netdev/[email protected]/T/
+    '''
     bpf_code = [
-        [40, 0, 0, 12],
-        [21, 0, 8, 2048],
-        [48, 0, 0, 23],
-        [21, 0, 6, 17],
-        [40, 0, 0, 20],
-        [69, 4, 0, 8191],
-        [177, 0, 0, 14],
-        [72, 0, 0, 16],
-        [21, 0, 1, port],
-        [6, 0, 0, 65535],
-        [6, 0, 0, 0],
+        # Load vlan presence indicator
+        [BPF.LD + BPF.B + BPF.ABS, 0, 0, SKF_AD_OFF + SKF_AD_VLAN_TAG_PRESENT],
+        # bail out immediately if there is one and we don't want it
+        [BPF.JMP + BPF.JEQ + BPF.K, 0, 10, 0],
+        # Load eth type at offset 12 and check it's IPv4
+        [BPF.LD + BPF.H + BPF.ABS, 0, 0, 12],
+        [BPF.JMP + BPF.JEQ + BPF.K, 0, 8, 0x0800],
+        # Load IP proto at offset 23 and check it's UDP
+        [BPF.LD + BPF.B + BPF.ABS, 0, 0, 23],
+        [BPF.JMP + BPF.JEQ + BPF.K, 0, 6, socket.IPPROTO_UDP],
+        # load frag offset at offset 20
+        [BPF.LD + BPF.H + BPF.ABS, 0, 0, 20],
+        # Check mask & drop fragmented packets
+        [BPF.JMP + BPF.JSET + BPF.K, 4, 0, 8191],
+        # load ip header length at offset 14
+        [BPF.LDX + BPF.B + BPF.MSH, 0, 0, 14],
+        # load udp dport from that offset + 16 and check it
+        [BPF.LD + BPF.H + BPF.IND, 0, 0, 16],
+        [BPF.JMP + BPF.JEQ + BPF.K, 0, 1, port],
+        # allow packet
+        [BPF.RET + BPF.K, 0, 0, 65535],
+        # drop packet
+        [BPF.RET + BPF.K, 0, 0, 0],
     ]
     return bpf_code
 

pyroute2/dhcp/server_detector.py

@@ -98,11 +98,11 @@ async def _get_offers(self, interface: str):
                     if next_msg.dhcp['xid'] != expected_xids[sock.ifname]:
                         LOG.debug(
                             '[%s] Got %s with xid mismatch, ignoring',
-                            sock.ifname,
+                            interface,
                             next_msg.message_type.name,
                         )
                         continue
-                    LOG.info('[%s] <- %s', sock.ifname, next_msg)
+                    LOG.info('[%s] <- %s', interface, next_msg)
                     await self._responses_queue.put((interface, next_msg))
                 except asyncio.CancelledError:
                     LOG.debug('[%s] stop discovery', interface)

pyroute2/ext/bpf.py

@@ -0,0 +1,89 @@
+
+from ctypes import (
+    Structure,
+    addressof,
+    c_int,
+    c_ubyte,
+    c_ushort,
+    c_void_p,
+    sizeof,
+    string_at,
+)
+from enum import IntEnum
+
+class sock_filter(Structure):
+    _fields_ = [
+        ('code', c_ushort),  # u16
+        ('jt', c_ubyte),  # u8
+        ('jf', c_ubyte),  # u8
+        ('k', c_int),  # can be signed or unsigned
+    ]
+
+
+class sock_fprog(Structure):
+    _fields_ = [('len', c_ushort), ('filter', c_void_p)]
+
+
+
+def compile(code: list[list[int]]):
+    ProgramType = sock_filter * len(code)
+    program = ProgramType(*[sock_filter(*line) for line in code])
+    sfp = sock_fprog(len(code), addressof(program[0]))
+    return string_at(addressof(sfp), sizeof(sfp)), program
+
+
+class BPF(IntEnum):
+    '''BPF constants.
+
+    See:
+    - https://www.kernel.org/doc/Documentation/networking/filter.txt
+    - https://github.com/iovisor/bpf-docs/blob/master/eBPF.md
+    '''
+    # Operations
+    LD = 0x00  # Load
+    LDX = 0x01  # Load Index
+    ST = 0x02  # Store
+    STX = 0x03  # Store Index
+    ALU = 0x04  # Arithmetic Logic Unit
+    JMP = 0x05  # Jump
+    RET = 0x06  # Return
+    MISC = 0x07  # Miscellaneous
+
+    # Sizes
+    W = 0x00  # Word (4 bytes)
+    H = 0x08  # Half-word (2 bytes)
+    B = 0x10  # Byte (1 byte)
+
+    # Offsets
+    IMM = 0x00  # Immediate
+    ABS = 0x20  # Absolute
+    IND = 0x40  # Indirect
+    MEM = 0x60  # Memory
+    LEN = 0x80  # Packet Length
+    MSH = 0xA0  # Masked
+
+    # gotos
+    JEQ = 0x10  # Jump if Equal
+    JGT = 0x20  # Jump if Greater
+    JGE = 0x30  # Jump if Greater or Equal
+    JSET = 0x40  # Jump if Bit is Set
+
+    # Sources
+    K = 0x00  # Constant
+    X = 0x08  # Register
+
+    # Operators
+    ADD = 0x00
+    SUB = 0x10
+    MUL = 0x20
+    DIV = 0x30
+    OR = 0x40
+    AND = 0x50
+    LSH = 0x60
+    RSH = 0x70
+    NEG = 0x80
+    MOD = 0x90
+    XOR = 0xA0
+    MOV = 0xB0
+    ARSH = 0xC0
+    END = 0xD0

pyroute2/ext/rawsocket.py

@@ -1,23 +1,14 @@
 import asyncio
 import logging
-from ctypes import (
-    Structure,
-    addressof,
-    c_ubyte,
-    c_uint,
-    c_ushort,
-    c_void_p,
-    sizeof,
-    string_at,
-)
 from socket import AF_PACKET, SOCK_RAW, SOL_SOCKET, errno, error, htons, socket
 from typing import Optional
 
+from pyroute2.ext import bpf
 from pyroute2.iproute.linux import AsyncIPRoute
 from pyroute2.netlink.rtnl import RTMGRP_LINK
 
 LOG = logging.getLogger(__name__)
-
+ETH_P_IP = 0x0800
 ETH_P_ALL = 3
 SO_ATTACH_FILTER = 26
 SO_DETACH_FILTER = 27
@@ -26,26 +17,6 @@
 total_filter = [[0x06, 0, 0, 0]]
 
 
-class sock_filter(Structure):
-    _fields_ = [
-        ('code', c_ushort),  # u16
-        ('jt', c_ubyte),  # u8
-        ('jf', c_ubyte),  # u8
-        ('k', c_uint),
-    ]  # u32
-
-
-class sock_fprog(Structure):
-    _fields_ = [('len', c_ushort), ('filter', c_void_p)]
-
-
-def compile_bpf(code: list[list[int]]):
-    ProgramType = sock_filter * len(code)
-    program = ProgramType(*[sock_filter(*line) for line in code])
-    sfp = sock_fprog(len(code), addressof(program[0]))
-    return string_at(addressof(sfp), sizeof(sfp)), program
-
-
 class AsyncRawSocket(socket):
     '''
     This raw socket binds to an interface and optionally installs a BPF
@@ -86,7 +57,7 @@ async def __aenter__(self):
         socket.bind(self, (self.ifname, ETH_P_ALL))
         if self.bpf:
             self.clear_buffer()
-            fstring, self.fprog = compile_bpf(self.bpf)
+            fstring, self.fprog = bpf.compile(self.bpf)
             socket.setsockopt(self, SOL_SOCKET, SO_ATTACH_FILTER, fstring)
         else:
             # FIXME: should be async
@@ -129,7 +100,7 @@ def clear_buffer(self, remove_total_filter: bool = False):
         # pcap-linux.c. libpcap sets a total filter which does not match any
         # packet.  It then clears what is already in the socket
         # before setting the desired filter
-        total_fstring, prog = compile_bpf(total_filter)
+        total_fstring, prog = bpf.compile(total_filter)
         socket.setsockopt(self, SOL_SOCKET, SO_ATTACH_FILTER, total_fstring)
         while True:
             try:

tests/test_linux/test_dhcp/test_server_detector.py

@@ -156,6 +156,7 @@ async def test_detect_with_vlan(
     async_ipr: AsyncIPRoute,
     caplog: pytest.LogCaptureFixture,
     run_dhcp_server_outside_vlan: bool,
+    async_context,
 ):
     '''Get an offer from dnsmasq over a vlan,
     and maybe another offer from udhcpd outside of it.
@@ -259,10 +260,10 @@ async def test_detect_with_vlan(
         # we stopped udhcpd so only got an offer from dnsmasq
         assert len(offers) == 1
 
-    #################################################################
-    # Here is the issue:
-    # the socket listening on the non-vlan interface also receives
-    # the offer sent over the vlan. since we have a different xid per
-    # interface, it is discarded, but should not happen anyway.
+    # The bpf filter drops packets that are intended for vlans; failing that,
+    # we would receive a copy of all packets meant for "upper" vlans when
+    # on a non-vlan interface.
+    # since we have a different xid per  interface, they're discarded,
+    # but should not happen anyway.
+    # So if this assert fails it means the BPF filter does not work
     assert b'Got OFFER with xid mismatch, ignoring' not in stderr
-    #################################################################