From 52403ca0a45202eb120228d03e9732072ec1c6a2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 15:42:18 +0100 Subject: [PATCH 01/12] add whitelist for webpush endpoint domains --- sygnal/utils.py | 26 +++++++++++++++++++++++++- sygnal/webpushpushkin.py | 14 +++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/sygnal/utils.py b/sygnal/utils.py index 0da68868..28bb7eb8 100644 --- a/sygnal/utils.py +++ b/sygnal/utils.py @@ -15,7 +15,7 @@ from logging import LoggerAdapter from twisted.internet.defer import Deferred - +import re async def twisted_sleep(delay, twisted_reactor): """ @@ -37,3 +37,27 @@ async def twisted_sleep(delay, twisted_reactor): class NotificationLoggerAdapter(LoggerAdapter): def process(self, msg, kwargs): return f"[{self.extra['request_id']}] {msg}", kwargs + + +def glob_to_regex(glob): + """Converts a glob to a compiled regex object. + + The regex is anchored at the beginning and end of the string. + + Args: + glob (str) + + Returns: + re.RegexObject + """ + res = "" + for c in glob: + if c == "*": + res = res + ".*" + elif c == "?": + res = res + "." + else: + res = res + re.escape(c) + + # \A anchors at start of string, \Z at end of string + return re.compile(r"\A" + res + r"\Z", re.IGNORECASE) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index ff53594d..9a69d3c1 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -28,6 +28,7 @@ from sygnal.helper.context_factory import ClientTLSOptionsFactory from sygnal.helper.proxy.proxyagent_twisted import ProxyAgent +from .utils import glob_to_regex from .exceptions import PushkinSetupException from .notifications import ConcurrencyLimitedPushkin @@ -96,6 +97,11 @@ def __init__(self, name, sygnal, config): ) self.http_agent_wrapper = HttpAgentWrapper(self.http_agent) + allowed_endpoints = self.get_config("allowed_endpoints") + if allowed_endpoints: + if not isinstance(allowed_endpoints, list): + raise PushkinSetupException("'allowed_endpoints' should be a list or not set") + self.allowed_endpoints = list(map(glob_to_regex, allowed_endpoints)) privkey_filename = self.get_config("vapid_private_key") if not privkey_filename: raise PushkinSetupException("'vapid_private_key' not set in config") @@ -119,6 +125,12 @@ async def _dispatch_notification_unlimited(self, n, device, context): endpoint = device.data.get("endpoint") auth = device.data.get("auth") + endpoint_domain = urlparse(endpoint).netloc + if self.allowed_endpoints: + match = next((regex for regex in self.allowed_endpoints if regex.fullmatch(endpoint_domain)), None) + if not match: + logger.error("push gateway %s is not in allowed_endpoints, blocking request", endpoint_domain) + return []; # don't reject push key though if not p256dh or not endpoint or not auth: logger.warn( @@ -168,7 +180,7 @@ async def _dispatch_notification_unlimited(self, n, device, context): logger.warn( "Rejecting pushkey %s; gateway %s failed with %d: %s", device.pushkey, - urlparse(endpoint).netloc, + endpoint_domain, response.code, response_text, ) From ff1d2257d9e66f378d8e2a96beda5f2dcd4e7937 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 15:50:27 +0100 Subject: [PATCH 02/12] add changelog file --- changelog.d/182.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/182.feature diff --git a/changelog.d/182.feature b/changelog.d/182.feature new file mode 100644 index 00000000..66db9691 --- /dev/null +++ b/changelog.d/182.feature @@ -0,0 +1 @@ +Add 'allowed_endpoints' configuration option for webpush pushkins to whitelist the allowed endpoints. \ No newline at end of file From c3c03efd02daf46f23bac5c18d5cea5a3a3a18f9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 15:51:38 +0100 Subject: [PATCH 03/12] reformatting --- sygnal/utils.py | 1 + sygnal/webpushpushkin.py | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/sygnal/utils.py b/sygnal/utils.py index 28bb7eb8..9680ecb0 100644 --- a/sygnal/utils.py +++ b/sygnal/utils.py @@ -17,6 +17,7 @@ from twisted.internet.defer import Deferred import re + async def twisted_sleep(delay, twisted_reactor): """ Creates a Deferred which will fire in a set time. diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index 9a69d3c1..f7addf48 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -100,7 +100,9 @@ def __init__(self, name, sygnal, config): allowed_endpoints = self.get_config("allowed_endpoints") if allowed_endpoints: if not isinstance(allowed_endpoints, list): - raise PushkinSetupException("'allowed_endpoints' should be a list or not set") + raise PushkinSetupException( + "'allowed_endpoints' should be a list or not set" + ) self.allowed_endpoints = list(map(glob_to_regex, allowed_endpoints)) privkey_filename = self.get_config("vapid_private_key") if not privkey_filename: @@ -127,10 +129,21 @@ async def _dispatch_notification_unlimited(self, n, device, context): auth = device.data.get("auth") endpoint_domain = urlparse(endpoint).netloc if self.allowed_endpoints: - match = next((regex for regex in self.allowed_endpoints if regex.fullmatch(endpoint_domain)), None) + match = next( + ( + regex + for regex in self.allowed_endpoints + if regex.fullmatch(endpoint_domain) + ), + None, + ) if not match: - logger.error("push gateway %s is not in allowed_endpoints, blocking request", endpoint_domain) - return []; # don't reject push key though + logger.error( + "push gateway %s is not in allowed_endpoints, blocking request", + endpoint_domain, + ) + return [] + # don't reject push key though if not p256dh or not endpoint or not auth: logger.warn( From 68ce5d426bb52752960b376a1f6571bcdfae0a55 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 16:00:11 +0100 Subject: [PATCH 04/12] the linter likes his imports in order --- sygnal/webpushpushkin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index f7addf48..0bb7bf18 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -28,9 +28,9 @@ from sygnal.helper.context_factory import ClientTLSOptionsFactory from sygnal.helper.proxy.proxyagent_twisted import ProxyAgent -from .utils import glob_to_regex from .exceptions import PushkinSetupException from .notifications import ConcurrencyLimitedPushkin +from .utils import glob_to_regex QUEUE_TIME_HISTOGRAM = Histogram( "sygnal_webpush_queue_time", From 329f3aadd646c3c6e0d9e3e414ec209ea7c7027c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 16:03:54 +0100 Subject: [PATCH 05/12] more import ordering --- sygnal/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sygnal/utils.py b/sygnal/utils.py index 9680ecb0..3ce3bfaa 100644 --- a/sygnal/utils.py +++ b/sygnal/utils.py @@ -12,10 +12,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import re from logging import LoggerAdapter from twisted.internet.defer import Deferred -import re async def twisted_sleep(delay, twisted_reactor): From aaef72028abfde6f21383d67f5ee9efbeb289177 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 18:48:57 +0100 Subject: [PATCH 06/12] add documentation for allowed_endpoints --- docs/applications.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/applications.md b/docs/applications.md index 25584925..096ce482 100644 --- a/docs/applications.md +++ b/docs/applications.md @@ -226,6 +226,19 @@ You also need to set an e-mail address in `vapid_contact_email` in the config fi where the push gateway operator can reach you in case they need to notify you about your usage of their API. +Since for webpush, the push gateway endpoint is variable and comes from the browser +through the push data, you may not want to have your sygnal instance connect to any +random addressable server. For this, you can set the `allowed_endpoints` option to +a list of allowed endpoints. Globs are supported. For example, to allow Firefox, +Chrome and Opera (Google) and Edge as a push gateway, you can use this: + +```yaml +allowed_endpoints: + - "updates.push.services.mozilla.com" + - "fcm.googleapis.com" + - "*.notify.windows.com" +``` + #### Push key and expected push data In your web application, [the push manager subscribe method](https://developer.mozilla.org/en-US/docs/Web/API/PushManager/subscribe) From 669213f3a1493b4428e630f2b2d10fd81e6adf18 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 19:12:45 +0100 Subject: [PATCH 07/12] cleanup finding matching allowed endpoint --- sygnal/webpushpushkin.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index 0bb7bf18..7475aac4 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -129,15 +129,10 @@ async def _dispatch_notification_unlimited(self, n, device, context): auth = device.data.get("auth") endpoint_domain = urlparse(endpoint).netloc if self.allowed_endpoints: - match = next( - ( - regex - for regex in self.allowed_endpoints - if regex.fullmatch(endpoint_domain) - ), - None, + allowed = any( + regex.fullmatch(endpoint_domain) for regex in self.allowed_endpoints ) - if not match: + if not allowed: logger.error( "push gateway %s is not in allowed_endpoints, blocking request", endpoint_domain, From 9d7c1b01332c4faee0cd85dcafc86902df4d9720 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 18:54:37 +0100 Subject: [PATCH 08/12] move comment above return --- sygnal/webpushpushkin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index 7475aac4..3b2ba4de 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -137,8 +137,8 @@ async def _dispatch_notification_unlimited(self, n, device, context): "push gateway %s is not in allowed_endpoints, blocking request", endpoint_domain, ) + # abort, but don't reject push key return [] - # don't reject push key though if not p256dh or not endpoint or not auth: logger.warn( From a8765c1a57d26a19e5507ad0cc2e15b8399221d3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 18:56:44 +0100 Subject: [PATCH 09/12] set allowed_endpoints to None when not specified --- sygnal/webpushpushkin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index 3b2ba4de..c5eed245 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -104,6 +104,8 @@ def __init__(self, name, sygnal, config): "'allowed_endpoints' should be a list or not set" ) self.allowed_endpoints = list(map(glob_to_regex, allowed_endpoints)) + else: + self.allowed_endpoints = None privkey_filename = self.get_config("vapid_private_key") if not privkey_filename: raise PushkinSetupException("'vapid_private_key' not set in config") From 5566dd5b29731af3800cb531e5f6a3d30d3dedc6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 20:14:02 +0100 Subject: [PATCH 10/12] make type check happy --- sygnal/webpushpushkin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index c5eed245..f580b4b8 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -17,6 +17,7 @@ import os.path from io import BytesIO from urllib.parse import urlparse +from typing import List, Optional, Pattern from prometheus_client import Gauge, Histogram from py_vapid import Vapid, VapidException @@ -97,6 +98,7 @@ def __init__(self, name, sygnal, config): ) self.http_agent_wrapper = HttpAgentWrapper(self.http_agent) + self.allowed_endpoints = None # type: Optional[List[Pattern]] allowed_endpoints = self.get_config("allowed_endpoints") if allowed_endpoints: if not isinstance(allowed_endpoints, list): @@ -104,8 +106,6 @@ def __init__(self, name, sygnal, config): "'allowed_endpoints' should be a list or not set" ) self.allowed_endpoints = list(map(glob_to_regex, allowed_endpoints)) - else: - self.allowed_endpoints = None privkey_filename = self.get_config("vapid_private_key") if not privkey_filename: raise PushkinSetupException("'vapid_private_key' not set in config") From 299bb7772ad41ea61933f33216020717c5746f57 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 19:14:55 +0000 Subject: [PATCH 11/12] Update changelog.d/182.feature Co-authored-by: Patrick Cloke --- changelog.d/182.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/182.feature b/changelog.d/182.feature index 66db9691..e071dcd8 100644 --- a/changelog.d/182.feature +++ b/changelog.d/182.feature @@ -1 +1 @@ -Add 'allowed_endpoints' configuration option for webpush pushkins to whitelist the allowed endpoints. \ No newline at end of file +Add 'allowed_endpoints' configuration option for limiting the endpoints that WebPush pushkins will contact. From c16334ed9864e33e1fb05d3246a3c3b6e79a8332 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Mar 2021 20:16:26 +0100 Subject: [PATCH 12/12] import ordeeeer! --- sygnal/webpushpushkin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py index f580b4b8..12582d95 100644 --- a/sygnal/webpushpushkin.py +++ b/sygnal/webpushpushkin.py @@ -16,8 +16,8 @@ import logging import os.path from io import BytesIO -from urllib.parse import urlparse from typing import List, Optional, Pattern +from urllib.parse import urlparse from prometheus_client import Gauge, Histogram from py_vapid import Vapid, VapidException