Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport : Implementation of MSC3967: Don't require UIA for initial u… #3

Merged
merged 3 commits into from
Mar 13, 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.
4 changes: 4 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:

# MSC3786 (Add a default push rule to ignore m.room.server_acl events)
self.msc3786_enabled: bool = experimental.get("msc3786_enabled", False)

# MSC3967 is backported by Tchap :
# 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 @@ -1214,6 +1214,20 @@ async def _retrieve_cross_signing_keys_for_remote_user(

return desired_key, desired_key_id, desired_verify_key

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 @@ -277,15 +277,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: 140 additions & 1 deletion tests/rest/client/test_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@

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


class KeyQueryTestCase(unittest.HomeserverTestCase):
servlets = [
keys.register_servlets,
Expand Down Expand Up @@ -89,3 +96,135 @@ def test_requires_device_key(self) -> None:
Codes.BAD_JSON,
channel.result,
)

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)