From 98772d741b4c65fd5f6b5ee9b583bbf1367146cf Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 20 May 2020 08:18:54 -0400 Subject: [PATCH 1/2] Store hash passwords during user interactive authentication. --- changelog.d/7538.bugfix | 1 + synapse/handlers/set_password.py | 5 +---- synapse/rest/admin/users.py | 13 +++++++++++-- synapse/rest/client/v2_alpha/account.py | 21 ++++++++++++++++++--- synapse/rest/client/v2_alpha/register.py | 4 ++-- 5 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 changelog.d/7538.bugfix diff --git a/changelog.d/7538.bugfix b/changelog.d/7538.bugfix new file mode 100644 index 000000000000..4a614a9e61cc --- /dev/null +++ b/changelog.d/7538.bugfix @@ -0,0 +1 @@ + Hash passwords as early as possible during password reset. diff --git a/synapse/handlers/set_password.py b/synapse/handlers/set_password.py index 63d8f9aa0d54..4d245b618b17 100644 --- a/synapse/handlers/set_password.py +++ b/synapse/handlers/set_password.py @@ -35,16 +35,13 @@ def __init__(self, hs): async def set_password( self, user_id: str, - new_password: str, + password_hash: str, logout_devices: bool, requester: Optional[Requester] = None, ): if not self.hs.config.password_localdb_enabled: raise SynapseError(403, "Password change disabled", errcode=Codes.FORBIDDEN) - self._password_policy_handler.validate_password(new_password) - password_hash = await self._auth_handler.hash(new_password) - try: await self.store.user_set_password_hash(user_id, password_hash) except StoreError as e: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 326682fbdb67..e7f6928c859d 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -222,8 +222,14 @@ async def on_PUT(self, request, user_id): else: new_password = body["password"] logout_devices = True + + new_password_hash = await self.auth_handler.hash(new_password) + await self.set_password_handler.set_password( - target_user.to_string(), new_password, logout_devices, requester + target_user.to_string(), + new_password_hash, + logout_devices, + requester, ) if "deactivated" in body: @@ -523,6 +529,7 @@ def __init__(self, hs): self.store = hs.get_datastore() self.hs = hs self.auth = hs.get_auth() + self.auth_handler = hs.get_auth_handler() self._set_password_handler = hs.get_set_password_handler() async def on_POST(self, request, target_user_id): @@ -539,8 +546,10 @@ async def on_POST(self, request, target_user_id): new_password = params["new_password"] logout_devices = params.get("logout_devices", True) + new_password_hash = await self.auth_handler.hash(new_password) + await self._set_password_handler.set_password( - target_user_id, new_password, logout_devices, requester + target_user_id, new_password_hash, logout_devices, requester ) return 200, {} diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 1bd023477902..0e5bc48a744f 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -220,12 +220,27 @@ def __init__(self, hs): self.auth = hs.get_auth() self.auth_handler = hs.get_auth_handler() self.datastore = self.hs.get_datastore() + self.password_policy_handler = hs.get_password_policy_handler() self._set_password_handler = hs.get_set_password_handler() @interactive_auth_handler async def on_POST(self, request): body = parse_json_object_from_request(request) + # we do basic sanity checks here because the auth layer will store these + # in sessions. Pull out the username/password provided to us. + if "new_password" in body: + new_password = body.pop("new_password") + if not isinstance(new_password, str) or len(new_password) > 512: + raise SynapseError(400, "Invalid password") + self.password_policy_handler.validate_password(new_password) + + # If the password is valid, hash it and store it back on the body. + # This ensures that only the hashed password is handled everywhere. + if "new_password_hash" in body: + raise SynapseError(400, "Unexpected property: new_password_hash") + body["new_password_hash"] = await self.auth_handler.hash(new_password) + # there are two possibilities here. Either the user does not have an # access token, and needs to do a password reset; or they have one and # need to validate their identity. @@ -276,12 +291,12 @@ async def on_POST(self, request): logger.error("Auth succeeded but no known type! %r", result.keys()) raise SynapseError(500, "", Codes.UNKNOWN) - assert_params_in_dict(params, ["new_password"]) - new_password = params["new_password"] + assert_params_in_dict(params, ["new_password_hash"]) + new_password_hash = params["new_password_hash"] logout_devices = params.get("logout_devices", True) await self._set_password_handler.set_password( - user_id, new_password, logout_devices, requester + user_id, new_password_hash, logout_devices, requester ) return 200, {} diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index c26927f27b9e..addd4cae1906 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -431,8 +431,8 @@ async def on_POST(self, request): raise SynapseError(400, "Invalid password") self.password_policy_handler.validate_password(password) - # If the password is valid, hash it and store it back on the request. - # This ensures the hashed password is handled everywhere. + # If the password is valid, hash it and store it back on the body. + # This ensures that only the hashed password is handled everywhere. if "password_hash" in body: raise SynapseError(400, "Unexpected property: password_hash") body["password_hash"] = await self.auth_handler.hash(password) From b952a0dac96ae0813224fc2091e2ff8f96c0ae39 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 20 May 2020 09:17:56 -0400 Subject: [PATCH 2/2] Clarify comment. --- synapse/rest/client/v2_alpha/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 0e5bc48a744f..d4f721b6b989 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -228,7 +228,7 @@ async def on_POST(self, request): body = parse_json_object_from_request(request) # we do basic sanity checks here because the auth layer will store these - # in sessions. Pull out the username/password provided to us. + # in sessions. Pull out the new password provided to us. if "new_password" in body: new_password = body.pop("new_password") if not isinstance(new_password, str) or len(new_password) > 512: