-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add an Admin API to temporarily grant the ability to update an existing cross-signing key without UIA #16634
Changes from 6 commits
490f4de
29b00d2
80eea39
b9f0daf
cdaecc0
7451f75
e3f333a
89dd3b1
5c0e682
f5ce34e
b597f87
03fc94f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add an internal [Admin API endpoint](https://matrix-org.github.io/synapse/v1.97/usage/configuration/config_documentation.html#allow-replacing-master-cross-signing-key-without-user-interactive-auth) to temporarily grant the ability to update an existing cross-signing key without UIA. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1450,19 +1450,25 @@ 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: | ||
async def check_cross_signing_setup(self, user_id: str) -> Tuple[bool, 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 | ||
Returns: a 2-tuple of booleans | ||
- whether the user has cross-signing set up, and | ||
- whether the user's master cross-signing key may be replaced without UIA. | ||
Comment on lines
+1459
to
+1461
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit naughty, since there aren't really 4 states here. But I think this makes things clearer at the call site. |
||
""" | ||
existing_master_key = await self.store.get_e2e_cross_signing_key( | ||
user_id, "master" | ||
) | ||
return existing_master_key is not None | ||
( | ||
exists, | ||
ts_replacable_without_uia_before, | ||
) = await self.store.get_master_cross_signing_key_updatable_before(user_id) | ||
|
||
if ts_replacable_without_uia_before is None: | ||
return exists, False | ||
else: | ||
return exists, self.clock.time_msec() < ts_replacable_without_uia_before | ||
|
||
|
||
def _check_cross_signing_key( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1270,6 +1270,46 @@ async def on_GET( | |
} | ||
|
||
|
||
class UserReplaceMasterCrossSigningKeyRestServlet(RestServlet): | ||
"""Allow a given user to replace their master cross-signing key without UIA. | ||
|
||
This replacement is permitted for a limited period (currently 10 minutes). | ||
|
||
While this is exposed via the admin API, this is intended for use by the | ||
Matrix Authentication Service rather than server admins. | ||
""" | ||
|
||
PATTERNS = admin_patterns( | ||
"/users/(?P<user_id>[^/]*)/_allow_cross_signing_replacement_without_uia" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sandhose and I wondered if this should come with warnings e.g. "This is an internal endpoint, we reserve the right to break this". That's what the |
||
) | ||
REPLACEMENT_PERIOD_MS = 10 * 60 * 1000 # 10 minutes | ||
|
||
def __init__(self, hs: "HomeServer"): | ||
self._auth = hs.get_auth() | ||
self._store = hs.get_datastores().main | ||
|
||
async def on_POST( | ||
self, | ||
request: SynapseRequest, | ||
user_id: str, | ||
) -> Tuple[int, JsonDict]: | ||
await assert_requester_is_admin(self._auth, request) | ||
|
||
if user_id is None: | ||
raise NotFoundError("User not found") | ||
|
||
timestamp = ( | ||
await self._store.allow_master_cross_signing_key_replacement_without_uia( | ||
user_id, self.REPLACEMENT_PERIOD_MS | ||
) | ||
) | ||
|
||
if timestamp is None: | ||
raise NotFoundError("User has no master cross-signing key") | ||
|
||
return HTTPStatus.OK, {"updatable_without_uia_before_ms": timestamp} | ||
|
||
|
||
class UserByExternalId(RestServlet): | ||
"""Find a user based on an external ID from an auth provider""" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/* Copyright 2023 The Matrix.org Foundation C.I.C | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* 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. | ||
*/ | ||
ALTER TABLE e2e_cross_signing_keys ADD COLUMN updatable_without_uia_before_ms bigint DEFAULT NULL; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did think about adding a check constraint to enforce that only master keys may have this column set to a non-null value. But then I'd have to think about adding a background update to validate the check in the background; it didn't seem worth it.
sandhose marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if I should omit this. But I thought it might be best to err on the side of transparency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to keep this documented, it's clear enough that as an admin you shouldn't use that