Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/zeroconf/_listener.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ cdef bint TYPE_CHECKING

cdef cython.uint _MAX_MSG_ABSOLUTE
cdef cython.uint _DUPLICATE_PACKET_SUPPRESSION_INTERVAL
cdef cython.uint _RECENT_PACKETS_MAX


cdef class AsyncListener:
Expand All @@ -30,11 +31,12 @@ cdef class AsyncListener:
cdef public cython.dict _deferred
cdef public cython.dict _timers
cdef public cython.dict _deferred_deadlines
cdef public cython.dict _recent_packets

@cython.locals(now=double, debug=cython.bint)
cpdef datagram_received(self, cython.bytes bytes, cython.tuple addrs)

@cython.locals(msg=DNSIncoming)
@cython.locals(msg=DNSIncoming, recent_time=double)
cpdef _process_datagram_at_time(self, bint debug, cython.uint data_len, double now, bytes data, cython.tuple addrs)

cdef _cancel_any_timers_for_addr(self, object addr)
Expand Down
43 changes: 42 additions & 1 deletion src/zeroconf/_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from ._protocol.incoming import DNSIncoming
from ._transport import _WrappedTransport, make_wrapped_transport
from ._utils.time import current_time_millis, millis_to_seconds
from .const import _DUPLICATE_PACKET_SUPPRESSION_INTERVAL, _MAX_MSG_ABSOLUTE
from .const import _DUPLICATE_PACKET_SUPPRESSION_INTERVAL, _MAX_MSG_ABSOLUTE, _RECENT_PACKETS_MAX
Comment thread
bdraco marked this conversation as resolved.

if TYPE_CHECKING:
from ._core import Zeroconf
Expand Down Expand Up @@ -60,6 +60,7 @@ class AsyncListener:
"_deferred",
"_deferred_deadlines",
"_query_handler",
"_recent_packets",
"_record_manager",
"_registry",
"_timers",
Expand All @@ -84,6 +85,12 @@ def __init__(self, zc: Zeroconf) -> None:
self._deferred: dict[str, list[DNSIncoming]] = {}
self._timers: dict[str, asyncio.TimerHandle] = {}
self._deferred_deadlines: dict[str, float] = {}
# Bounded recency window so an alternating (A, B, A, B, ...)
# flood cannot defeat single-slot dedup; relies on dict insertion
# order so the oldest entry is evicted first. Only payloads
# without a QU question are cached so unicast replies still go
# out on every receipt.
self._recent_packets: dict[bytes, float] = {}
super().__init__()

def datagram_received(self, data: _bytes, addrs: tuple[str, int] | tuple[str, int, int, int]) -> None:
Expand Down Expand Up @@ -130,6 +137,31 @@ def _process_datagram_at_time(
)
return

# `get(data, -1e30)` keeps the suppression compare a single C
# double compare; the sentinel is far below any real `now -
# _DUPLICATE_PACKET_SUPPRESSION_INTERVAL` so a cache miss never
# triggers the branch even when `now` is small (time.monotonic
# is allowed to start near zero, leaving `now - INTERVAL`
# negative for the first ~1s after boot). Only non-QU payloads
# are cached, so any hit here is safe to suppress without re-
# checking has_qu_question.
recent_time = self._recent_packets.get(data, -1e30)
if (now - _DUPLICATE_PACKET_SUPPRESSION_INTERVAL) < recent_time:
# No timestamp refresh on hit so the suppression window
# ends at first-observation + interval; one parse-and-
# dispatch fires per payload per interval, capping the
# CPU cost under a sustained alternating flood.
if debug:
log.debug(
"Ignoring duplicate message with no unicast questions"
" received from %s [socket %s] (%d bytes) as [%r]",
addrs,
self.sock_description,
data_len,
data,
)
return

