In order to avoid "unpermitted parameters" errors, the password parameter is permitted by default for the sign_in action (see https://github.com/heartcombo/devise/pull/2440 and https://github.com/heartcombo/devise/pull/2452). Now, Devise::SessionsController#new creates a new resource with the permitted parameters before cleaning up the password. This means that an invalid password is hashed twice: The first time - as it should be - when it is validated with Devise::Encryptor.compare (invoked by resource.valid_password?), and then a second time with Devise::Encryptor.digest (invoked by Devise::SessionsController#new). Something similar happens if the authentication key is invalid and Devise.paranoid is true.
I think this is problematic as the number of stretches is always a compromise between password security on the one hand and performance and the danger of generating DoS opportunities on the other hand. Fixing this would probably allow some applications to increase the number of stretches.
My solution is to simply remove the password from the return value of sign_in_params in Devise::SessionsController#new. I think the risk that this breaks something in the application is lower than the risk of changing sign_in_params itself.
In order to avoid "unpermitted parameters" errors, the password parameter is permitted by default for the
sign_inaction (see https://github.com/heartcombo/devise/pull/2440 and https://github.com/heartcombo/devise/pull/2452). Now,Devise::SessionsController#newcreates a new resource with the permitted parameters before cleaning up the password. This means that an invalid password is hashed twice: The first time - as it should be - when it is validated withDevise::Encryptor.compare(invoked byresource.valid_password?), and then a second time withDevise::Encryptor.digest(invoked byDevise::SessionsController#new). Something similar happens if the authentication key is invalid andDevise.paranoidis true.I think this is problematic as the number of stretches is always a compromise between password security on the one hand and performance and the danger of generating DoS opportunities on the other hand. Fixing this would probably allow some applications to increase the number of stretches.
My solution is to simply remove the password from the return value of
sign_in_paramsinDevise::SessionsController#new. I think the risk that this breaks something in the application is lower than the risk of changingsign_in_paramsitself.