Bug fixes, add tests

Green light!
This commit is contained in:
Isaac Levy 2017-09-23 23:54:04 -04:00
parent cdfa5607c4
commit 72db437770
8 changed files with 239 additions and 61 deletions

View file

@ -90,7 +90,8 @@ final class Env(
)
lazy val passwordHasher = new PasswordHasher(
secret = PasswordBPassSecret
secret = PasswordBPassSecret,
logRounds = 12
)
lazy val upgradeShaPasswords = PasswordUpgradeSha

View file

@ -4,32 +4,46 @@ import javax.crypto.Cipher
import javax.crypto.spec.{ IvParameterSpec, SecretKeySpec }
import java.util.Base64
class PasswordHasher(secret: String) {
private val (lameIv, sKey) = {
/**
* Encryption for bcrypt hashes.
*
* Security is dependent on plaintext format.
* This class should not be used for other purposes.
*/
private[user] class DumbAes(secret: String) {
// Bcrypt hashes start with a full block (16 bytes) of
// random data, so a static IV won't break primitives.
private val (sIV, sKey) = {
val bs = Base64.getDecoder.decode(secret)
if (bs.size != 32) throw new IllegalStateException
(new IvParameterSpec(bs take 16), new SecretKeySpec(bs drop 16, "AES"))
}
// Static IV because bcrypt hashes start with 16 bytes of random data
private def dumbAes(mode: Int, data: Array[Byte]) = {
@inline private def process(mode: Int, data: Array[Byte]) = {
val c = Cipher.getInstance("AES/CTS/NoPadding")
c.init(mode, sKey, lameIv)
c.init(mode, sKey, sIV)
c.doFinal(data)
}
import org.mindrot.BCrypt
import Cipher.{ ENCRYPT_MODE, DECRYPT_MODE }
def encrypt(data: Array[Byte]) = process(ENCRYPT_MODE, data)
def decrypt(data: Array[Byte]) = process(DECRYPT_MODE, data)
}
class PasswordHasher(secret: String, logRounds: Int) {
private val aes = new DumbAes(secret)
import org.mindrot.BCrypt
def hash(pass: String) = {
val salt = BCrypt.gensaltRaw
val hash = BCrypt.hashpwRaw(pass, 'a', 12, salt)
dumbAes(ENCRYPT_MODE, salt ++ hash)
val hash = BCrypt.hashpwRaw(pass, 'a', logRounds, salt)
aes.encrypt(salt ++ hash)
}
def check(encHash: Array[Byte], pass: String) = encHash.size == 39 && {
val (salt, hash) = dumbAes(DECRYPT_MODE, encHash).splitAt(16)
val newHash = BCrypt.hashpwRaw(pass, 'a', 12, salt)
val (salt, hash) = aes.decrypt(encHash).splitAt(16)
val newHash = BCrypt.hashpwRaw(pass, 'a', logRounds, salt)
BCrypt.bytesEqualSecure(hash, newHash)
}
}

View file

@ -253,3 +253,36 @@ object User {
)
}
}
class Authenticator(passHasher: PasswordHasher) {
import com.roundeights.hasher.Implicits._
private def salted(p: String, salt: String) = s"$p{$salt}"
def passEnc(id: String, pass: String) = passHasher.hash(salted(pass, id))
case class AuthData(
_id: String,
bpass: Option[Array[Byte]] = None,
password: Option[String] = None,
salt: Option[String] = None,
sha512: Option[Boolean] = None
) {
def compare(p: String) = {
val newP = (password, sha512) match {
case (None, None) => p
case _ => {
val pSalt = salt.fold(p) { salted(p, _) }
(~sha512).fold(pSalt.sha512, pSalt.sha1).hex
}
}
bpass match {
// Deprecated fallback. Log & fail after DB migration.
case None => password ?? { _ == newP }
case Some(bHash) => passHasher.check(bHash, salted(newP, _id))
}
}
def hashToken = bpass.fold(~password) { _.sha512.hex }
}
}

View file

@ -1,6 +1,5 @@
package lila.user
import com.roundeights.hasher.Implicits._
import org.joda.time.DateTime
import reactivemongo.api._
import reactivemongo.api.commands.GetLastError
@ -231,46 +230,15 @@ object UserRepo {
def authenticateByEmail(email: EmailAddress, password: String): Fu[Option[User]] =
loginCandidateByEmail(email) map { _ flatMap { _(password) } }
@inline def passHasher = Env.current.passwordHasher
private val authWrapper = new Authenticator(Env.current.passwordHasher)
import authWrapper.{ passEnc, AuthData }
private def salted(p: String, salt: String) = s"$p{$salt}"
private def passEnc(pass: String, id: String) = passHasher.hash(salted(pass, id))
private case class AuthData(
_id: String,
bpass: Option[Array[Byte]],
password: Option[String],
salt: Option[String],
sha512: Option[Boolean]
) {
def compare(p: String) = {
val newP = (password, sha512) match {
case (None, None) => p
case _ => {
val pSalt = salt.fold(p) { salted(p, _) }
(~sha512).fold(pSalt.sha512, pSalt.sha1).hex
}
}
val res = bpass match {
// Deprecated fallback. Log & fail after DB migration.
case None => password ?? { _ == newP }
case Some(bHash) => passHasher.check(bHash, salted(newP, _id))
}
if (res && password.isDefined && Env.current.upgradeShaPasswords)
passwd(id = _id, pass = p)
res
}
}
// This creates a bcrypt password using the an existing sha hash as
// the "plain text", allowing us to migrate all users in bulk.
// 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(p)) => coll.update($id(a._id), $set(
case (None, Some(pass)) => coll.update($id(a._id), $set(
F.sha512 -> ~a.sha512,
F.bpass -> passEnc(p, a._id)
F.bpass -> passEnc(id = a._id, pass = pass)
) ++ $unset(F.password)).void.some
case _ => None
@ -287,14 +255,22 @@ object UserRepo {
def loginCandidate(u: User): Fu[User.LoginCandidate] =
loginCandidateById(u.id) map { _ | User.LoginCandidate(u, _ => false) }
def authWithBenefits(auth: AuthData)(p: String) = {
val res = auth compare p
if (res && auth.password.isDefined && Env.current.upgradeShaPasswords)
passwd(id = auth._id, pass = p)
res
}
private def loginCandidate(select: Bdoc): Fu[Option[User.LoginCandidate]] =
coll.uno[AuthData](select) zip coll.uno[User](select) map {
case (Some(login), Some(user)) if user.enabled => User.LoginCandidate(user, login.compare).some
case (Some(authData), Some(user)) if user.enabled =>
User.LoginCandidate(user, authWithBenefits(authData)).some
case _ => none
}
def getPasswordHash(id: ID): Fu[Option[String]] = coll.byId[AuthData](id) map {
_.map { auth => auth.bpass.fold(~auth.password) { _.sha512.hex } }
_.map { _.hashToken }
}
def create(
@ -375,7 +351,8 @@ object UserRepo {
)
def passwd(id: ID, pass: String): Funit =
coll.update($id(id), $set(F.bpass -> passEnc(pass, id)) ++ $unset(F.salt, F.password, F.sha512)).void
coll.update($id(id), $set(F.bpass -> passEnc(id, pass = 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
@ -488,7 +465,7 @@ object UserRepo {
F.username -> username,
F.email -> email,
F.mustConfirmEmail -> mustConfirmEmail.option(DateTime.now),
F.bpass -> passEnc(password, id),
F.bpass -> passEnc(id, pass = password),
F.perfs -> $empty,
F.count -> Count.default,
F.enabled -> true,

View file

@ -398,14 +398,12 @@ public final class BCrypt {
* @return base64-encoded string
* @exception IllegalArgumentException if the length is invalid
*/
private static String encode_base64(byte d[], int len)
public static String encode_base64(byte d[])
throws IllegalArgumentException {
int off = 0;
StringBuilder rs = new StringBuilder();
int c1, c2;
if (len <= 0 || len > d.length)
throw new IllegalArgumentException ("Invalid len");
int len = d.length;
while (off < len) {
c1 = d[off++] & 0xff;
@ -626,7 +624,7 @@ public final class BCrypt {
int rounds, i, j;
byte ret[];
if (log_rounds < 4 || log_rounds > 30)
if (log_rounds < 2 || log_rounds > 30)
throw new IllegalArgumentException ("Bad number of rounds");
rounds = 1 << log_rounds;
if (salt.length != BCRYPT_SALT_LEN)
@ -716,8 +714,8 @@ public final class BCrypt {
}
rs.append(Integer.toString(rounds));
rs.append("$");
rs.append(encode_base64(saltb, saltb.length));
rs.append(encode_base64(hashed, hashed.length));
rs.append(encode_base64(saltb));
rs.append(encode_base64(hashed));
return rs.toString();
}
@ -742,7 +740,7 @@ public final class BCrypt {
}
rs.append(Integer.toString(log_rounds));
rs.append("$");
rs.append(encode_base64(rnd, rnd.length));
rs.append(encode_base64(rnd));
return rs.toString();
}

View file

@ -0,0 +1,76 @@
package lila.user
import org.specs2.mutable.Specification
import org.mindrot.BCrypt
import javax.crypto.Cipher.{ ENCRYPT_MODE, DECRYPT_MODE }
import BCrypt.{ bytesEqualSecure => bcryptEq }
import java.util.Base64
class AuthTest extends Specification {
val secret = Array.fill(32)(1.toByte).toBase64
val aes = new DumbAes(secret)
val passHasher = new PasswordHasher(secret, 2)
val authWrapper = new Authenticator(passHasher)
import authWrapper.{ passEnc, AuthData }
// Extracted from mongo
val shaUser = AuthData(
_id = "foo",
password = Some("1c4b2f9a0605c1af73d0ac66ab67c89a6bc76efa"),
salt = Some("7IzdmPSe0iZnGc1ChY32fVsfrZBLdIlN")
)
"sha matches" in {
// Mongo after password change
val shaUserWithKey = shaUser.copy(sha512 = Some(false))
"correct1" >> shaUser.compare("password")
"correct2" >> shaUserWithKey.compare("password")
"wrong1" >> !shaUser.compare("")
"wrong2" >> !shaUser.compare("")
"wrong sha" >> !shaUser.copy(sha512 = Some(true)).compare("password")
}
"bcrypt checks" in {
val bCryptUser = AuthData(
_id = "foo",
bpass = Some(Base64.getDecoder.decode(
"nMY6Pi45178YiOMcWncklizO3Z2enZfiFF5RDBkFZFYEOP7rZ2F0"
))
)
"correct" >> bCryptUser.compare("password")
"wrong pass" >> !bCryptUser.compare("")
// bpass is salted with id to prevent copying a bpass field
"wrong user" >> !bCryptUser.copy(_id = "bar").compare("password")
// sanity check of aes encryption
"wrong secret" >> !{
val badHasher = new PasswordHasher((new Array[Byte](32)).toBase64, 2)
new Authenticator(badHasher).AuthData(
_id = "foo",
bpass = bCryptUser.bpass
).compare("password")
}
}
"migrated user" in {
val shaToBcrypt = shaUser.copy(
// generated purely from stored data
bpass = Some(passEnc("foo", shaUser.password.get))
)
val shaToBcryptNoPass = shaToBcrypt.copy(
password = None,
sha512 = Some(false)
)
"correct" >> shaToBcrypt.compare("password")
"wrong pass" >> !shaToBcrypt.compare("")
"wrong user" >> !shaToBcrypt.copy(_id = "bar").compare("password")
"no pass" >> shaToBcryptNoPass.compare("password")
"sha flag lost" >> !shaToBcryptNoPass.copy(sha512 = None).compare("password")
"wrong sha" >> !shaToBcryptNoPass.copy(sha512 = Some(true)).compare("password")
}
}

View file

@ -0,0 +1,41 @@
package lila.user
import org.specs2.mutable.Specification
import org.mindrot.BCrypt
import javax.crypto.Cipher.{ ENCRYPT_MODE, DECRYPT_MODE }
import BCrypt.{ bytesEqualSecure => bcryptEq }
class BCryptTest extends Specification {
// From jBcrypt test suite.
val pass = "abc"
val b64Hash = "$2a$06$If6bvum7DFjUnE9p2uDeDu0YHzrHM6tf.iqN8.yx.jNN1ILEf7h0i"
"bcrypt" should {
"accept correct pass" >> BCrypt.checkpw(pass, b64Hash)
"reject bad password" >> !BCrypt.checkpw("", b64Hash)
val salt = BCrypt.gensaltRaw
"with raw bytes" in {
val rawHash = BCrypt.hashpwRaw(pass, 'a', 6, salt)
salt.size must_== 16
rawHash.size must_== 23
import BCrypt.{ encode_base64 => bc64 }
val bString = "$2a$06$" + bc64(salt) + bc64(rawHash)
"accept good" >> BCrypt.checkpw(pass, bString)
"reject bad" >> !BCrypt.checkpw("", bString)
"uniq salts" >> { salt !== BCrypt.gensaltRaw }
}
"handle crazy passwords" in {
val hashIt = (p: String) => BCrypt.hashpwRaw(p, 'a', 2, salt)
val abcHash = hashIt("abc")
"test eq" >> bcryptEq(abcHash, hashIt("abc"))
"vs null bytes" >> !bcryptEq(abcHash, hashIt("abc\u0000"))
"vs unicode" >> !bcryptEq(abcHash, hashIt("abc\uD83D\uDE01"))
"vs empty" >> !bcryptEq(abcHash, hashIt(""))
}
}
}

View file

@ -0,0 +1,38 @@
package lila.user
import org.specs2.mutable.Specification
import org.mindrot.BCrypt
class PasswordHasherTest extends Specification {
val secret = Array.fill(32)(1.toByte).toBase64
"bad secrets throw exceptions" in {
new DumbAes("") must throwA[IllegalStateException]
new PasswordHasher("", 12) must throwA[IllegalStateException]
new PasswordHasher("t=", 12) must throwA[IllegalArgumentException]
}
"aes" should {
val aes = new DumbAes(secret)
def emptyArr(i: Int) = new Array[Byte](i)
"preserve size" in {
aes.encrypt(emptyArr(20)).size must_== 20
aes.encrypt(emptyArr(39)).size must_== 39
}
val enc20 = aes.encrypt(emptyArr(20))
"encrypt input" >> { enc20 !== emptyArr(20) }
"and decrypt" >> { aes.decrypt(aes.encrypt(emptyArr(20))).sum == 0 }
"constant encryption" >> { enc20 === aes.encrypt(emptyArr(20)) }
}
"hasher" should {
val passHasher = new PasswordHasher(secret, 2)
val liHash = passHasher.hash("abc")
"accept good" >> passHasher.check(liHash, "abc")
"reject bad" >> !passHasher.check(liHash, "abc ")
"uniq hash" >> { liHash !== passHasher.hash("abc") }
}
}