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

This now matches the logic of the registration process as modified in
56db0b1365 / #7523.
This commit is contained in:
Patrick Cloke 2020-05-20 09:48:03 -04:00 committed by GitHub
parent 4fa74c7606
commit 9dc6f3075a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 33 additions and 11 deletions

1
changelog.d/7538.bugfix Normal file
View file

@ -0,0 +1 @@
Hash passwords as early as possible during password reset.

View file

@ -35,16 +35,13 @@ class SetPasswordHandler(BaseHandler):
async def set_password( async def set_password(
self, self,
user_id: str, user_id: str,
new_password: str, password_hash: str,
logout_devices: bool, logout_devices: bool,
requester: Optional[Requester] = None, requester: Optional[Requester] = None,
): ):
if not self.hs.config.password_localdb_enabled: if not self.hs.config.password_localdb_enabled:
raise SynapseError(403, "Password change disabled", errcode=Codes.FORBIDDEN) 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: try:
await self.store.user_set_password_hash(user_id, password_hash) await self.store.user_set_password_hash(user_id, password_hash)
except StoreError as e: except StoreError as e:

View file

@ -222,8 +222,14 @@ class UserRestServletV2(RestServlet):
else: else:
new_password = body["password"] new_password = body["password"]
logout_devices = True logout_devices = True
new_password_hash = await self.auth_handler.hash(new_password)
await self.set_password_handler.set_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: if "deactivated" in body:
@ -523,6 +529,7 @@ class ResetPasswordRestServlet(RestServlet):
self.store = hs.get_datastore() self.store = hs.get_datastore()
self.hs = hs self.hs = hs
self.auth = hs.get_auth() self.auth = hs.get_auth()
self.auth_handler = hs.get_auth_handler()
self._set_password_handler = hs.get_set_password_handler() self._set_password_handler = hs.get_set_password_handler()
async def on_POST(self, request, target_user_id): async def on_POST(self, request, target_user_id):
@ -539,8 +546,10 @@ class ResetPasswordRestServlet(RestServlet):
new_password = params["new_password"] new_password = params["new_password"]
logout_devices = params.get("logout_devices", True) logout_devices = params.get("logout_devices", True)
new_password_hash = await self.auth_handler.hash(new_password)
await self._set_password_handler.set_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, {} return 200, {}

View file

@ -220,12 +220,27 @@ class PasswordRestServlet(RestServlet):
self.auth = hs.get_auth() self.auth = hs.get_auth()
self.auth_handler = hs.get_auth_handler() self.auth_handler = hs.get_auth_handler()
self.datastore = self.hs.get_datastore() self.datastore = self.hs.get_datastore()
self.password_policy_handler = hs.get_password_policy_handler()
self._set_password_handler = hs.get_set_password_handler() self._set_password_handler = hs.get_set_password_handler()
@interactive_auth_handler @interactive_auth_handler
async def on_POST(self, request): async def on_POST(self, request):
body = parse_json_object_from_request(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 # 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 # access token, and needs to do a password reset; or they have one and
# need to validate their identity. # need to validate their identity.
@ -276,12 +291,12 @@ class PasswordRestServlet(RestServlet):
logger.error("Auth succeeded but no known type! %r", result.keys()) logger.error("Auth succeeded but no known type! %r", result.keys())
raise SynapseError(500, "", Codes.UNKNOWN) raise SynapseError(500, "", Codes.UNKNOWN)
assert_params_in_dict(params, ["new_password"]) assert_params_in_dict(params, ["new_password_hash"])
new_password = params["new_password"] new_password_hash = params["new_password_hash"]
logout_devices = params.get("logout_devices", True) logout_devices = params.get("logout_devices", True)
await self._set_password_handler.set_password( await self._set_password_handler.set_password(
user_id, new_password, logout_devices, requester user_id, new_password_hash, logout_devices, requester
) )
return 200, {} return 200, {}

View file

@ -431,8 +431,8 @@ class RegisterRestServlet(RestServlet):
raise SynapseError(400, "Invalid password") raise SynapseError(400, "Invalid password")
self.password_policy_handler.validate_password(password) self.password_policy_handler.validate_password(password)
# If the password is valid, hash it and store it back on the request. # If the password is valid, hash it and store it back on the body.
# This ensures the hashed password is handled everywhere. # This ensures that only the hashed password is handled everywhere.
if "password_hash" in body: if "password_hash" in body:
raise SynapseError(400, "Unexpected property: password_hash") raise SynapseError(400, "Unexpected property: password_hash")
body["password_hash"] = await self.auth_handler.hash(password) body["password_hash"] = await self.auth_handler.hash(password)