Merge pull request #94 from oremanj/release-v1.1.0

Release v1.1.0

.github/workflows/ci.yml

@@ -15,13 +15,14 @@ jobs:
       fail-fast: false
       matrix:
         python:
-          - '3.6'
           - '3.7'
           - '3.8'
           - '3.9'
           - '3.10'
+          - '3.11'
           - 'pypy-3.7'
           - 'pypy-3.8'
+          - 'pypy-3.9'
         check_lint: ['0']
         extra_name: ['']
         include:

CHANGES.txt

@@ -1,13 +1,18 @@
-v1.0.0, unreleased
+v1.1.0, 1 Mar 2023
+    Add Packet accessors for {indev, outdev, physindev, physoutdev} interface indices
+
+v1.0.0, 14 Jan 2022
     Propagate exceptions raised by the user's packet callback
-    Warn about exceptions raised by the packet callback during queue unbinding
+    Avoid calls to the packet callback during queue unbinding
     Raise an error if a packet verdict is set after its parent queue is closed
     set_payload() now affects the result of later get_payload()
     Handle signals received when run() is blocked in recv()
     Accept packets in COPY_META mode, only failing on an attempt to access the payload
     Add a parameter NetfilterQueue(sockfd=N) that uses an already-opened Netlink socket
+    Add type hints
+    Remove the Packet.payload attribute; it was never safe (treated as a char* but not NUL-terminated) nor documented, but was exposed in the API (perhaps inadvertently).
 
-v0.9.0, 12 Jan 2021
+v0.9.0, 12 Jan 2022
     Improve usability when Packet objects are retained past the callback
     Add Packet.retain() to save the packet contents in such cases
     Eliminate warnings during build on py3

MANIFEST.in

@@ -1,6 +1,3 @@
-include *.txt
-include *.rst
-include *.c
-include *.pyx
-include *.pxd
-recursive-include tests/ *.py
+include LICENSE.txt README.rst CHANGES.txt
+recursive-include netfilterqueue *.py *.pyx *.pxd *.c *.pyi py.typed
+recursive-include tests *.py

README.rst

@@ -1,3 +1,11 @@
+.. image:: https://img.shields.io/pypi/v/netfilterqueue.svg
+   :target: https://pypi.org/project/netfilterqueue
+   :alt: Latest PyPI version
+
+.. image:: https://github.com/oremanj/python-netfilterqueue/actions/workflows/ci.yml/badge.svg?branch=master
+   :target: https://github.com/oremanj/python-netfilterqueue/actions?query=branch%3Amaster
+   :alt: Automated test status
+
 ==============
 NetfilterQueue
 ==============
@@ -9,6 +17,9 @@ or given a mark.
 libnetfilter_queue (the netfilter library, not this module) is part of the
 `Netfilter project <http://netfilter.org/projects/libnetfilter_queue/>`_.
 
+The current version of NetfilterQueue requires Python 3.6 or later.
+The last version with support for Python 2.7 was 0.9.0.
+
 Example
 =======
 
@@ -68,7 +79,7 @@ Before installing, ensure you have:
 
 On Debian or Ubuntu, install these files with::
 
-    apt-get install build-essential python-dev libnetfilter-queue-dev
+    apt-get install build-essential python3-dev libnetfilter-queue-dev
 
 From PyPI
 ---------
@@ -206,6 +217,18 @@ Objects of this type are passed to your callback.
     into our queue. Values 0 through 4 correspond to PREROUTING, INPUT,
     FORWARD, OUTPUT, and POSTROUTING respectively.
 
+``Packet.indev``, ``Packet.outdev``, ``Packet.physindev``, ``Packet.physoutdev``
+    The interface indices on which the packet arrived (``indev``) or is slated
+    to depart (``outdev``). These are integers, which can be converted to
+    names like "eth0" by using ``socket.if_indextoname()``. Zero means
+    no interface is applicable, either because the packet was locally generated
+    or locally received, or because the interface information wasn't available
+    when the packet was queued (for example, ``PREROUTING`` rules don't yet
+    know the ``outdev``). If the ``indev`` or ``outdev`` refers to a bridge
+    device, then the corresponding ``physindev`` or ``physoutdev`` will name
+    the bridge member on which the actual traffic occurred; otherwise
+    ``physindev`` and ``physoutdev`` will be zero.
+
 ``Packet.retain()``
     Allocate a copy of the packet payload for use after the callback
     has returned. ``get_payload()`` will raise an exception at that

