From 848aed2d05c43c5f290c9f1807daa0fe7d4f041d Mon Sep 17 00:00:00 2001 From: Sorunome Date: Sat, 20 Feb 2021 14:13:06 +0100 Subject: [PATCH 1/8] Add encrypted http pusher --- changelog.d/11512.feature | 1 + synapse/push/httppusher.py | 130 ++++++++++++++++++++++++++------ synapse/python_dependencies.py | 3 + synapse/rest/client/versions.py | 2 + tests/push/test_http.py | 107 ++++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 22 deletions(-) create mode 100644 changelog.d/11512.feature diff --git a/changelog.d/11512.feature b/changelog.d/11512.feature new file mode 100644 index 000000000000..e2c127b6725c --- /dev/null +++ b/changelog.d/11512.feature @@ -0,0 +1 @@ +Add support for MSC3013 Encrypted Push. Contributed by Famedly GmbH / Sorunome. diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 3fa603ccb7f7..8448f3312e6b 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -1,5 +1,7 @@ # Copyright 2015, 2016 OpenMarket Ltd # Copyright 2017 New Vector Ltd +# Copyright 2021 Sorunome +# Copyright 2021 Famedly GmbH # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,10 +14,17 @@ # 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 hashlib +import hmac +import json import logging import urllib.parse from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Union +import unpaddedbase64 +from Crypto.Cipher import AES +from Crypto.Util.Padding import pad +from donna25519 import PrivateKey, PublicKey # type: ignore from prometheus_client import Counter from twisted.internet.error import AlreadyCalled, AlreadyCancelled @@ -106,9 +115,84 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): self.url = url self.http_client = hs.get_proxied_blacklisted_http_client() - self.data_minus_url = {} - self.data_minus_url.update(self.data) - del self.data_minus_url["url"] + self.sanitized_data = {} + self.sanitized_data.update(self.data) + del self.sanitized_data["url"] + + if "algorithm" not in self.data: + self.algorithm = "com.famedly.plain" + elif self.data["algorithm"] not in ( + "com.famedly.plain", + "com.famedly.curve25519-aes-sha2", + ): + raise PusherConfigException( + "'algorithm' must be one of 'com.famedly.plain' or 'com.famedly.curve25519-aes-sha2'" + ) + else: + self.algorithm = self.data["algorithm"] + + if self.algorithm == "com.famedly.curve25519-aes-sha2": + base64_public_key = self.data["public_key"] + if not isinstance(base64_public_key, str): + raise PusherConfigException("'public_key' must be a string") + try: + self.public_key = PublicKey( + unpaddedbase64.decode_base64(base64_public_key) + ) + except Exception as e: + logger.warning( + "Failed to unpack public key: %s: %s", type(e).__name__, e + ) + raise PusherConfigException( + "'public_key' must be a valid base64-encoded curve25519 public key" + ) + del self.sanitized_data["public_key"] + + def _process_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: + """Called to process a payload according to the algorithm the pusher is + configured with. Namely, if the algorithm is `com.famedly.curve25519-aes-sha2` + we will encrypt the payload. + + Args: + payload: The payload that should be processed + """ + if self.algorithm == "com.famedly.curve25519-aes-sha2": + # we have an encrypted pusher, encrypt the payload + cleartext_notif = payload["notification"] + devices = cleartext_notif["devices"] + del cleartext_notif["devices"] + cleartext = json.dumps(cleartext_notif) + + # create an ephemeral curve25519 keypair + private_key = PrivateKey() + # do ECDH + secret_key = private_key.do_exchange(self.public_key) + # expand with HKDF + zerosalt = bytes([0] * 32) + prk = hmac.new(zerosalt, secret_key, hashlib.sha256).digest() + aes_key = hmac.new(prk, bytes([1]), hashlib.sha256).digest() + mac_key = hmac.new(prk, aes_key + bytes([2]), hashlib.sha256).digest() + aes_iv = hmac.new(prk, mac_key + bytes([3]), hashlib.sha256).digest()[0:16] + # create the ciphertext with AES-CBC-256 + ciphertext = AES.new(aes_key, AES.MODE_CBC, aes_iv).encrypt( + pad(cleartext.encode("utf-8"), AES.block_size) + ) + # create the mac + mac = hmac.new(mac_key, ciphertext, hashlib.sha256).digest()[0:8] + + return { + "notification": { + "ephemeral": unpaddedbase64.encode_base64( + private_key.get_public().public + ), + "ciphertext": unpaddedbase64.encode_base64(ciphertext), + "mac": unpaddedbase64.encode_base64(mac), + "devices": devices, + } + } + + # else fall back to just plaintext + return payload def on_started(self, should_check_for_notifs: bool) -> None: """Called when this pusher has been started. @@ -333,12 +417,12 @@ async def _build_notification_dict( "app_id": self.app_id, "pushkey": self.pushkey, "pushkey_ts": int(self.pushkey_ts / 1000), - "data": self.data_minus_url, + "data": self.sanitized_data, } ], } } - return d + return self._process_notification_dict(d) ctx = await push_tools.get_context_for_event(self.storage, event, self.user_id) @@ -359,7 +443,7 @@ async def _build_notification_dict( "app_id": self.app_id, "pushkey": self.pushkey, "pushkey_ts": int(self.pushkey_ts / 1000), - "data": self.data_minus_url, + "data": self.sanitized_data, "tweaks": tweaks, } ], @@ -378,7 +462,7 @@ async def _build_notification_dict( if "name" in ctx and len(ctx["name"]) > 0: d["notification"]["room_name"] = ctx["name"] - return d + return self._process_notification_dict(d) async def dispatch_push( self, event: EventBase, tweaks: Dict[str, bool], badge: int @@ -410,22 +494,24 @@ async def _send_badge(self, badge: int) -> None: badge: number of unread messages """ logger.debug("Sending updated badge count %d to %s", badge, self.name) - d = { - "notification": { - "id": "", - "type": None, - "sender": "", - "counts": {"unread": badge}, - "devices": [ - { - "app_id": self.app_id, - "pushkey": self.pushkey, - "pushkey_ts": int(self.pushkey_ts / 1000), - "data": self.data_minus_url, - } - ], + d = self._process_notification_dict( + { + "notification": { + "id": "", + "type": None, + "sender": "", + "counts": {"unread": badge}, + "devices": [ + { + "app_id": self.app_id, + "pushkey": self.pushkey, + "pushkey_ts": int(self.pushkey_ts / 1000), + "data": self.sanitized_data, + } + ], + } } - } + ) try: await self.http_client.post_json_get_json(self.url, d) http_badges_processed_counter.inc() diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 7d26954244ea..5738de7f561f 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -87,6 +87,9 @@ # with the latest security patches. "cryptography>=3.4.7", "ijson>=3.1", + # Used for encrypted push + "pycryptodome>=3", + "donna25519>=0.1.1", ] CONDITIONAL_REQUIREMENTS = { diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 8d888f456520..6f44151d2028 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -93,6 +93,8 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]: "org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled, # Supports receiving hidden read receipts as per MSC2285 "org.matrix.msc2285": self.config.experimental.msc2285_enabled, + # Adds support for encrypted push + "com.famedly.msc3013": True, }, }, ) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index c068d329a98b..03919e6c2177 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -11,8 +11,16 @@ # 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 hashlib +import hmac +import json from unittest.mock import Mock +import unpaddedbase64 +from Crypto.Cipher import AES +from Crypto.Util.Padding import unpad +from donna25519 import PrivateKey, PublicKey # type: ignore + from twisted.internet.defer import Deferred import synapse.rest.admin @@ -198,6 +206,105 @@ def test_sends_http(self): self.assertEqual(len(pushers), 1) self.assertTrue(pushers[0].last_stream_ordering > last_stream_ordering) + def test_sends_encrypted_push(self): + """ + The HTTP pusher will send an encrypted push message if the pusher + has been configured with a public key and the corresponding algorithm + """ + private_key = "ocE2RWd/yExYEk0JCAx3100//WQkmM3syidCVFsndS0=" + public_key = "odb+sBwaK0bZtaAqzcuFR3UVg5Wa1cW7ZMwJY1SnDng" + + # Register the user who gets notified + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # Register the user who sends the message + other_user_id = self.register_user("otheruser", "pass") + other_access_token = self.login("otheruser", "pass") + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastore().get_user_by_access_token(access_token) + ) + token_id = user_tuple.token_id + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={ + "url": "http://example.com/_matrix/push/v1/notify", + "algorithm": "com.famedly.curve25519-aes-sha2", + "public_key": public_key, + }, + ) + ) + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # The other user joins + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # The other user sends some messages + self.helper.send(room, body="Foxes are cute!", tok=other_access_token) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + + # Make the push succeed + self.push_attempts[0][0].callback({}) + self.pump() + + # One push was attempted to be sent -- it'll be the first message + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual( + self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify" + ) + self.assertEqual( + self.push_attempts[0][2]["notification"]["devices"][0]["data"]["algorithm"], + "com.famedly.curve25519-aes-sha2", + ) + ephemeral = unpaddedbase64.decode_base64( + self.push_attempts[0][2]["notification"]["ephemeral"] + ) + mac = unpaddedbase64.decode_base64( + self.push_attempts[0][2]["notification"]["mac"] + ) + ciphertext = unpaddedbase64.decode_base64( + self.push_attempts[0][2]["notification"]["ciphertext"] + ) + + # do the exchange + exchanged = PrivateKey.load( + unpaddedbase64.decode_base64(private_key) + ).do_exchange(PublicKey(ephemeral)) + # expand with HKDF + zerosalt = bytes([0] * 32) + prk = hmac.new(zerosalt, exchanged, hashlib.sha256).digest() + aes_key = hmac.new(prk, bytes([1]), hashlib.sha256).digest() + mac_key = hmac.new(prk, aes_key + bytes([2]), hashlib.sha256).digest() + aes_iv = hmac.new(prk, mac_key + bytes([3]), hashlib.sha256).digest()[0:16] + # create the cleartext with AES-CBC-256 + cleartext = json.loads( + unpad( + AES.new(aes_key, AES.MODE_CBC, aes_iv).decrypt(ciphertext), + AES.block_size, + ).decode("utf-8") + ) + # create the mac + calculated_mac = hmac.new(mac_key, ciphertext, hashlib.sha256).digest()[0:8] + + # test if we decrypted everything correctly + self.assertEqual(calculated_mac, mac) + self.assertEqual(cleartext["content"]["body"], "Foxes are cute!") + def test_sends_high_priority_for_encrypted(self): """ The HTTP pusher will send pushes at high priority if they correspond From d4833c230e1b86f8629e097d626e9935d9b82af0 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Fri, 10 Dec 2021 10:55:47 +0100 Subject: [PATCH 2/8] properly type-ignore donna25519 and fix an absent public_key --- mypy.ini | 3 +++ synapse/push/httppusher.py | 4 ++-- tests/push/test_http.py | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/mypy.ini b/mypy.ini index 1caf807e8505..5b2594272bcb 100644 --- a/mypy.ini +++ b/mypy.ini @@ -327,3 +327,6 @@ ignore_missing_imports = True [mypy-zope] ignore_missing_imports = True + +[mypy-donna25519] +ignore_missing_imports = True diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 8448f3312e6b..0e3a011f3519 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -24,7 +24,7 @@ import unpaddedbase64 from Crypto.Cipher import AES from Crypto.Util.Padding import pad -from donna25519 import PrivateKey, PublicKey # type: ignore +from donna25519 import PrivateKey, PublicKey from prometheus_client import Counter from twisted.internet.error import AlreadyCalled, AlreadyCancelled @@ -132,7 +132,7 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): self.algorithm = self.data["algorithm"] if self.algorithm == "com.famedly.curve25519-aes-sha2": - base64_public_key = self.data["public_key"] + base64_public_key = self.data.get("public_key") if not isinstance(base64_public_key, str): raise PusherConfigException("'public_key' must be a string") try: diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 03919e6c2177..b6588e31d649 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -19,7 +19,7 @@ import unpaddedbase64 from Crypto.Cipher import AES from Crypto.Util.Padding import unpad -from donna25519 import PrivateKey, PublicKey # type: ignore +from donna25519 import PrivateKey, PublicKey from twisted.internet.defer import Deferred From 251f304cc929ebe9afa0939ac33b5b2148daef53 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Tue, 14 Dec 2021 09:33:23 +0100 Subject: [PATCH 3/8] use cryptography in favour of donna25519 --- mypy.ini | 3 --- synapse/push/httppusher.py | 16 +++++++++++----- synapse/python_dependencies.py | 1 - tests/push/test_http.py | 9 ++++++--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/mypy.ini b/mypy.ini index 5b2594272bcb..1caf807e8505 100644 --- a/mypy.ini +++ b/mypy.ini @@ -327,6 +327,3 @@ ignore_missing_imports = True [mypy-zope] ignore_missing_imports = True - -[mypy-donna25519] -ignore_missing_imports = True diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 0e3a011f3519..61725febe1e5 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -24,7 +24,11 @@ import unpaddedbase64 from Crypto.Cipher import AES from Crypto.Util.Padding import pad -from donna25519 import PrivateKey, PublicKey +from cryptography.hazmat.primitives.asymmetric.x25519 import ( + X25519PrivateKey, + X25519PublicKey, +) +from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat from prometheus_client import Counter from twisted.internet.error import AlreadyCalled, AlreadyCancelled @@ -136,7 +140,7 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): if not isinstance(base64_public_key, str): raise PusherConfigException("'public_key' must be a string") try: - self.public_key = PublicKey( + self.public_key = X25519PublicKey.from_public_bytes( unpaddedbase64.decode_base64(base64_public_key) ) except Exception as e: @@ -164,9 +168,9 @@ def _process_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: cleartext = json.dumps(cleartext_notif) # create an ephemeral curve25519 keypair - private_key = PrivateKey() + private_key = X25519PrivateKey.generate() # do ECDH - secret_key = private_key.do_exchange(self.public_key) + secret_key = private_key.exchange(self.public_key) # expand with HKDF zerosalt = bytes([0] * 32) prk = hmac.new(zerosalt, secret_key, hashlib.sha256).digest() @@ -183,7 +187,9 @@ def _process_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: return { "notification": { "ephemeral": unpaddedbase64.encode_base64( - private_key.get_public().public + private_key.public_key().public_bytes( + Encoding.Raw, PublicFormat.Raw + ) ), "ciphertext": unpaddedbase64.encode_base64(ciphertext), "mac": unpaddedbase64.encode_base64(mac), diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 5738de7f561f..47b03b538b76 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -89,7 +89,6 @@ "ijson>=3.1", # Used for encrypted push "pycryptodome>=3", - "donna25519>=0.1.1", ] CONDITIONAL_REQUIREMENTS = { diff --git a/tests/push/test_http.py b/tests/push/test_http.py index b6588e31d649..9402d6731ee4 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -19,7 +19,10 @@ import unpaddedbase64 from Crypto.Cipher import AES from Crypto.Util.Padding import unpad -from donna25519 import PrivateKey, PublicKey +from cryptography.hazmat.primitives.asymmetric.x25519 import ( + X25519PrivateKey, + X25519PublicKey, +) from twisted.internet.defer import Deferred @@ -282,9 +285,9 @@ def test_sends_encrypted_push(self): ) # do the exchange - exchanged = PrivateKey.load( + exchanged = X25519PrivateKey.from_private_bytes( unpaddedbase64.decode_base64(private_key) - ).do_exchange(PublicKey(ephemeral)) + ).exchange(X25519PublicKey.from_public_bytes(ephemeral)) # expand with HKDF zerosalt = bytes([0] * 32) prk = hmac.new(zerosalt, exchanged, hashlib.sha256).digest() From bbf3baa3221deb868096d314b3a9e2f08e3caf95 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Tue, 14 Dec 2021 09:39:28 +0100 Subject: [PATCH 4/8] rename _process_notification_dict to _encrypt_notification_dict --- synapse/push/httppusher.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 61725febe1e5..44b91dad3e20 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -152,7 +152,7 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): ) del self.sanitized_data["public_key"] - def _process_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: + def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: """Called to process a payload according to the algorithm the pusher is configured with. Namely, if the algorithm is `com.famedly.curve25519-aes-sha2` we will encrypt the payload. @@ -428,7 +428,7 @@ async def _build_notification_dict( ], } } - return self._process_notification_dict(d) + return self._encrypt_notification_dict(d) ctx = await push_tools.get_context_for_event(self.storage, event, self.user_id) @@ -468,7 +468,7 @@ async def _build_notification_dict( if "name" in ctx and len(ctx["name"]) > 0: d["notification"]["room_name"] = ctx["name"] - return self._process_notification_dict(d) + return self._encrypt_notification_dict(d) async def dispatch_push( self, event: EventBase, tweaks: Dict[str, bool], badge: int @@ -500,7 +500,7 @@ async def _send_badge(self, badge: int) -> None: badge: number of unread messages """ logger.debug("Sending updated badge count %d to %s", badge, self.name) - d = self._process_notification_dict( + d = self._encrypt_notification_dict( { "notification": { "id": "", From e129b0167595943dfcb33a22e4203ff043588140 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Wed, 15 Dec 2021 14:03:02 +0100 Subject: [PATCH 5/8] remove dependency of pynacl --- synapse/push/httppusher.py | 14 ++++++++++---- synapse/python_dependencies.py | 2 -- tests/push/test_http.py | 13 ++++++++----- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 44b91dad3e20..6232d71604c7 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -22,12 +22,12 @@ from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Union import unpaddedbase64 -from Crypto.Cipher import AES -from Crypto.Util.Padding import pad +from cryptography.hazmat.primitives import padding from cryptography.hazmat.primitives.asymmetric.x25519 import ( X25519PrivateKey, X25519PublicKey, ) +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat from prometheus_client import Counter @@ -178,8 +178,14 @@ def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: mac_key = hmac.new(prk, aes_key + bytes([2]), hashlib.sha256).digest() aes_iv = hmac.new(prk, mac_key + bytes([3]), hashlib.sha256).digest()[0:16] # create the ciphertext with AES-CBC-256 - ciphertext = AES.new(aes_key, AES.MODE_CBC, aes_iv).encrypt( - pad(cleartext.encode("utf-8"), AES.block_size) + encryptor = Cipher(algorithms.AES(aes_key), modes.CBC(aes_iv)).encryptor() + # AES blocksize is always 128 bits + padder = padding.PKCS7(128).padder() + ciphertext = ( + encryptor.update( + padder.update(cleartext.encode("utf-8")) + padder.finalize() + ) + + encryptor.finalize() ) # create the mac mac = hmac.new(mac_key, ciphertext, hashlib.sha256).digest()[0:8] diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 47b03b538b76..7d26954244ea 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -87,8 +87,6 @@ # with the latest security patches. "cryptography>=3.4.7", "ijson>=3.1", - # Used for encrypted push - "pycryptodome>=3", ] CONDITIONAL_REQUIREMENTS = { diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 9402d6731ee4..86437a327104 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -17,12 +17,12 @@ from unittest.mock import Mock import unpaddedbase64 -from Crypto.Cipher import AES -from Crypto.Util.Padding import unpad +from cryptography.hazmat.primitives import padding from cryptography.hazmat.primitives.asymmetric.x25519 import ( X25519PrivateKey, X25519PublicKey, ) +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes from twisted.internet.defer import Deferred @@ -295,10 +295,13 @@ def test_sends_encrypted_push(self): mac_key = hmac.new(prk, aes_key + bytes([2]), hashlib.sha256).digest() aes_iv = hmac.new(prk, mac_key + bytes([3]), hashlib.sha256).digest()[0:16] # create the cleartext with AES-CBC-256 + decryptor = Cipher(algorithms.AES(aes_key), modes.CBC(aes_iv)).decryptor() + # AES blocksize is always 128 bits + unpadder = padding.PKCS7(128).unpadder() cleartext = json.loads( - unpad( - AES.new(aes_key, AES.MODE_CBC, aes_iv).decrypt(ciphertext), - AES.block_size, + ( + unpadder.update(decryptor.update(ciphertext) + decryptor.finalize()) + + unpadder.finalize() ).decode("utf-8") ) # create the mac From ba1d216acd5f1305df4648fde779d84e19b3639e Mon Sep 17 00:00:00 2001 From: Sorunome Date: Wed, 15 Dec 2021 14:18:34 +0100 Subject: [PATCH 6/8] Hide encrypted push behind a feature flag --- synapse/config/experimental.py | 3 ++ synapse/push/httppusher.py | 58 ++++++++++++++++++---------------- tests/push/test_http.py | 1 + 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d78a15097c87..d4c7b6acb3b9 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -47,5 +47,8 @@ def read_config(self, config: JsonDict, **kwargs): # MSC3266 (room summary api) self.msc3266_enabled: bool = experimental.get("msc3266_enabled", False) + # MSC3013 (encrypted push) + self.msc3013_enabled: bool = experimental.get("msc3013_enabled", False) + # MSC3030 (Jump to date API endpoint) self.msc3030_enabled: bool = experimental.get("msc3030_enabled", False) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 6232d71604c7..78e884132ca9 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -123,34 +123,35 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): self.sanitized_data.update(self.data) del self.sanitized_data["url"] - if "algorithm" not in self.data: - self.algorithm = "com.famedly.plain" - elif self.data["algorithm"] not in ( - "com.famedly.plain", - "com.famedly.curve25519-aes-sha2", - ): - raise PusherConfigException( - "'algorithm' must be one of 'com.famedly.plain' or 'com.famedly.curve25519-aes-sha2'" - ) - else: - self.algorithm = self.data["algorithm"] - - if self.algorithm == "com.famedly.curve25519-aes-sha2": - base64_public_key = self.data.get("public_key") - if not isinstance(base64_public_key, str): - raise PusherConfigException("'public_key' must be a string") - try: - self.public_key = X25519PublicKey.from_public_bytes( - unpaddedbase64.decode_base64(base64_public_key) - ) - except Exception as e: - logger.warning( - "Failed to unpack public key: %s: %s", type(e).__name__, e - ) + if self.hs.config.experimental.msc3013_enabled: + if "algorithm" not in self.data: + self.algorithm = "com.famedly.plain" + elif self.data["algorithm"] not in ( + "com.famedly.plain", + "com.famedly.curve25519-aes-sha2", + ): raise PusherConfigException( - "'public_key' must be a valid base64-encoded curve25519 public key" + "'algorithm' must be one of 'com.famedly.plain' or 'com.famedly.curve25519-aes-sha2'" ) - del self.sanitized_data["public_key"] + else: + self.algorithm = self.data["algorithm"] + + if self.algorithm == "com.famedly.curve25519-aes-sha2": + base64_public_key = self.data.get("public_key") + if not isinstance(base64_public_key, str): + raise PusherConfigException("'public_key' must be a string") + try: + self.public_key = X25519PublicKey.from_public_bytes( + unpaddedbase64.decode_base64(base64_public_key) + ) + except Exception as e: + logger.warning( + "Failed to unpack public key: %s: %s", type(e).__name__, e + ) + raise PusherConfigException( + "'public_key' must be a valid base64-encoded curve25519 public key" + ) + del self.sanitized_data["public_key"] def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: """Called to process a payload according to the algorithm the pusher is @@ -160,7 +161,10 @@ def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: Args: payload: The payload that should be processed """ - if self.algorithm == "com.famedly.curve25519-aes-sha2": + if ( + self.hs.config.experimental.msc3013_enabled + and self.algorithm == "com.famedly.curve25519-aes-sha2" + ): # we have an encrypted pusher, encrypt the payload cleartext_notif = payload["notification"] devices = cleartext_notif["devices"] diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 86437a327104..5377b27c2e8f 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -209,6 +209,7 @@ def test_sends_http(self): self.assertEqual(len(pushers), 1) self.assertTrue(pushers[0].last_stream_ordering > last_stream_ordering) + @override_config({"experimental_features": {"msc3013_enabled": True}}) def test_sends_encrypted_push(self): """ The HTTP pusher will send an encrypted push message if the pusher From 2f32b2862a884521ec7171ba248e0c20d603d170 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Wed, 15 Dec 2021 14:22:32 +0100 Subject: [PATCH 7/8] forgot to advertise encrypted push properly --- synapse/rest/client/versions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 6f44151d2028..c09a2b259760 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -93,8 +93,8 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]: "org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled, # Supports receiving hidden read receipts as per MSC2285 "org.matrix.msc2285": self.config.experimental.msc2285_enabled, - # Adds support for encrypted push - "com.famedly.msc3013": True, + # Adds support for encrypted push as per MSC3013 + "com.famedly.msc3013": self.config.experimental.msc3013_enabled, }, }, ) From d38ed7343793c5be3227c9eefc744a313314d443 Mon Sep 17 00:00:00 2001 From: Sorunome Date: Mon, 27 Dec 2021 11:31:09 +0100 Subject: [PATCH 8/8] add handeling of counts_only_type pusher data --- synapse/push/httppusher.py | 47 ++++++++---- tests/push/test_http.py | 142 ++++++++++++++++++++++++++++++------- 2 files changed, 153 insertions(+), 36 deletions(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 78e884132ca9..70fc93bab5f1 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -151,9 +151,22 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): raise PusherConfigException( "'public_key' must be a valid base64-encoded curve25519 public key" ) + + if "counts_only_type" not in self.data: + self.counts_only_type = "none" + elif self.data["counts_only_type"] not in ("none", "boolean", "full"): + raise PusherConfigException( + "'counts_only_type' must be one of 'none', 'boolean' or 'full'" + ) + else: + self.counts_only_type = self.data["counts_only_type"] + del self.sanitized_data["counts_only_type"] + del self.sanitized_data["public_key"] - def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: + def _encrypt_notification_dict( + self, payload: Dict[str, Any], counts_only: bool = False + ) -> Dict[str, Any]: """Called to process a payload according to the algorithm the pusher is configured with. Namely, if the algorithm is `com.famedly.curve25519-aes-sha2` we will encrypt the payload. @@ -194,17 +207,26 @@ def _encrypt_notification_dict(self, payload: Dict[str, Any]) -> Dict[str, Any]: # create the mac mac = hmac.new(mac_key, ciphertext, hashlib.sha256).digest()[0:8] + d = { + "ephemeral": unpaddedbase64.encode_base64( + private_key.public_key().public_bytes( + Encoding.Raw, PublicFormat.Raw + ) + ), + "ciphertext": unpaddedbase64.encode_base64(ciphertext), + "mac": unpaddedbase64.encode_base64(mac), + "devices": devices, + } + + # now, if the push frame is a counts only frame, we have to respect the counts_only_type setting + if counts_only: + if self.counts_only_type == "boolean": + d["is_counts_only"] = True + elif self.counts_only_type == "full": + d["counts"] = cleartext_notif["counts"] + return { - "notification": { - "ephemeral": unpaddedbase64.encode_base64( - private_key.public_key().public_bytes( - Encoding.Raw, PublicFormat.Raw - ) - ), - "ciphertext": unpaddedbase64.encode_base64(ciphertext), - "mac": unpaddedbase64.encode_base64(mac), - "devices": devices, - } + "notification": d, } # else fall back to just plaintext @@ -526,7 +548,8 @@ async def _send_badge(self, badge: int) -> None: } ], } - } + }, + counts_only=True, ) try: await self.http_client.post_json_get_json(self.url, d) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 5377b27c2e8f..ebabfaa193f3 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -312,6 +312,94 @@ def test_sends_encrypted_push(self): self.assertEqual(calculated_mac, mac) self.assertEqual(cleartext["content"]["body"], "Foxes are cute!") + @override_config({"experimental_features": {"msc3013_enabled": True}}) + def tests_encrypted_push_counts_only_none(self): + """ + The HTTP pusher will not add any extra fields to encrypted push frames if the counts_only_type + is none. + """ + # Set up the counts-only push + self._test_push_unread_count( + pusher_data={ + "url": "http://example.com/_matrix/push/v1/notify", + "algorithm": "com.famedly.curve25519-aes-sha2", + "public_key": "odb+sBwaK0bZtaAqzcuFR3UVg5Wa1cW7ZMwJY1SnDng", + }, + perform_checks=False, + ) + + # counts-only push frame + self.assertEqual("counts" in self.push_attempts[1][2]["notification"], False) + self.assertEqual( + "is_counts_only" in self.push_attempts[1][2]["notification"], False + ) + + # normal push frame + self.assertEqual("counts" in self.push_attempts[5][2]["notification"], False) + self.assertEqual( + "is_counts_only" in self.push_attempts[5][2]["notification"], False + ) + + @override_config({"experimental_features": {"msc3013_enabled": True}}) + def tests_encrypted_push_counts_only_boolean(self): + """ + The HTTP pusher will add the is_counts_only field to encrypted push frames if the counts_only_type + is boolean. + """ + # Set up the counts-only push + self._test_push_unread_count( + pusher_data={ + "url": "http://example.com/_matrix/push/v1/notify", + "algorithm": "com.famedly.curve25519-aes-sha2", + "public_key": "odb+sBwaK0bZtaAqzcuFR3UVg5Wa1cW7ZMwJY1SnDng", + "counts_only_type": "boolean", + }, + perform_checks=False, + ) + + # counts-only push frame + self.assertEqual("counts" in self.push_attempts[1][2]["notification"], False) + self.assertEqual( + self.push_attempts[1][2]["notification"]["is_counts_only"], True + ) + + # normal push frame + self.assertEqual("counts" in self.push_attempts[5][2]["notification"], False) + self.assertEqual( + "is_counts_only" in self.push_attempts[5][2]["notification"], False + ) + + @override_config({"experimental_features": {"msc3013_enabled": True}}) + def tests_encrypted_push_counts_only_full(self): + """ + The HTTP pusher will add the counts dict to encrypted push frames if the counts_only_type + is full. + """ + # Set up the counts-only push + self._test_push_unread_count( + pusher_data={ + "url": "http://example.com/_matrix/push/v1/notify", + "algorithm": "com.famedly.curve25519-aes-sha2", + "public_key": "odb+sBwaK0bZtaAqzcuFR3UVg5Wa1cW7ZMwJY1SnDng", + "counts_only_type": "full", + }, + perform_checks=False, + ) + + # counts-only push frame + self.assertEqual( + self.push_attempts[1][2]["notification"]["counts"]["unread"], 0 + ) + self.assertEqual( + "is_counts_only" in self.push_attempts[1][2]["notification"], False + ) + + # normal push frame + self.assertEqual("counts" in self.push_attempts[5][2]["notification"], False) + self.assertEqual( + "is_counts_only" in self.push_attempts[5][2]["notification"], False + ) + def test_sends_high_priority_for_encrypted(self): """ The HTTP pusher will send pushes at high priority if they correspond @@ -705,7 +793,7 @@ def test_push_unread_count_message_count(self): self.push_attempts[5][2]["notification"]["counts"]["unread"], 4 ) - def _test_push_unread_count(self): + def _test_push_unread_count(self, pusher_data=None, perform_checks=True): """ Tests that the correct unread count appears in sent push notifications @@ -734,6 +822,9 @@ def _test_push_unread_count(self): ) token_id = user_tuple.token_id + if pusher_data is None: + pusher_data = {"url": "http://example.com/_matrix/push/v1/notify"} + self.get_success( self.hs.get_pusherpool().add_pusher( user_id=user_id, @@ -744,7 +835,7 @@ def _test_push_unread_count(self): device_display_name="pushy push", pushkey="a@example.com", lang=None, - data={"url": "http://example.com/_matrix/push/v1/notify"}, + data=pusher_data, ) ) @@ -761,18 +852,19 @@ def _test_push_unread_count(self): self.push_attempts[0][0].callback({}) self.pump() - # Check our push made it - self.assertEqual(len(self.push_attempts), 1) - self.assertEqual( - self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify" - ) + if perform_checks: + # Check our push made it + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual( + self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify" + ) - # Check that the unread count for the room is 0 - # - # The unread count is zero as the user has no read receipt in the room yet - self.assertEqual( - self.push_attempts[0][2]["notification"]["counts"]["unread"], 0 - ) + # Check that the unread count for the room is 0 + # + # The unread count is zero as the user has no read receipt in the room yet + self.assertEqual( + self.push_attempts[0][2]["notification"]["counts"]["unread"], 0 + ) # Now set the user's read receipt position to the first event # @@ -791,11 +883,12 @@ def _test_push_unread_count(self): self.push_attempts[1][0].callback({}) self.pump() - # Unread count is still zero as we've read the only message in the room - self.assertEqual(len(self.push_attempts), 2) - self.assertEqual( - self.push_attempts[1][2]["notification"]["counts"]["unread"], 0 - ) + if perform_checks: + # Unread count is still zero as we've read the only message in the room + self.assertEqual(len(self.push_attempts), 2) + self.assertEqual( + self.push_attempts[1][2]["notification"]["counts"]["unread"], 0 + ) # Send another message self.helper.send( @@ -806,12 +899,13 @@ def _test_push_unread_count(self): self.push_attempts[2][0].callback({}) self.pump() - # This push should contain an unread count of 1 as there's now been one - # message since our last read receipt - self.assertEqual(len(self.push_attempts), 3) - self.assertEqual( - self.push_attempts[2][2]["notification"]["counts"]["unread"], 1 - ) + if perform_checks: + # This push should contain an unread count of 1 as there's now been one + # message since our last read receipt + self.assertEqual(len(self.push_attempts), 3) + self.assertEqual( + self.push_attempts[2][2]["notification"]["counts"]["unread"], 1 + ) # Since we're grouping by room, sending more messages shouldn't increase the # unread count, as they're all being sent in the same room