From 6162060ea404a8646a161fda99b24a8f6ca9e81b Mon Sep 17 00:00:00 2001 From: Thibault Duplessis Date: Wed, 27 Sep 2017 18:40:44 -0500 Subject: [PATCH] back to synchronous, unthrottled password hashing --- modules/user/src/main/DataForm.scala | 7 +-- modules/user/src/main/Env.scala | 16 ++----- modules/user/src/main/PasswordHasher.scala | 14 ------ modules/user/src/main/User.scala | 20 ++++---- modules/user/src/main/UserRepo.scala | 54 +++++++++------------- 5 files changed, 39 insertions(+), 72 deletions(-) diff --git a/modules/user/src/main/DataForm.scala b/modules/user/src/main/DataForm.scala index 5055ac064c..5ce2870930 100644 --- a/modules/user/src/main/DataForm.scala +++ b/modules/user/src/main/DataForm.scala @@ -2,7 +2,6 @@ package lila.user import play.api.data._ import play.api.data.Forms._ -import scala.concurrent.duration._ object DataForm { @@ -39,11 +38,7 @@ object DataForm { def passwd(u: User) = UserRepo loginCandidate u map { candidate => Form(mapping( - "oldPasswd" -> nonEmptyText.verifying("incorrectPassword", { pass => - // ugly shit, but Play forms are synchronous. - // make sure the form is ratelimited by user upstream - candidate.check(pass) await 2.seconds - }), + "oldPasswd" -> nonEmptyText.verifying("incorrectPassword", candidate.check), "newPasswd1" -> nonEmptyText(minLength = 2), "newPasswd2" -> nonEmptyText(minLength = 2) )(Passwd.apply)(Passwd.unapply).verifying("the new passwords don't match", _.samePasswords)) diff --git a/modules/user/src/main/Env.scala b/modules/user/src/main/Env.scala index 708e56a9ef..db239e42a0 100644 --- a/modules/user/src/main/Env.scala +++ b/modules/user/src/main/Env.scala @@ -91,18 +91,12 @@ final class Env( ) lazy val passwordAuth = new Authenticator( - new AsyncPasswordHasher( - new PasswordHasher( - secret = PasswordBPassSecret, - logRounds = 10, - hashTimer = lila.mon.measure(_.user.auth.hashTime) - ), - new lila.hub.SyncMultiSequencer( - system = system, - parallelismFactor = PasswordParallelism - ) + new PasswordHasher( + secret = PasswordBPassSecret, + logRounds = 10, + hashTimer = lila.mon.measure(_.user.auth.hashTime) ), - lila.mon.user.auth.shaLogin() + onShaLogin = lila.mon.user.auth.shaLogin ) lazy val upgradeShaPasswords = PasswordUpgradeSha diff --git a/modules/user/src/main/PasswordHasher.scala b/modules/user/src/main/PasswordHasher.scala index 802a6acf69..c407f0413e 100644 --- a/modules/user/src/main/PasswordHasher.scala +++ b/modules/user/src/main/PasswordHasher.scala @@ -61,17 +61,3 @@ final class PasswordHasher( BCrypt.bytesEqualSecure(hash, bHash(salt, pass)) } } - -final class AsyncPasswordHasher( - hasher: PasswordHasher, - sequencer: lila.hub.SyncMultiSequencer -) { - - def hash(pass: String): Fu[Array[Byte]] = sequencer { - hasher.hash(pass) - } - - def check(bytes: Array[Byte], pass: String): Fu[Boolean] = sequencer { - hasher.check(bytes, pass) - } -} diff --git a/modules/user/src/main/User.scala b/modules/user/src/main/User.scala index 06ec5d0710..c80a802a6e 100644 --- a/modules/user/src/main/User.scala +++ b/modules/user/src/main/User.scala @@ -111,13 +111,13 @@ object User { type ID = String - type CredentialCheck = String => Fu[Boolean] + type CredentialCheck = String => Boolean case class LoginCandidate(user: User, check: CredentialCheck) { - def apply(password: String): Fu[Option[User]] = - check(password) map { res => - lila.mon.user.auth.result(res)() - res option user - } + def apply(password: String): Option[User] = { + val res = check(password) + lila.mon.user.auth.result(res)() + res option user + } } val anonymous = "Anonymous" @@ -259,10 +259,10 @@ object User { } } -final class Authenticator(passHasher: AsyncPasswordHasher, onShaLogin: => Unit) { +final class Authenticator(passHasher: PasswordHasher, onShaLogin: () => Unit) { import com.roundeights.hasher.Implicits._ - def passEnc(pass: String): Fu[Array[Byte]] = passHasher.hash(pass) + def passEnc(pass: String): Array[Byte] = passHasher.hash(pass) case class AuthData( _id: String, @@ -271,7 +271,7 @@ final class Authenticator(passHasher: AsyncPasswordHasher, onShaLogin: => Unit) salt: Option[String] = None, sha512: Option[Boolean] = None ) { - def compare(p: String): Fu[Boolean] = { + def compare(p: String): Boolean = { val newP = salt.fold(p) { s => val salted = s"$p{$s}" // BC (~sha512).fold(salted.sha512, salted.sha1).hex @@ -279,7 +279,7 @@ final class Authenticator(passHasher: AsyncPasswordHasher, onShaLogin: => Unit) bpass match { // Deprecated fallback. Log & fail after DB migration. - case None => password ?? { p => onShaLogin; fuccess(p == newP) } + case None => password ?? { p => onShaLogin(); p == newP } case Some(bHash) => passHasher.check(bHash, newP) } } diff --git a/modules/user/src/main/UserRepo.scala b/modules/user/src/main/UserRepo.scala index 4e7500111a..bb38c34bd8 100644 --- a/modules/user/src/main/UserRepo.scala +++ b/modules/user/src/main/UserRepo.scala @@ -225,24 +225,21 @@ object UserRepo { def removeAllToints = coll.update($empty, $unset("toints"), multi = true) def authenticateById(id: ID, password: String): Fu[Option[User]] = - loginCandidateById(id) flatMap { _ ?? { _(password) } } + loginCandidateById(id) map { _ flatMap { _(password) } } def authenticateByEmail(email: EmailAddress, password: String): Fu[Option[User]] = - loginCandidateByEmail(email) flatMap { _ ?? { _(password) } } + loginCandidateByEmail(email) map { _ flatMap { _(password) } } import Env.current.passwordAuth._ // This creates a bcrypt hash using the existing sha as input, // allowing us to migrate all users in bulk. def upgradePassword(a: AuthData) = (a.bpass, a.password) match { - case (None, Some(pass)) => Some { - passEnc(pass) flatMap { hash => - coll.update( - $id(a._id), - $set(F.bpass -> hash) ++ $unset(F.password) - ).void >>- lila.mon.user.auth.shaBcUpgrade() - } - } + case (None, Some(pass)) => Some(coll.update( + $id(a._id), + $set(F.bpass -> passEnc(pass)) ++ $unset(F.password) + ).void >>- lila.mon.user.auth.shaBcUpgrade()) + case _ => None } @@ -253,19 +250,19 @@ object UserRepo { loginCandidate($doc(F.email -> email)) def loginCandidate(u: User): Fu[User.LoginCandidate] = - loginCandidateById(u.id) map { _ | User.LoginCandidate(u, _ => fuccess(false)) } + loginCandidateById(u.id) map { _ | User.LoginCandidate(u, _ => false) } - def authWithBenefits(auth: AuthData)(p: String): Fu[Boolean] = - auth compare p map { res => - if (res && auth.salt.isDefined && Env.current.upgradeShaPasswords) - passwd(id = auth._id, pass = p) >>- lila.mon.user.auth.bcFullMigrate() - res - } + def authWithBenefits(auth: AuthData)(p: String) = { + val res = auth compare p + if (res && auth.salt.isDefined && Env.current.upgradeShaPasswords) + passwd(id = auth._id, pass = p) >>- lila.mon.user.auth.bcFullMigrate() + res + } private def loginCandidate(select: Bdoc): Fu[Option[User.LoginCandidate]] = coll.uno[AuthData](select) zip coll.uno[User](select) map { case (Some(authData), Some(user)) if user.enabled => - User.LoginCandidate(user, authWithBenefits(authData) _).some + User.LoginCandidate(user, authWithBenefits(authData)).some case _ => none } @@ -283,11 +280,9 @@ object UserRepo { ): Fu[Option[User]] = !nameExists(username) flatMap { _ ?? { - passEnc(password) flatMap { hash => - val doc = newUser(username, hash, email, blind, mobileApiVersion, mustConfirmEmail) ++ - ("len" -> BSONInteger(username.size)) - coll.insert(doc) >> named(normalize(username)) - } + val doc = newUser(username, password, email, blind, mobileApiVersion, mustConfirmEmail) ++ + ("len" -> BSONInteger(username.size)) + coll.insert(doc) >> named(normalize(username)) } } @@ -352,12 +347,9 @@ object UserRepo { } ) - def passwd(id: ID, pass: String): Funit = passEnc(pass) flatMap { hash => - coll.update( - $id(id), - $set(F.bpass -> hash) ++ $unset(F.salt, F.password, F.sha512) - ).void - } + def passwd(id: ID, pass: String): Funit = + coll.update($id(id), $set(F.bpass -> passEnc(pass)) + ++ $unset(F.salt, F.password, F.sha512)).void def email(id: ID, email: EmailAddress): Funit = coll.update($id(id), $set(F.email -> email) ++ $unset(F.prevEmail)).void @@ -452,7 +444,7 @@ object UserRepo { private def newUser( username: String, - passwordHash: Array[Byte], + password: String, email: EmailAddress, blind: Boolean, mobileApiVersion: Option[ApiVersion], @@ -468,7 +460,7 @@ object UserRepo { F.username -> username, F.email -> email, F.mustConfirmEmail -> mustConfirmEmail.option(DateTime.now), - F.bpass -> passwordHash, + F.bpass -> passEnc(password), F.perfs -> $empty, F.count -> Count.default, F.enabled -> true,