ci.sh

@@ -11,34 +11,42 @@ python setup.py sdist --formats=zip
 
 # ... but not to install it
 pip uninstall -y cython
+python setup.py build_ext
 pip install dist/*.zip
 
 pip install -Ur test-requirements.txt
 
 if [ "$CHECK_LINT" = "1" ]; then
     error=0
-    if ! black --check setup.py tests; then
+    black_files="setup.py tests netfilterqueue"
+    if ! black --check $black_files; then
+        error=$?
+        black --diff $black_files
+    fi
+    mypy --strict -p netfilterqueue || error=$?
+    ( mkdir empty; cd empty; python -m mypy.stubtest netfilterqueue ) || error=$?
+
+    if [ $error -ne 0 ]; then
         cat <<EOF
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
-Formatting problems were found (listed above). To fix them, run
+Problems were found by static analysis (listed above).
+To fix formatting and see remaining errors, run:
 
    pip install -r test-requirements.txt
-   black setup.py tests
+   black $black_files
+   mypy --strict -p netfilterqueue
+   ( mkdir empty; cd empty; python -m mypy.stubtest netfilterqueue )
 
 in your local checkout.
 
-EOF
-        error=1
-    fi
-    if [ "$error" = "1" ]; then
-        cat <<EOF
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 EOF
+        exit 1
     fi
-    exit $error
+    exit 0
 fi
 
 cd tests

netfilterqueue/__init__.py

@@ -0,0 +1,12 @@
+from ._impl import (
+    COPY_NONE as COPY_NONE,
+    COPY_META as COPY_META,
+    COPY_PACKET as COPY_PACKET,
+    Packet as Packet,
+    NetfilterQueue as NetfilterQueue,
+    PROTOCOLS as PROTOCOLS,
+)
+from ._version import (
+    VERSION as VERSION,
+    __version__ as __version__,
+)

netfilterqueue/_impl.pxd

@@ -153,7 +153,11 @@ cdef extern from "libnetfilter_queue/libnetfilter_queue.h":
     int nfq_get_payload(nfq_data *nfad, unsigned char **data)
     int nfq_get_timestamp(nfq_data *nfad, timeval *tv)
     nfqnl_msg_packet_hw *nfq_get_packet_hw(nfq_data *nfad)
-    int nfq_get_nfmark (nfq_data *nfad)
+    int nfq_get_nfmark(nfq_data *nfad)
+    u_int32_t nfq_get_indev(nfq_data *nfad)
+    u_int32_t nfq_get_outdev(nfq_data *nfad)
+    u_int32_t nfq_get_physindev(nfq_data *nfad)
+    u_int32_t nfq_get_physoutdev(nfq_data *nfad)
     nfnl_handle *nfq_nfnlh(nfq_handle *h)
 
 # Dummy defines from linux/socket.h:
@@ -184,8 +188,7 @@ cdef class NetfilterQueue:
 
 cdef class Packet:
     cdef NetfilterQueue _queue
-    cdef bint _verdict_is_set # True if verdict has been issued,
-        # false otherwise
+    cdef bint _verdict_is_set # True if verdict has been issued, false otherwise
     cdef bint _mark_is_set # True if a mark has been given, false otherwise
     cdef bint _hwaddr_is_set
     cdef bint _timestamp_is_set
@@ -201,16 +204,13 @@ cdef class Packet:
 
     # Packet details:
     cdef Py_ssize_t payload_len
-    cdef readonly unsigned char *payload
+    cdef unsigned char *payload
     cdef timeval timestamp
     cdef u_int8_t hw_addr[8]
-
-    # TODO: implement these
-    #cdef readonly u_int32_t nfmark
-    #cdef readonly u_int32_t indev
-    #cdef readonly u_int32_t physindev
-    #cdef readonly u_int32_t outdev
-    #cdef readonly u_int32_t physoutdev
+    cdef readonly u_int32_t indev
+    cdef readonly u_int32_t physindev
+    cdef readonly u_int32_t outdev
+    cdef readonly u_int32_t physoutdev
 
     cdef set_nfq_data(self, NetfilterQueue queue, nfq_data *nfa)
     cdef drop_refs(self)

netfilterqueue/_impl.pyi

@@ -0,0 +1,47 @@
+import socket
+from enum import IntEnum
+from typing import Callable, Dict, Optional, Tuple
+
+COPY_NONE: int
+COPY_META: int
+COPY_PACKET: int
+
+class Packet:
+    hook: int
+    hw_protocol: int
+    id: int
+    mark: int
+    # These are ifindexes, pass to socket.if_indextoname() to get names:
+    indev: int
+    outdev: int
+    physindev: int
+    physoutdev: int
+    def get_hw(self) -> Optional[bytes]: ...
+    def get_payload(self) -> bytes: ...
+    def get_payload_len(self) -> int: ...
+    def get_timestamp(self) -> float: ...
+    def get_mark(self) -> int: ...
+    def set_payload(self, payload: bytes) -> None: ...
+    def set_mark(self, mark: int) -> None: ...
+    def retain(self) -> None: ...
+    def accept(self) -> None: ...
+    def drop(self) -> None: ...
+    def repeat(self) -> None: ...
+
+class NetfilterQueue:
+    def __new__(self, *, af: int = ..., sockfd: int = ...) -> NetfilterQueue: ...
+    def bind(
+        self,
+        queue_num: int,
+        user_callback: Callable[[Packet], None],
+        max_len: int = ...,
+        mode: int = COPY_PACKET,
+        range: int = ...,
+        sock_len: int = ...,
+    ) -> None: ...
+    def unbind(self) -> None: ...
+    def get_fd(self) -> int: ...
+    def run(self, block: bool = ...) -> None: ...
+    def run_socket(self, s: socket.socket) -> None: ...
+
+PROTOCOLS: Dict[int, str]

netfilterqueue/_impl.pyx

@@ -5,7 +5,6 @@ function.
 Copyright: (c) 2011, Kerkhoff Technologies Inc.
 License: MIT; see LICENSE.txt
 """
-VERSION = (0, 9, 0)
 
 # Constants for module users
 COPY_NONE = 0
@@ -26,6 +25,10 @@ DEF SockRcvSize = DEFAULT_MAX_QUEUELEN * SockCopySize // 2
 
 from cpython.exc cimport PyErr_CheckSignals
 
+cdef extern from "Python.h":
+    ctypedef struct PyTypeObject:
+        const char* tp_name
+
 # A negative return value from this callback will stop processing and
 # make nfq_handle_packet return -1, so we use that as the error flag.
 cdef int global_callback(nfq_q_handle *qh, nfgenmsg *nfmsg,
@@ -54,7 +57,15 @@ cdef class Packet:
         self._given_payload = None
 
     def __str__(self):
-        cdef iphdr *hdr = <iphdr*>self.payload
+        cdef unsigned char *payload = NULL
+        if self._owned_payload:
+            payload = self._owned_payload
+        elif self.payload != NULL:
+            payload = self.payload
+        else:
+            return "%d byte packet, contents unretained" % (self.payload_len,)
+
+        cdef iphdr *hdr = <iphdr*>payload
         protocol = PROTOCOLS.get(hdr.protocol, "Unknown protocol")
         return "%s packet, %s bytes" % (protocol, self.payload_len)
 
@@ -88,6 +99,10 @@ cdef class Packet:
 
         nfq_get_timestamp(nfa, &self.timestamp)
         self.mark = nfq_get_nfmark(nfa)
+        self.indev = nfq_get_indev(nfa)
+        self.outdev = nfq_get_outdev(nfa)
+        self.physindev = nfq_get_physindev(nfa)
+        self.physoutdev = nfq_get_physoutdev(nfa)
 
     cdef drop_refs(self):
         """
@@ -343,6 +358,17 @@ cdef class NetfilterQueue:
             else:
                 nfq_handle_packet(self.h, buf, len(buf))
 
+cdef void _fix_names():
+    # Avoid ._impl showing up in reprs. This doesn't work on PyPy; there we would
+    # need to modify the name before PyType_Ready(), but I can't find any way to
+    # write Cython code that would execute at that time.
+    cdef PyTypeObject* tp = <PyTypeObject*>Packet
+    tp.tp_name = "netfilterqueue.Packet"
+    tp = <PyTypeObject*>NetfilterQueue
+    tp.tp_name = "netfilterqueue.NetfilterQueue"
+
+_fix_names()
+
 PROTOCOLS = {
     0: "HOPOPT",
     1: "ICMP",

netfilterqueue/_version.py

@@ -0,0 +1,4 @@
+# This file is imported from __init__.py and exec'd from setup.py
+
+__version__ = "1.1.0+dev"
+VERSION = (1, 1, 0)

setup.py

@@ -1,16 +1,18 @@
 import os, sys
 from setuptools import setup, Extension
 
-VERSION = "0.9.0"  # Remember to change CHANGES.txt and netfilterqueue.pyx when version changes.
+exec(open("netfilterqueue/_version.py", encoding="utf-8").read())
 
-setup_requires = []
+setup_requires = ["wheel"]
 try:
     # Use Cython
     from Cython.Build import cythonize
 
     ext_modules = cythonize(
         Extension(
-            "netfilterqueue", ["netfilterqueue.pyx"], libraries=["netfilter_queue"]
+            "netfilterqueue._impl",
+            ["netfilterqueue/_impl.pyx"],
+            libraries=["netfilter_queue"],
         ),
         compiler_directives={"language_level": "3str"},
     )
@@ -19,9 +21,9 @@ except ImportError:
     if "egg_info" in sys.argv:
         # We're being run by pip to figure out what we need. Request cython in
         # setup_requires below.
-        setup_requires = ["cython"]
+        setup_requires += ["cython"]
     elif not os.path.exists(
-        os.path.join(os.path.dirname(__file__), "netfilterqueue.c")
+        os.path.join(os.path.dirname(__file__), "netfilterqueue/_impl.c")
     ):
         sys.stderr.write(
             "You must have Cython installed (`pip install cython`) to build this "
@@ -31,21 +33,28 @@ except ImportError:
         )
         sys.exit(1)
     ext_modules = [
-        Extension("netfilterqueue", ["netfilterqueue.c"], libraries=["netfilter_queue"])
+        Extension(
+            "netfilterqueue._impl",
+            ["netfilterqueue/_impl.c"],
+            libraries=["netfilter_queue"],
+        )
     ]
 
 setup(
-    ext_modules=ext_modules,
-    setup_requires=setup_requires,
-    python_requires=">=3.6",
     name="NetfilterQueue",
-    version=VERSION,
+    version=__version__,
     license="MIT",
-    author="Matthew Fox",
-    author_email="matt@tansen.ca",
+    author="Matthew Fox <matt@tansen.ca>, Joshua Oreman <oremanj@gmail.com>",
+    author_email="oremanj@gmail.com",
     url="https://github.com/oremanj/python-netfilterqueue",
     description="Python bindings for libnetfilter_queue",
-    long_description=open("README.rst").read(),
+    long_description=open("README.rst", encoding="utf-8").read(),
+    packages=["netfilterqueue"],
+    ext_modules=ext_modules,
+    include_package_data=True,
+    exclude_package_data={"netfilterqueue": ["*.c"]},
+    setup_requires=setup_requires,
+    python_requires=">=3.6",
     classifiers=[
         "Development Status :: 5 - Production/Stable",
         "License :: OSI Approved :: MIT License",

test-requirements.in

@@ -5,3 +5,4 @@ pytest-trio
 async_generator
 black
 platformdirs <= 2.4.0  # needed by black; 2.4.1+ don't support py3.6
+mypy; implementation_name == "cpython"

test-requirements.txt

@@ -22,8 +22,12 @@ idna==3.3
     # via trio
 iniconfig==1.1.1
     # via pytest
+mypy==0.931 ; implementation_name == "cpython"
+    # via -r test-requirements.in
 mypy-extensions==0.4.3
-    # via black
+    # via
+    #   black
+    #   mypy
 outcome==1.1.0
     # via
     #   pytest-trio
@@ -57,10 +61,14 @@ sortedcontainers==2.4.0
 toml==0.10.2
     # via pytest
 tomli==1.2.3
-    # via black
+    # via
+    #   black
+    #   mypy
 trio==0.19.0
     # via
     #   -r test-requirements.in
     #   pytest-trio
 typing-extensions==4.0.1
-    # via black
+    # via
+    #   black
+    #   mypy

tests/conftest.py

@@ -5,12 +5,12 @@ import socket
 import subprocess
 import sys
 import trio
-import unshare
+import unshare  # type: ignore
 import netfilterqueue
 from functools import partial
-from typing import AsyncIterator, Callable, Optional, Tuple
+from typing import Any, AsyncIterator, Callable, Dict, Optional, Tuple
 from async_generator import asynccontextmanager
-from pytest_trio.enable_trio_mode import *
+from pytest_trio.enable_trio_mode import *  # type: ignore
 
 
 # We'll create three network namespaces, representing a router (which
@@ -45,8 +45,8 @@ def enter_netns() -> None:
     subprocess.run("/sbin/ip link set lo up".split(), check=True)
 
 
-@pytest.hookimpl(tryfirst=True)
-def pytest_runtestloop():
+@pytest.hookimpl(tryfirst=True)  # type: ignore
+def pytest_runtestloop() -> None:
     if os.getuid() != 0:
         # Create a new user namespace for the whole test session
         outer = {"uid": os.getuid(), "gid": os.getgid()}
@@ -93,7 +93,9 @@ async def peer_main(idx: int, parent_fd: int) -> None:
     await peer.connect((peer_ip, peer_port))
 
     # Enter the message-forwarding loop
-    async def proxy_one_way(src, dest):
+    async def proxy_one_way(
+        src: trio.socket.SocketType, dest: trio.socket.SocketType
+    ) -> None:
         while src.fileno() >= 0:
             try:
                 msg = await src.recv(4096)
@@ -121,13 +123,13 @@ def _default_capture_cb(
 
 
 class Harness:
-    def __init__(self):
-        self._received = {}
-        self._conn = {}
-        self.dest_addr = {}
+    def __init__(self) -> None:
+        self._received: Dict[int, trio.MemoryReceiveChannel[bytes]] = {}
+        self._conn: Dict[int, trio.socket.SocketType] = {}
+        self.dest_addr: Dict[int, Tuple[str, int]] = {}
         self.failed = False
 
-    async def _run_peer(self, idx: int, *, task_status):
+    async def _run_peer(self, idx: int, *, task_status: Any) -> None:
         their_ip = PEER_IP[idx]
         my_ip = ROUTER_IP[idx]
         conn, child_conn = trio.socket.socketpair(socket.AF_UNIX, socket.SOCK_SEQPACKET)
@@ -169,10 +171,10 @@ class Harness:
                 # and its netns goes away. check=False to suppress that error.
                 await trio.run_process(f"ip link delete veth{idx}".split(), check=False)
 
-    async def _manage_peer(self, idx: int, *, task_status):
+    async def _manage_peer(self, idx: int, *, task_status: Any) -> None:
         async with trio.open_nursery() as nursery:
             await nursery.start(self._run_peer, idx)
-            packets_w, packets_r = trio.open_memory_channel(math.inf)
+            packets_w, packets_r = trio.open_memory_channel[bytes](math.inf)
             self._received[idx] = packets_r
             task_status.started()
             async with packets_w:
@@ -183,7 +185,7 @@ class Harness:
                     await packets_w.send(msg)
 
     @asynccontextmanager
-    async def run(self):
+    async def run(self) -> AsyncIterator[None]:
         async with trio.open_nursery() as nursery:
             async with trio.open_nursery() as start_nursery:
                 start_nursery.start_soon(nursery.start, self._manage_peer, 1)
@@ -258,14 +260,14 @@ class Harness:
         **options: int,
     ) -> AsyncIterator["trio.MemoryReceiveChannel[netfilterqueue.Packet]"]:
 
-        packets_w, packets_r = trio.open_memory_channel(math.inf)
+        packets_w, packets_r = trio.open_memory_channel[netfilterqueue.Packet](math.inf)
         queue_num, nfq = self.bind_queue(partial(cb, packets_w), **options)
         try:
             async with self.enqueue_packets_to(idx, queue_num):
                 async with packets_w, trio.open_nursery() as nursery:
 
                     @nursery.start_soon
-                    async def listen_for_packets():
+                    async def listen_for_packets() -> None:
                         while True:
                             await trio.lowlevel.wait_readable(nfq.get_fd())
                             nfq.run(block=False)
@@ -275,7 +277,7 @@ class Harness:
         finally:
             nfq.unbind()
 
-    async def expect(self, idx: int, *packets: bytes):
+    async def expect(self, idx: int, *packets: bytes) -> None:
         for expected in packets:
             with trio.move_on_after(5) as scope:
                 received = await self._received[idx].receive()
@@ -291,13 +293,13 @@ class Harness:
                     f"received {received!r}"
                 )
 
-    async def send(self, idx: int, *packets: bytes):
+    async def send(self, idx: int, *packets: bytes) -> None:
         for packet in packets:
             await self._conn[3 - idx].send(packet)
 
 
 @pytest.fixture
-async def harness() -> Harness:
+async def harness() -> AsyncIterator[Harness]:
     h = Harness()
     async with h.run():
         yield h

tests/test_basic.py

@@ -23,6 +23,7 @@ async def test_comms_without_queue(harness):
 async def test_queue_dropping(harness):
     async def drop(packets, msg):
         async for packet in packets:
+            assert "UDP packet" in str(packet)
             if packet.get_payload()[28:] == msg:
                 packet.drop()
             else:
@@ -118,11 +119,13 @@ async def test_mark_repeat(harness):
     assert t0 < timestamps[0] < t1
 
 
-async def test_hwaddr(harness):
+async def test_hwaddr_and_inoutdev(harness):
     hwaddrs = []
+    inoutdevs = []
 
     def cb(pkt):
         hwaddrs.append((pkt.get_hw(), pkt.hook, pkt.get_payload()[28:]))
+        inoutdevs.append((pkt.indev, pkt.outdev))
         pkt.accept()
 
     queue_num, nfq = harness.bind_queue(cb)
@@ -161,6 +164,20 @@ async def test_hwaddr(harness):
         (None, OUTPUT, b"four"),
     ]
 
+    if sys.implementation.name != "pypy":
+        # pypy doesn't appear to provide if_nametoindex()
+        iface1 = socket.if_nametoindex("veth1")
+        iface2 = socket.if_nametoindex("veth2")
+    else:
+        iface1, iface2 = inoutdevs[0]
+    assert 0 != iface1 != iface2 != 0
+    assert inoutdevs == [
+        (iface1, iface2),
+        (iface1, iface2),
+        (0, iface2),
+        (0, iface2),
+    ]
+
 
 async def test_errors(harness):
     with pytest.warns(RuntimeWarning, match="rcvbuf limit is") as record:
@@ -190,6 +207,7 @@ async def test_errors(harness):
 async def test_unretained(harness):
     def cb(chan, pkt):
         # Can access payload within callback
+        assert "UDP packet" in str(pkt)
         assert pkt.get_payload()[-3:] in (b"one", b"two")
         chan.send_nowait(pkt)
 
@@ -202,6 +220,7 @@ async def test_unretained(harness):
                 RuntimeError, match="Payload data is no longer available"
             ):
                 p.get_payload()
+            assert "contents unretained" in str(p)
             # Can still issue verdicts though
             if accept:
                 p.accept()