if len(addrs) == 2:
v6_flow_scope: tuple[()] | tuple[int, int] = ()
# https://github.com/python/mypy/issues/1178
Expand All @@ -150,6 +182,15 @@ def _process_datagram_at_time(
self.data = data
self.last_time = now
self.last_message = msg
if not msg.has_qu_question():
# Refresh LRU position when an entry exists but the
# suppression window has expired; otherwise evict the oldest
# entry once the window is full.
if data in self._recent_packets:
del self._recent_packets[data]
elif len(self._recent_packets) >= _RECENT_PACKETS_MAX:
del self._recent_packets[next(iter(self._recent_packets))]
self._recent_packets[data] = now
if msg.valid is True:
if debug:
log.debug(
Expand Down
6 changes: 6 additions & 0 deletions src/zeroconf/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
_LISTENER_TIME = 200 # ms
_BROWSER_TIME = 10000 # ms
_DUPLICATE_PACKET_SUPPRESSION_INTERVAL = 1000 # ms
# Per-listener bounded recency window. 16 is large enough to defeat
# the alternating-payload bypass (RFC 6762 §6.2, issue #1724 — even a
# rotation of a dozen distinct payloads still dedups), and small
# enough that the dict bookkeeping per miss stays cheap under a
# hostile flood.
_RECENT_PACKETS_MAX = 16
_DUPLICATE_QUESTION_INTERVAL = 999 # ms # Must be 1ms less than _DUPLICATE_PACKET_SUPPRESSION_INTERVAL
_CACHE_CLEANUP_INTERVAL = 10 # s
_LOADED_SYSTEM_TIMEOUT = 10 # s
Expand Down
8 changes: 8 additions & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ def has_working_ipv6():
def _clear_cache(zc: Zeroconf) -> None:
zc.cache.cache.clear()
zc.question_history.clear()
# Reset per-listener dedup state so identical packets sent in the
# next phase of the test are not suppressed by the bounded recency
# window populated during the previous phase.
if zc.engine is not None:
for protocol in zc.engine.protocols:
protocol._recent_packets.clear()
protocol.data = None
protocol.last_time = 0


def _backdate_cache(zc: Zeroconf, ms: int = 1100) -> None:
Expand Down
12 changes: 8 additions & 4 deletions tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,8 @@ async def test_response_aggregation_timings_multiple(run_isolated, disable_dupli
with patch.object(aiozc.zeroconf, "async_send") as send_mock:
send_mock.reset_mock()
protocol.datagram_received(query2.packets()[0], ("127.0.0.1", const._MDNS_PORT))
protocol.last_time = 0 # manually reset the last time to avoid duplicate packet suppression
protocol.last_time = 0 # manually reset to avoid duplicate packet suppression
protocol._recent_packets.clear()
await asyncio.sleep(0.2)
calls = send_mock.mock_calls
assert len(calls) == 1
Expand All @@ -1769,7 +1770,8 @@ async def test_response_aggregation_timings_multiple(run_isolated, disable_dupli

send_mock.reset_mock()
protocol.datagram_received(query2.packets()[0], ("127.0.0.1", const._MDNS_PORT))
protocol.last_time = 0 # manually reset the last time to avoid duplicate packet suppression
protocol.last_time = 0 # manually reset to avoid duplicate packet suppression
protocol._recent_packets.clear()
await asyncio.sleep(1.2)
calls = send_mock.mock_calls
assert len(calls) == 1
Expand All @@ -1780,9 +1782,11 @@ async def test_response_aggregation_timings_multiple(run_isolated, disable_dupli

send_mock.reset_mock()
protocol.datagram_received(query2.packets()[0], ("127.0.0.1", const._MDNS_PORT))
protocol.last_time = 0 # manually reset the last time to avoid duplicate packet suppression
protocol.last_time = 0 # manually reset to avoid duplicate packet suppression
protocol._recent_packets.clear()
protocol.datagram_received(query2.packets()[0], ("127.0.0.1", const._MDNS_PORT))
protocol.last_time = 0 # manually reset the last time to avoid duplicate packet suppression
protocol.last_time = 0 # manually reset to avoid duplicate packet suppression
protocol._recent_packets.clear()
# The minimum protected send_after is 1000ms + 20ms random; sleep
# well under that so coarse timers on slow runners cannot push the
# send into this window and flake the assertion.
Expand Down
180 changes: 166 additions & 14 deletions tests/test_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,16 @@ def handle_query_or_defer(
_handle_query_or_defer.assert_called_once()
_handle_query_or_defer.reset_mock()

# Now call with the different packet and handle_query_or_defer should fire
# Replay the first packet — the recency window remembers more than
# just the most recent payload, so this is a duplicate.
listener._process_datagram_at_time(
False,
len(packet_with_qm_question),
new_time,
packet_with_qm_question,
addrs,
)
_handle_query_or_defer.assert_called_once()
_handle_query_or_defer.assert_not_called()
_handle_query_or_defer.reset_mock()

# Now call with the different packet with qu question and handle_query_or_defer should fire
Expand All @@ -257,18 +258,8 @@ def handle_query_or_defer(

log.setLevel(logging.WARNING)

# Call with the QM packet again
listener._process_datagram_at_time(
False,
len(packet_with_qm_question),
new_time,
packet_with_qm_question,
addrs,
)
_handle_query_or_defer.assert_called_once()
_handle_query_or_defer.reset_mock()

# Now call with the same packet again and handle_query_or_defer should not fire
# Replay the QM packet with debug disabled — suppression must hold
# off the debug-log path too.
listener._process_datagram_at_time(
False,
len(packet_with_qm_question),
Expand All @@ -285,3 +276,164 @@ def handle_query_or_defer(
_handle_query_or_defer.reset_mock()

zc.close()


def test_guard_against_alternating_duplicate_packets() -> None:
"""Alternating two distinct payloads must not bypass duplicate suppression."""
zc = Zeroconf(interfaces=["127.0.0.1"])
zc.registry.async_add(
ServiceInfo(
"_http._tcp.local.",
"Test._http._tcp.local.",
server="Test._http._tcp.local.",
port=4,
)
)
zc.question_history = QuestionHistoryWithoutSuppression()

class SubListener(_listener.AsyncListener):
def handle_query_or_defer(
self,
msg: DNSIncoming,
addr: str,
port: int,
transport: _engine._WrappedTransport,
v6_flow_scope: tuple[()] | tuple[int, int] = (),
) -> None:
super().handle_query_or_defer(msg, addr, port, transport, v6_flow_scope)

listener = SubListener(zc)
listener.transport = MagicMock()

query_a = r.DNSOutgoing(const._FLAGS_QR_QUERY, multicast=True)
query_a.add_question(r.DNSQuestion("a._http._tcp.local.", const._TYPE_PTR, const._CLASS_IN))
packet_a = query_a.packets()[0]

query_b = r.DNSOutgoing(const._FLAGS_QR_QUERY, multicast=True)
query_b.add_question(r.DNSQuestion("b._http._tcp.local.", const._TYPE_PTR, const._CLASS_IN))
packet_b = query_b.packets()[0]

assert packet_a != packet_b

addrs = ("1.2.3.4", 43)

with patch.object(listener, "handle_query_or_defer") as _handle_query_or_defer:
now = current_time_millis()

# Prime both payloads.
listener._process_datagram_at_time(False, len(packet_a), now, packet_a, addrs)
listener._process_datagram_at_time(False, len(packet_b), now, packet_b, addrs)
assert _handle_query_or_defer.call_count == 2
_handle_query_or_defer.reset_mock()

for _ in range(4):
listener._process_datagram_at_time(False, len(packet_a), now, packet_a, addrs)
listener._process_datagram_at_time(False, len(packet_b), now, packet_b, addrs)
_handle_query_or_defer.assert_not_called()

zc.close()


def test_recent_packets_window_is_bounded() -> None:
"""Distinct payloads beyond the recency window evict oldest entries."""
zc = Zeroconf(interfaces=["127.0.0.1"])
zc.registry.async_add(
ServiceInfo(
"_http._tcp.local.",
"Test._http._tcp.local.",
server="Test._http._tcp.local.",
port=4,
)
)
zc.question_history = QuestionHistoryWithoutSuppression()

class SubListener(_listener.AsyncListener):
def handle_query_or_defer(
self,
msg: DNSIncoming,
addr: str,
port: int,
transport: _engine._WrappedTransport,
v6_flow_scope: tuple[()] | tuple[int, int] = (),
) -> None:
super().handle_query_or_defer(msg, addr, port, transport, v6_flow_scope)

listener = SubListener(zc)
listener.transport = MagicMock()

addrs = ("1.2.3.4", 43)
now = current_time_millis()

packets = []
for i in range(const._RECENT_PACKETS_MAX + 4):
query = r.DNSOutgoing(const._FLAGS_QR_QUERY, multicast=True)
query.add_question(r.DNSQuestion(f"n{i}._http._tcp.local.", const._TYPE_PTR, const._CLASS_IN))
packets.append(query.packets()[0])

with patch.object(listener, "handle_query_or_defer") as _handle_query_or_defer:
for packet in packets:
listener._process_datagram_at_time(False, len(packet), now, packet, addrs)
assert _handle_query_or_defer.call_count == len(packets)
_handle_query_or_defer.reset_mock()

# The newest _RECENT_PACKETS_MAX entries are still in the
# window; replaying them must be suppressed. Checked before
# replaying the evicted ones below since that would mutate the
# window and could mask an off-by-one in eviction.
kept = packets[-const._RECENT_PACKETS_MAX :]
Comment thread
bdraco marked this conversation as resolved.
for packet in kept:
listener._process_datagram_at_time(False, len(packet), now, packet, addrs)
_handle_query_or_defer.assert_not_called()

# The oldest packets should have been evicted and now replay.
evicted = packets[: len(packets) - const._RECENT_PACKETS_MAX]
for packet in evicted:
listener._process_datagram_at_time(False, len(packet), now, packet, addrs)
assert _handle_query_or_defer.call_count == len(evicted)

zc.close()


def test_recent_packets_miss_with_small_now_is_not_suppressed() -> None:
"""A cache miss must not trigger suppression when `now` is below the suppression interval."""
# time.monotonic() can start near zero on freshly booted systems, so
# `now - _DUPLICATE_PACKET_SUPPRESSION_INTERVAL` is negative for the
# first second of process lifetime. A 0.0 default on the recency
# dict would let any negative `now - INTERVAL` satisfy the compare
# and suppress legitimate traffic.
zc = Zeroconf(interfaces=["127.0.0.1"])
zc.registry.async_add(
ServiceInfo(
"_http._tcp.local.",
"Test._http._tcp.local.",
server="Test._http._tcp.local.",
port=4,
)
)
zc.question_history = QuestionHistoryWithoutSuppression()

class SubListener(_listener.AsyncListener):
def handle_query_or_defer(
self,
msg: DNSIncoming,
addr: str,
port: int,
transport: _engine._WrappedTransport,
v6_flow_scope: tuple[()] | tuple[int, int] = (),
) -> None:
super().handle_query_or_defer(msg, addr, port, transport, v6_flow_scope)

listener = SubListener(zc)
listener.transport = MagicMock()

query = r.DNSOutgoing(const._FLAGS_QR_QUERY, multicast=True)
query.add_question(r.DNSQuestion("a._http._tcp.local.", const._TYPE_PTR, const._CLASS_IN))
packet = query.packets()[0]

addrs = ("1.2.3.4", 43)

with patch.object(listener, "handle_query_or_defer") as _handle_query_or_defer:
listener._process_datagram_at_time(False, len(packet), 0.0, packet, addrs)
_handle_query_or_defer.assert_called_once()

zc.close()
Loading