Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Implementation of MSC3967: Don't require UIA for initial upload of cross signing keys #15077

Merged
merged 8 commits into from
Mar 2, 2023
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
1 change: 1 addition & 0 deletions changelog.d/15077.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Experimental support for MSC3967 to not require UIA for setting up cross-signing on first use.
3 changes: 3 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.msc3966_exact_event_property_contains = experimental.get(
"msc3966_exact_event_property_contains", False
)

# MSC3967: Do not require UIA when first uploading cross signing keys
self.msc3967_enabled = experimental.get("msc3967_enabled", False)
14 changes: 14 additions & 0 deletions synapse/handlers/e2e_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,20 @@ async def _retrieve_cross_signing_keys_for_remote_user(

return desired_key_data

async def is_cross_signing_set_up_for_user(self, user_id: str) -> bool:
"""Checks if the user has cross-signing set up

Args:
user_id: The user to check

Returns:
True if the user has cross-signing set up, False otherwise
"""
existing_master_key = await self.store.get_e2e_cross_signing_key(
user_id, "master"
)
return existing_master_key is not None


def _check_cross_signing_key(
key: JsonDict, user_id: str, key_type: str, signing_key: Optional[VerifyKey] = None
Expand Down
32 changes: 23 additions & 9 deletions synapse/rest/client/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,29 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
user_id = requester.user.to_string()
body = parse_json_object_from_request(request)

await self.auth_handler.validate_user_via_ui_auth(
requester,
request,
body,
"add a device signing key to your account",
# Allow skipping of UI auth since this is frequently called directly
# after login and it is silly to ask users to re-auth immediately.
can_skip_ui_auth=True,
)
if self.hs.config.experimental.msc3967_enabled:
if await self.e2e_keys_handler.is_cross_signing_set_up_for_user(user_id):
# If we already have a master key then cross signing is set up and we require UIA to reset
await self.auth_handler.validate_user_via_ui_auth(
requester,
request,
body,
"reset the device signing key on your account",
# Do not allow skipping of UIA auth.
can_skip_ui_auth=False,
)
# Otherwise we don't require UIA since we are setting up cross signing for first time
else:
# Previous behaviour is to always require UIA but allow it to be skipped
await self.auth_handler.validate_user_via_ui_auth(
requester,
request,
body,
"add a device signing key to your account",
# Allow skipping of UI auth since this is frequently called directly
# after login and it is silly to ask users to re-auth immediately.
can_skip_ui_auth=True,
)

result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body)
return 200, result
Expand Down
141 changes: 141 additions & 0 deletions tests/rest/client/test_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,21 @@

from http import HTTPStatus

from signedjson.key import (
encode_verify_key_base64,
generate_signing_key,
get_verify_key,
)
from signedjson.sign import sign_json

from synapse.api.errors import Codes
from synapse.rest import admin
from synapse.rest.client import keys, login
from synapse.types import JsonDict

from tests import unittest
from tests.http.server._base import make_request_with_cancellation_test
from tests.unittest import override_config


class KeyQueryTestCase(unittest.HomeserverTestCase):
Expand Down Expand Up @@ -118,3 +127,135 @@ def test_key_query_cancellation(self) -> None:

self.assertEqual(200, channel.code, msg=channel.result["body"])
self.assertIn(bob, channel.json_body["device_keys"])

def make_device_keys(self, user_id: str, device_id: str) -> JsonDict:
# We only generate a master key to simplify the test.
master_signing_key = generate_signing_key(device_id)
master_verify_key = encode_verify_key_base64(get_verify_key(master_signing_key))

return {
"master_key": sign_json(
{
"user_id": user_id,
"usage": ["master"],
"keys": {"ed25519:" + master_verify_key: master_verify_key},
},
user_id,
master_signing_key,
),
}

def test_device_signing_with_uia(self) -> None:
"""Device signing key upload requires UIA."""
password = "wonderland"
device_id = "ABCDEFGHI"
alice_id = self.register_user("alice", password)
alice_token = self.login("alice", password, device_id=device_id)

content = self.make_device_keys(alice_id, device_id)

channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
content,
alice_token,
)

self.assertEqual(channel.code, HTTPStatus.UNAUTHORIZED, channel.result)
# Grab the session
session = channel.json_body["session"]
# Ensure that flows are what is expected.
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])

# add UI auth
content["auth"] = {
"type": "m.login.password",
"identifier": {"type": "m.id.user", "user": alice_id},
"password": password,
"session": session,
}

channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
content,
alice_token,
)

self.assertEqual(channel.code, HTTPStatus.OK, channel.result)

@override_config({"ui_auth": {"session_timeout": "15m"}})
def test_device_signing_with_uia_session_timeout(self) -> None:
"""Device signing key upload requires UIA buy passes with grace period."""
password = "wonderland"
device_id = "ABCDEFGHI"
alice_id = self.register_user("alice", password)
alice_token = self.login("alice", password, device_id=device_id)

content = self.make_device_keys(alice_id, device_id)

channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
content,
alice_token,
)

self.assertEqual(channel.code, HTTPStatus.OK, channel.result)

@override_config(
{
"experimental_features": {"msc3967_enabled": True},
"ui_auth": {"session_timeout": "15s"},
}
)
def test_device_signing_with_msc3967(self) -> None:
"""Device signing key follows MSC3967 behaviour when enabled."""
password = "wonderland"
device_id = "ABCDEFGHI"
alice_id = self.register_user("alice", password)
alice_token = self.login("alice", password, device_id=device_id)

keys1 = self.make_device_keys(alice_id, device_id)

# Initial request should succeed as no existing keys are present.
channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
keys1,
alice_token,
)
self.assertEqual(channel.code, HTTPStatus.OK, channel.result)

keys2 = self.make_device_keys(alice_id, device_id)

# Subsequent request should require UIA as keys already exist even though session_timeout is set.
channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
keys2,
alice_token,
)
self.assertEqual(channel.code, HTTPStatus.UNAUTHORIZED, channel.result)

# Grab the session
session = channel.json_body["session"]
# Ensure that flows are what is expected.
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])

# add UI auth
keys2["auth"] = {
"type": "m.login.password",
"identifier": {"type": "m.id.user", "user": alice_id},
"password": password,
"session": session,
}

# Request should complete
channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
keys2,
alice_token,
)
self.assertEqual(channel.code, HTTPStatus.OK, channel.result)