drop the send-time public address check

Refusing non-global resolved addresses broke Tailscale (100.64.0.0/10), split-horizon DNS and HTTPS_PROXY-only setups, and added little once redirects are no longer followed and TLS verification stays on.
This commit is contained in:
Josh Hawkins
2026-10-18 13:12:48 -05:00
parent 6ad2367a27
commit 7e2d04f851
2 changed files with 2 additions and 102 deletions
-24
View File
@@ -6,7 +6,6 @@ import json
import logging import logging
import os import os
import queue import queue
import socket
import threading import threading
from collections.abc import Callable, Iterator from collections.abc import Callable, Iterator
from dataclasses import dataclass from dataclasses import dataclass
@@ -124,20 +123,6 @@ class _NoRedirectSession(requests.Session):
_push_session = _NoRedirectSession() _push_session = _NoRedirectSession()
def _resolves_to_public_address(endpoint: str) -> bool:
hostname = urlparse(endpoint).hostname
if not hostname:
return False
try:
infos = socket.getaddrinfo(hostname, 443, proto=socket.IPPROTO_TCP)
except (socket.gaierror, UnicodeError):
return False
return all(ipaddress.ip_address(info[4][0]).is_global for info in infos)
def _vapid_audience(endpoint: str) -> str: def _vapid_audience(endpoint: str) -> str:
parsed = urlparse(endpoint) parsed = urlparse(endpoint)
return f"{parsed.scheme}://{parsed.netloc}" return f"{parsed.scheme}://{parsed.netloc}"
@@ -434,15 +419,6 @@ class WebPushClient(Communicator):
for pusher in self.web_pushers[notification.user]: for pusher in self.web_pushers[notification.user]:
endpoint = pusher.subscription_info["endpoint"] endpoint = pusher.subscription_info["endpoint"]
# Resolve-then-send leaves a DNS rebinding window. Closing it
# needs the connection pinned to the checked address.
if not _resolves_to_public_address(endpoint):
logger.warning(
"Skipping notification to %s: endpoint does not resolve to a public address",
notification.user,
)
continue
headers = self.claim_headers[_vapid_audience(endpoint)].copy() headers = self.claim_headers[_vapid_audience(endpoint)].copy()
headers["urgency"] = "high" headers["urgency"] = "high"
+2 -78
View File
@@ -1,27 +1,15 @@
"""Tests that push sends cannot be aimed at internal addresses.""" """Tests that push sends never follow redirects."""
import queue
import socket
import unittest import unittest
from unittest.mock import MagicMock, patch
import requests import requests
from requests.adapters import BaseAdapter from requests.adapters import BaseAdapter
from frigate.comms.webpush import ( from frigate.comms.webpush import _NoRedirectSession
PushNotification,
WebPushClient,
_NoRedirectSession,
_resolves_to_public_address,
)
ENDPOINT = "https://push.example.com/token" ENDPOINT = "https://push.example.com/token"
def _addrinfo(address: str) -> list:
return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (address, 443))]
class RedirectAdapter(BaseAdapter): class RedirectAdapter(BaseAdapter):
def __init__(self): def __init__(self):
super().__init__() super().__init__()
@@ -53,69 +41,5 @@ class TestNoRedirectSession(unittest.TestCase):
self.assertEqual(adapter.calls, 1) self.assertEqual(adapter.calls, 1)
class TestResolvesToPublicAddress(unittest.TestCase):
def test_accepts_global_address(self):
with patch("socket.getaddrinfo", return_value=_addrinfo("8.8.8.8")):
self.assertTrue(_resolves_to_public_address(ENDPOINT))
def test_rejects_non_global_addresses(self):
for address in ("127.0.0.1", "10.0.0.5", "169.254.169.254", "::1"):
with (
self.subTest(address=address),
patch("socket.getaddrinfo", return_value=_addrinfo(address)),
):
self.assertFalse(_resolves_to_public_address(ENDPOINT))
def test_rejects_when_any_address_is_non_global(self):
infos = _addrinfo("8.8.8.8") + _addrinfo("127.0.0.1")
with patch("socket.getaddrinfo", return_value=infos):
self.assertFalse(_resolves_to_public_address(ENDPOINT))
def test_rejects_unresolvable_host(self):
with patch("socket.getaddrinfo", side_effect=socket.gaierror):
self.assertFalse(_resolves_to_public_address(ENDPOINT))
class TestProcessNotifications(unittest.TestCase):
def _client(self, pusher: MagicMock) -> WebPushClient:
client = WebPushClient.__new__(WebPushClient)
client.stop_event = MagicMock()
client.stop_event.is_set.side_effect = [False, True]
client.notification_queue = queue.Queue()
client.notification_queue.put(
PushNotification(user="viewer", payload={}, title="t", message="m")
)
client.check_registrations = MagicMock()
client.claim_headers = {"https://push.example.com": {}}
client.web_pushers = {"viewer": [pusher]}
client.expired_subs = {}
return client
def test_skips_endpoint_resolving_to_internal_address(self):
pusher = MagicMock()
pusher.subscription_info = {"endpoint": ENDPOINT}
client = self._client(pusher)
with (
patch("socket.getaddrinfo", return_value=_addrinfo("127.0.0.1")),
self.assertLogs("frigate.comms.webpush", level="WARNING"),
):
client._process_notifications()
pusher.send.assert_not_called()
def test_sends_to_public_endpoint(self):
pusher = MagicMock()
pusher.subscription_info = {"endpoint": ENDPOINT}
pusher.send.return_value.status_code = 201
client = self._client(pusher)
with patch("socket.getaddrinfo", return_value=_addrinfo("8.8.8.8")):
client._process_notifications()
pusher.send.assert_called_once()
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()