Hash passwords earlier in the password reset process (#7538)

This now matches the logic of the registration process as modified in
56db0b1365965c02ff539193e26c333b7f70d101 / #7523.

5 files changed, 33 insertions, 11 deletions

diff --git a/changelog.d/7538.bugfix b/changelog.d/7538.bugfix
new file mode 100644
index 0000000000..4a614a9e61
--- /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 63d8f9aa0d..4d245b618b 100644
--- a/synapse/handlers/set_password.py
+++ b/synapse/handlers/set_password.py
@@ -35,16 +35,13 @@ class SetPasswordHandler(BaseHandler):
     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 326682fbdb..e7f6928c85 100644
--- a/synapse/rest/admin/users.py
+++ b/synapse/rest/admin/users.py
@@ -222,8 +222,14 @@ class UserRestServletV2(RestServlet):
                 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 @@ class ResetPasswordRestServlet(RestServlet):
         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 @@ class ResetPasswordRestServlet(RestServlet):
         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 1bd0234779..d4f721b6b9 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -220,12 +220,27 @@ class PasswordRestServlet(RestServlet):
         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 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:
+                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 @@ class PasswordRestServlet(RestServlet):
                 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 c26927f27b..addd4cae19 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -431,8 +431,8 @@ class RegisterRestServlet(RestServlet):
                 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)