Remove SHA256 as a supported password hashing algorithm (#2310)

We introduced Scrypt as the default password hashing algorithm in
November 2023 and have been auto-converting saved hashes whenever a
successful EPP login or registry lock/unlock request is processed.

We will send comms to registrars to inform them the upcoming removal of
SHA256 support and urge them to log in at least once before the change.
Otherwise, they will need to contact support to reset the password out of
band after the change.

This PR will NOT be submitted until comms are out and the effective date
is immediate.

Co-authored-by: Weimin Yu <weiminyu@google.com>
This commit is contained in:
Lai Jiang
2024-02-26 15:28:12 +00:00
committed by GitHub
co-authored by Weimin Yu
parent 1eef260da9
commit f72a0d2f16
9 changed files with 27 additions and 223 deletions
@@ -47,7 +47,6 @@ import google.registry.model.eppinput.EppInput.Options;
import google.registry.model.eppinput.EppInput.Services;
import google.registry.model.eppoutput.EppResponse;
import google.registry.model.registrar.Registrar;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.util.Optional;
import java.util.Set;
import javax.inject.Inject;
@@ -142,17 +141,8 @@ public class LoginFlow implements MutatingFlow {
throw new RegistrarAccountNotActiveException();
}
if (login.getNewPassword().isPresent()
|| registrar.get().getCurrentHashAlgorithm(login.getPassword()).orElse(null)
!= HashAlgorithm.SCRYPT) {
String newPassword =
login
.getNewPassword()
.orElseGet(
() -> {
logger.atInfo().log("Rehashing existing registrar password with Scrypt");
return login.getPassword();
});
if (login.getNewPassword().isPresent()) {
String newPassword = login.getNewPassword().get();
// Load fresh from database (bypassing the cache) to ensure we don't save stale data.
Optional<Registrar> freshRegistrar = Registrar.loadByRegistrarId(login.getClientId());
if (freshRegistrar.isEmpty()) {
@@ -86,8 +86,7 @@ public class User extends UpdateAutoTimestampEntity implements Buildable {
return false;
}
return PasswordUtils.verifyPassword(
registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt)
.isPresent();
registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt);
}
/**
@@ -62,7 +62,6 @@ import google.registry.model.tld.Tld.TldType;
import google.registry.persistence.VKey;
import google.registry.util.CidrAddressBlock;
import google.registry.util.PasswordUtils;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.security.cert.CertificateParsingException;
import java.util.Comparator;
import java.util.List;
@@ -642,10 +641,6 @@ public class Registrar extends UpdateAutoTimestampEntity implements Buildable, J
}
public boolean verifyPassword(String password) {
return getCurrentHashAlgorithm(password).isPresent();
}
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String password) {
return PasswordUtils.verifyPassword(password, passwordHash, salt);
}
@@ -38,7 +38,6 @@ import google.registry.model.UnsafeSerializable;
import google.registry.model.registrar.RegistrarPoc.RegistrarPocId;
import google.registry.persistence.VKey;
import google.registry.util.PasswordUtils;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.io.Serializable;
import java.util.Map;
import java.util.Optional;
@@ -242,10 +241,6 @@ public class RegistrarPoc extends ImmutableObject implements Jsonifiable, Unsafe
|| isNullOrEmpty(registryLockPasswordHash)) {
return false;
}
return getCurrentHashAlgorithm(registryLockPassword).isPresent();
}
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String registryLockPassword) {
return PasswordUtils.verifyPassword(
registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt);
}
@@ -47,7 +47,6 @@ import google.registry.request.auth.UserAuthInfo;
import google.registry.security.JsonResponseHelper;
import google.registry.tools.DomainLockUtils;
import google.registry.util.EmailMessage;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.net.URISyntaxException;
import java.util.Map;
import java.util.Optional;
@@ -223,19 +222,6 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
checkArgument(
registrarPoc.verifyRegistryLockPassword(postInput.password),
"Incorrect registry lock password for contact");
if (registrarPoc.getCurrentHashAlgorithm(postInput.password).orElse(null)
!= HashAlgorithm.SCRYPT) {
logger.atInfo().log("Rehashing existing registry lock password with Scrypt.");
tm().transact(
() -> {
tm().update(
tm().loadByEntity(registrarPoc)
.asBuilder()
.setAllowedToSetRegistryLockPassword(true)
.setRegistryLockPassword(postInput.password)
.build());
});
}
return registrarPoc
.getRegistryLockEmailAddress()
.orElseThrow(
@@ -14,15 +14,11 @@
package google.registry.flows.session;
import static com.google.common.io.BaseEncoding.base64;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.deleteResource;
import static google.registry.testing.DatabaseHelper.loadRegistrar;
import static google.registry.testing.DatabaseHelper.persistResource;
import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
import static google.registry.util.PasswordUtils.HashAlgorithm.SCRYPT;
import static google.registry.util.PasswordUtils.HashAlgorithm.SHA256;
import static org.junit.jupiter.api.Assertions.assertThrows;
import com.google.common.collect.ImmutableMap;
@@ -42,7 +38,6 @@ import google.registry.model.eppoutput.EppOutput;
import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.Registrar.State;
import google.registry.testing.DatabaseHelper;
import google.registry.util.PasswordUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -191,33 +186,6 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class);
}
@Test
void testSuccess_sha256Password() throws Exception {
String password = "foo-BAR2";
tm().transact(
() -> {
// The salt is not exposed by Registrar (nor should it be), so we query it
// directly.
String encodedSalt =
tm().query("SELECT salt FROM Registrar WHERE registrarId = :id", String.class)
.setParameter("id", registrar.getRegistrarId())
.getSingleResult();
byte[] salt = base64().decode(encodedSalt);
String newHash = PasswordUtils.hashPassword(password, salt, SHA256);
// Set password directly, as the Java method would have used Scrypt.
tm().query("UPDATE Registrar SET passwordHash = :hash WHERE registrarId = :id")
.setParameter("id", registrar.getRegistrarId())
.setParameter("hash", newHash)
.executeUpdate();
});
assertThat(loadRegistrar("NewRegistrar").getCurrentHashAlgorithm(password).get())
.isEqualTo(SHA256);
doSuccessfulTest("login_valid.xml");
// Verifies that after successfully login, the password is re-hased with Scrypt.
assertThat(loadRegistrar("NewRegistrar").getCurrentHashAlgorithm(password).get())
.isEqualTo(SCRYPT);
}
@Test
void testFailure_incorrectPassword() {
persistResource(getRegistrarBuilder().setPassword("diff password").build());
@@ -15,10 +15,8 @@
package google.registry.ui.server.registrar;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
import static com.google.common.io.BaseEncoding.base64;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.loadRegistrar;
import static google.registry.testing.DatabaseHelper.persistResource;
@@ -27,8 +25,6 @@ import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCod
import static google.registry.testing.SqlHelper.saveRegistryLock;
import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
import static google.registry.ui.server.registrar.RegistryLockGetActionTest.userFromRegistrarPoc;
import static google.registry.util.PasswordUtils.HashAlgorithm.SCRYPT;
import static google.registry.util.PasswordUtils.HashAlgorithm.SHA256;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
@@ -42,9 +38,6 @@ import google.registry.model.console.RegistrarRole;
import google.registry.model.console.UserRoles;
import google.registry.model.domain.Domain;
import google.registry.model.domain.RegistryLock;
import google.registry.model.registrar.RegistrarPoc;
import google.registry.model.registrar.RegistrarPoc.RegistrarPocId;
import google.registry.persistence.VKey;
import google.registry.persistence.transaction.JpaTestExtensions;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
import google.registry.persistence.transaction.JpaTransactionManagerExtension;
@@ -61,7 +54,6 @@ import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.tools.DomainLockUtils;
import google.registry.util.EmailMessage;
import google.registry.util.PasswordUtils;
import google.registry.util.StringGenerator.Alphabets;
import java.util.Map;
import java.util.Optional;
@@ -131,48 +123,6 @@ final class RegistryLockPostActionTest {
assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
}
@Test
void testSuccess_lock_sha256Password() throws Exception {
tm().transact(
() -> {
// The salt is not exposed by RegistrarPoc (nor should it be), so we query
// it directly.
String encodedSalt =
tm().query(
"SELECT registryLockPasswordSalt FROM RegistrarPoc "
+ "WHERE emailAddress = :email "
+ "AND registrarId = :registrarId",
String.class)
.setParameter("email", "Marla.Singer@crr.com")
.setParameter("registrarId", "TheRegistrar")
.getSingleResult();
byte[] salt = base64().decode(encodedSalt);
String newHash = PasswordUtils.hashPassword("hi", salt, SHA256);
// Set password directly, as the Java method would have used Scrypt.
tm().query("UPDATE RegistrarPoc SET registryLockPasswordHash = :hash")
.setParameter("hash", newHash)
.executeUpdate();
});
RegistrarPoc registrarPoc =
tm().transact(
() ->
tm().loadByKey(
VKey.create(
RegistrarPoc.class,
new RegistrarPocId("Marla.Singer@crr.com", "TheRegistrar"))));
assertThat(registrarPoc.getCurrentHashAlgorithm("hi").get()).isEqualTo(SHA256);
Map<String, ?> response = action.handleJsonRequest(lockRequest());
RegistrarPoc updatedRegistrarPoc =
tm().transact(
() ->
tm().loadByKey(
VKey.create(
RegistrarPoc.class,
new RegistrarPocId("Marla.Singer@crr.com", "TheRegistrar"))));
assertThat(updatedRegistrarPoc.getCurrentHashAlgorithm("hi").get()).isEqualTo(SCRYPT);
assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
}
@Test
void testSuccess_unlock() throws Exception {
saveRegistryLock(createLock().asBuilder().setLockCompletionTime(clock.nowUtc()).build());