Switch to a stronger algorithm for password hashing (#2191)

We have been using SHA256 to hash passwords (for both EPP and registry lock),
which is now considered too weak.

This PR switches to using Scrypt, a memory-hard slow hash function, with
recommended parameters per go/crypto-password-hash.

To ease the transition, when a password is being verified, both Scrypt
and SHA256 are tried. If SHA256 verification is successful, we re-hash
the verified password with Scrypt and replace the stored SHA256 hash
with the new one. This way, as long as a user uses the password once
before the transition period ends (when Scrypt becomes the only valid
algorithm), there would be no need for manual intervention from them.

We will send out notifications to users to remind them of the transition
and urge them to use the password (which should not be a problem with
EPP, but less so with the registry lock). After the transition,
out-of-band reset for EPP password, or remove-and-add on the console for
registry lock password, would be required for the hashes that have not
been re-saved.

Note that the re-save logic is not present for console user's registry
lock password, as there is no production data for console users yet.
Only legacy GAE user's password requires re-save.
This commit is contained in:
Lai Jiang 2023-11-03 17:29:01 -04:00 committed by GitHub
parent ba54208dad
commit cd23fea698
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 286 additions and 43 deletions

View file

@ -28,11 +28,17 @@ import google.registry.flows.EppException.CommandUseErrorException;
import google.registry.flows.EppException.ParameterValuePolicyErrorException; import google.registry.flows.EppException.ParameterValuePolicyErrorException;
import google.registry.flows.EppException.UnimplementedExtensionException; import google.registry.flows.EppException.UnimplementedExtensionException;
import google.registry.flows.EppException.UnimplementedObjectServiceException; import google.registry.flows.EppException.UnimplementedObjectServiceException;
import google.registry.flows.EppException.UnimplementedProtocolVersionException;
import google.registry.flows.ExtensionManager; import google.registry.flows.ExtensionManager;
import google.registry.flows.FlowModule.RegistrarId; import google.registry.flows.FlowModule.RegistrarId;
import google.registry.flows.FlowUtils.GenericXmlSyntaxErrorException;
import google.registry.flows.MutatingFlow; import google.registry.flows.MutatingFlow;
import google.registry.flows.SessionMetadata; import google.registry.flows.SessionMetadata;
import google.registry.flows.TlsCredentials.BadRegistrarCertificateException;
import google.registry.flows.TlsCredentials.BadRegistrarIpAddressException;
import google.registry.flows.TlsCredentials.MissingRegistrarCertificateException;
import google.registry.flows.TransportCredentials; import google.registry.flows.TransportCredentials;
import google.registry.flows.TransportCredentials.BadRegistrarPasswordException;
import google.registry.model.eppcommon.ProtocolDefinition; import google.registry.model.eppcommon.ProtocolDefinition;
import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension; import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension;
import google.registry.model.eppinput.EppInput; import google.registry.model.eppinput.EppInput;
@ -41,6 +47,7 @@ import google.registry.model.eppinput.EppInput.Options;
import google.registry.model.eppinput.EppInput.Services; import google.registry.model.eppinput.EppInput.Services;
import google.registry.model.eppoutput.EppResponse; import google.registry.model.eppoutput.EppResponse;
import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import javax.inject.Inject; import javax.inject.Inject;
@ -48,14 +55,14 @@ import javax.inject.Inject;
/** /**
* An EPP flow for login. * An EPP flow for login.
* *
* @error {@link google.registry.flows.EppException.UnimplementedExtensionException} * @error {@link UnimplementedExtensionException}
* @error {@link google.registry.flows.EppException.UnimplementedObjectServiceException} * @error {@link UnimplementedObjectServiceException}
* @error {@link google.registry.flows.EppException.UnimplementedProtocolVersionException} * @error {@link UnimplementedProtocolVersionException}
* @error {@link google.registry.flows.FlowUtils.GenericXmlSyntaxErrorException} * @error {@link GenericXmlSyntaxErrorException}
* @error {@link google.registry.flows.TlsCredentials.BadRegistrarCertificateException} * @error {@link BadRegistrarCertificateException}
* @error {@link google.registry.flows.TlsCredentials.BadRegistrarIpAddressException} * @error {@link BadRegistrarIpAddressException}
* @error {@link google.registry.flows.TlsCredentials.MissingRegistrarCertificateException} * @error {@link MissingRegistrarCertificateException}
* @error {@link google.registry.flows.TransportCredentials.BadRegistrarPasswordException} * @error {@link BadRegistrarPasswordException}
* @error {@link LoginFlow.AlreadyLoggedInException} * @error {@link LoginFlow.AlreadyLoggedInException}
* @error {@link BadRegistrarIdException} * @error {@link BadRegistrarIdException}
* @error {@link LoginFlow.TooManyFailedLoginsException} * @error {@link LoginFlow.TooManyFailedLoginsException}
@ -134,13 +141,24 @@ public class LoginFlow implements MutatingFlow {
if (!registrar.get().isLive()) { if (!registrar.get().isLive()) {
throw new RegistrarAccountNotActiveException(); throw new RegistrarAccountNotActiveException();
} }
if (login.getNewPassword().isPresent()) {
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();
});
// Load fresh from database (bypassing the cache) to ensure we don't save stale data. // Load fresh from database (bypassing the cache) to ensure we don't save stale data.
Optional<Registrar> freshRegistrar = Registrar.loadByRegistrarId(login.getClientId()); Optional<Registrar> freshRegistrar = Registrar.loadByRegistrarId(login.getClientId());
if (!freshRegistrar.isPresent()) { if (!freshRegistrar.isPresent()) {
throw new BadRegistrarIdException(login.getClientId()); throw new BadRegistrarIdException(login.getClientId());
} }
tm().put(freshRegistrar.get().asBuilder().setPassword(login.getNewPassword().get()).build()); tm().put(freshRegistrar.get().asBuilder().setPassword(newPassword).build());
} }
// We are in! // We are in!
@ -152,35 +170,35 @@ public class LoginFlow implements MutatingFlow {
/** Registrar with this ID could not be found. */ /** Registrar with this ID could not be found. */
static class BadRegistrarIdException extends AuthenticationErrorException { static class BadRegistrarIdException extends AuthenticationErrorException {
public BadRegistrarIdException(String registrarId) { BadRegistrarIdException(String registrarId) {
super("Registrar with this ID could not be found: " + registrarId); super("Registrar with this ID could not be found: " + registrarId);
} }
} }
/** Registrar login failed too many times. */ /** Registrar login failed too many times. */
static class TooManyFailedLoginsException extends AuthenticationErrorClosingConnectionException { static class TooManyFailedLoginsException extends AuthenticationErrorClosingConnectionException {
public TooManyFailedLoginsException() { TooManyFailedLoginsException() {
super("Registrar login failed too many times"); super("Registrar login failed too many times");
} }
} }
/** Registrar account is not active. */ /** Registrar account is not active. */
static class RegistrarAccountNotActiveException extends AuthorizationErrorException { static class RegistrarAccountNotActiveException extends AuthorizationErrorException {
public RegistrarAccountNotActiveException() { RegistrarAccountNotActiveException() {
super("Registrar account is not active"); super("Registrar account is not active");
} }
} }
/** Registrar is already logged in. */ /** Registrar is already logged in. */
static class AlreadyLoggedInException extends CommandUseErrorException { static class AlreadyLoggedInException extends CommandUseErrorException {
public AlreadyLoggedInException() { AlreadyLoggedInException() {
super("Registrar is already logged in"); super("Registrar is already logged in");
} }
} }
/** Specified language is not supported. */ /** Specified language is not supported. */
static class UnsupportedLanguageException extends ParameterValuePolicyErrorException { static class UnsupportedLanguageException extends ParameterValuePolicyErrorException {
public UnsupportedLanguageException() { UnsupportedLanguageException() {
super("Specified language is not supported"); super("Specified language is not supported");
} }
} }

View file

@ -25,6 +25,7 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import google.registry.model.Buildable; import google.registry.model.Buildable;
import google.registry.model.UpdateAutoTimestampEntity; import google.registry.model.UpdateAutoTimestampEntity;
import google.registry.persistence.VKey; import google.registry.persistence.VKey;
import google.registry.util.PasswordUtils;
import javax.persistence.Column; import javax.persistence.Column;
import javax.persistence.Entity; import javax.persistence.Entity;
import javax.persistence.GeneratedValue; import javax.persistence.GeneratedValue;
@ -84,8 +85,9 @@ public class User extends UpdateAutoTimestampEntity implements Buildable {
|| isNullOrEmpty(registryLockPasswordHash)) { || isNullOrEmpty(registryLockPasswordHash)) {
return false; return false;
} }
return hashPassword(registryLockPassword, registryLockPasswordSalt) return PasswordUtils.verifyPassword(
.equals(registryLockPasswordHash); registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt)
.isPresent();
} }
/** /**
@ -154,9 +156,9 @@ public class User extends UpdateAutoTimestampEntity implements Buildable {
!getInstance().hasRegistryLockPassword(), "User already has a password, remove it first"); !getInstance().hasRegistryLockPassword(), "User already has a password, remove it first");
checkArgument( checkArgument(
!isNullOrEmpty(registryLockPassword), "Registry lock password was null or empty"); !isNullOrEmpty(registryLockPassword), "Registry lock password was null or empty");
getInstance().registryLockPasswordSalt = base64().encode(SALT_SUPPLIER.get()); byte[] salt = SALT_SUPPLIER.get();
getInstance().registryLockPasswordHash = getInstance().registryLockPasswordSalt = base64().encode(salt);
hashPassword(registryLockPassword, getInstance().registryLockPasswordSalt); getInstance().registryLockPasswordHash = hashPassword(registryLockPassword, salt);
return this; return this;
} }
} }

View file

@ -60,6 +60,8 @@ import google.registry.model.tld.Tld;
import google.registry.model.tld.Tld.TldType; import google.registry.model.tld.Tld.TldType;
import google.registry.persistence.VKey; import google.registry.persistence.VKey;
import google.registry.util.CidrAddressBlock; import google.registry.util.CidrAddressBlock;
import google.registry.util.PasswordUtils;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.security.cert.CertificateParsingException; import java.security.cert.CertificateParsingException;
import java.util.Comparator; import java.util.Comparator;
import java.util.List; import java.util.List;
@ -97,7 +99,7 @@ import org.joda.time.DateTime;
column = @Column(nullable = false, name = "lastUpdateTime")) column = @Column(nullable = false, name = "lastUpdateTime"))
public class Registrar extends UpdateAutoTimestampEntity implements Buildable, Jsonifiable { public class Registrar extends UpdateAutoTimestampEntity implements Buildable, Jsonifiable {
/** Represents the type of a registrar entity. */ /** Represents the type of registrar entity. */
public enum Type { public enum Type {
/** A real-world, third-party registrar. Should have non-null IANA and billing account IDs. */ /** A real-world, third-party registrar. Should have non-null IANA and billing account IDs. */
REAL(Objects::nonNull), REAL(Objects::nonNull),
@ -376,7 +378,7 @@ public class Registrar extends UpdateAutoTimestampEntity implements Buildable, J
*/ */
@Expose String icannReferralEmail; @Expose String icannReferralEmail;
/** Id of the folder in drive used to publish information for this registrar. */ /** ID of the folder in drive used to publish information for this registrar. */
@Expose String driveFolderId; @Expose String driveFolderId;
// Metadata. // Metadata.
@ -639,7 +641,11 @@ public class Registrar extends UpdateAutoTimestampEntity implements Buildable, J
} }
public boolean verifyPassword(String password) { public boolean verifyPassword(String password) {
return hashPassword(password, salt).equals(passwordHash); return getCurrentHashAlgorithm(password).isPresent();
}
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String password) {
return PasswordUtils.verifyPassword(password, passwordHash, salt);
} }
public String getPhonePasscode() { public String getPhonePasscode() {
@ -861,8 +867,9 @@ public class Registrar extends UpdateAutoTimestampEntity implements Buildable, J
checkArgument( checkArgument(
Range.closed(6, 16).contains(nullToEmpty(password).length()), Range.closed(6, 16).contains(nullToEmpty(password).length()),
"Password must be 6-16 characters long."); "Password must be 6-16 characters long.");
getInstance().salt = base64().encode(SALT_SUPPLIER.get()); byte[] salt = SALT_SUPPLIER.get();
getInstance().passwordHash = hashPassword(password, getInstance().salt); getInstance().salt = base64().encode(salt);
getInstance().passwordHash = hashPassword(password, salt);
return this; return this;
} }

View file

@ -37,6 +37,8 @@ import google.registry.model.Jsonifiable;
import google.registry.model.UnsafeSerializable; import google.registry.model.UnsafeSerializable;
import google.registry.model.registrar.RegistrarPoc.RegistrarPocId; import google.registry.model.registrar.RegistrarPoc.RegistrarPocId;
import google.registry.persistence.VKey; import google.registry.persistence.VKey;
import google.registry.util.PasswordUtils;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.io.Serializable; import java.io.Serializable;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
@ -240,8 +242,12 @@ public class RegistrarPoc extends ImmutableObject implements Jsonifiable, Unsafe
|| isNullOrEmpty(registryLockPasswordHash)) { || isNullOrEmpty(registryLockPasswordHash)) {
return false; return false;
} }
return hashPassword(registryLockPassword, registryLockPasswordSalt) return getCurrentHashAlgorithm(registryLockPassword).isPresent();
.equals(registryLockPasswordHash); }
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String registryLockPassword) {
return PasswordUtils.verifyPassword(
registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt);
} }
/** /**
@ -436,9 +442,9 @@ public class RegistrarPoc extends ImmutableObject implements Jsonifiable, Unsafe
"Not allowed to set registry lock password for this contact"); "Not allowed to set registry lock password for this contact");
checkArgument( checkArgument(
!isNullOrEmpty(registryLockPassword), "Registry lock password was null or empty"); !isNullOrEmpty(registryLockPassword), "Registry lock password was null or empty");
getInstance().registryLockPasswordSalt = base64().encode(SALT_SUPPLIER.get()); byte[] salt = SALT_SUPPLIER.get();
getInstance().registryLockPasswordHash = getInstance().registryLockPasswordSalt = base64().encode(salt);
hashPassword(registryLockPassword, getInstance().registryLockPasswordSalt); getInstance().registryLockPasswordHash = hashPassword(registryLockPassword, salt);
getInstance().allowedToSetRegistryLockPassword = false; getInstance().allowedToSetRegistryLockPassword = false;
return this; return this;
} }

View file

@ -47,6 +47,7 @@ import google.registry.request.auth.UserAuthInfo;
import google.registry.security.JsonResponseHelper; import google.registry.security.JsonResponseHelper;
import google.registry.tools.DomainLockUtils; import google.registry.tools.DomainLockUtils;
import google.registry.util.EmailMessage; import google.registry.util.EmailMessage;
import google.registry.util.PasswordUtils.HashAlgorithm;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
@ -126,6 +127,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
.userAuthInfo() .userAuthInfo()
.orElseThrow(() -> new ForbiddenException("User is not logged in")); .orElseThrow(() -> new ForbiddenException("User is not logged in"));
// TODO: Move this line to the transaction below during nested transaction refactoring.
String userEmail = verifyPasswordAndGetEmail(userAuthInfo, postInput); String userEmail = verifyPasswordAndGetEmail(userAuthInfo, postInput);
tm().transact( tm().transact(
() -> { () -> {
@ -208,6 +210,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
throws RegistrarAccessDeniedException { throws RegistrarAccessDeniedException {
// Verify that the user can access the registrar, that the user has // Verify that the user can access the registrar, that the user has
// registry lock enabled, and that the user provided a correct password // registry lock enabled, and that the user provided a correct password
Registrar registrar = Registrar registrar =
getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.registrarId, false); getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.registrarId, false);
RegistrarPoc registrarPoc = RegistrarPoc registrarPoc =
@ -220,6 +223,19 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
checkArgument( checkArgument(
registrarPoc.verifyRegistryLockPassword(postInput.password), registrarPoc.verifyRegistryLockPassword(postInput.password),
"Incorrect registry lock password for contact"); "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 return registrarPoc
.getRegistryLockEmailAddress() .getRegistryLockEmailAddress()
.orElseThrow( .orElseThrow(

View file

@ -14,11 +14,15 @@
package google.registry.flows.session; package google.registry.flows.session;
import static com.google.common.io.BaseEncoding.base64;
import static com.google.common.truth.Truth.assertThat; 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.deleteResource;
import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.loadRegistrar;
import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistResource;
import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions; 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 static org.junit.jupiter.api.Assertions.assertThrows;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
@ -38,6 +42,7 @@ import google.registry.model.eppoutput.EppOutput;
import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.State;
import google.registry.testing.DatabaseHelper; import google.registry.testing.DatabaseHelper;
import google.registry.util.PasswordUtils;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -186,6 +191,33 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class); 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 @Test
void testFailure_incorrectPassword() { void testFailure_incorrectPassword() {
persistResource(getRegistrarBuilder().setPassword("diff password").build()); persistResource(getRegistrarBuilder().setPassword("diff password").build());

View file

@ -15,8 +15,10 @@
package google.registry.ui.server.registrar; package google.registry.ui.server.registrar;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; 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 com.google.common.truth.Truth.assertThat;
import static google.registry.model.EppResourceUtils.loadByForeignKey; 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.createTld;
import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.loadRegistrar;
import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistResource;
@ -25,6 +27,8 @@ import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCod
import static google.registry.testing.SqlHelper.saveRegistryLock; import static google.registry.testing.SqlHelper.saveRegistryLock;
import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
import static google.registry.ui.server.registrar.RegistryLockGetActionTest.userFromRegistrarPoc; 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.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@ -38,6 +42,9 @@ import google.registry.model.console.RegistrarRole;
import google.registry.model.console.UserRoles; import google.registry.model.console.UserRoles;
import google.registry.model.domain.Domain; import google.registry.model.domain.Domain;
import google.registry.model.domain.RegistryLock; 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;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
import google.registry.persistence.transaction.JpaTransactionManagerExtension; import google.registry.persistence.transaction.JpaTransactionManagerExtension;
@ -54,6 +61,7 @@ import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
import google.registry.tools.DomainLockUtils; import google.registry.tools.DomainLockUtils;
import google.registry.util.EmailMessage; import google.registry.util.EmailMessage;
import google.registry.util.PasswordUtils;
import google.registry.util.StringGenerator.Alphabets; import google.registry.util.StringGenerator.Alphabets;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
@ -123,6 +131,48 @@ final class RegistryLockPostActionTest {
assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com"); 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 @Test
void testSuccess_unlock() throws Exception { void testSuccess_unlock() throws Exception {
saveRegistryLock(createLock().asBuilder().setLockCompletionTime(clock.nowUtc()).build()); saveRegistryLock(createLock().asBuilder().setLockCompletionTime(clock.nowUtc()).build());

View file

@ -15,33 +15,114 @@
package google.registry.util; package google.registry.util;
import static com.google.common.io.BaseEncoding.base64; import static com.google.common.io.BaseEncoding.base64;
import static google.registry.util.PasswordUtils.HashAlgorithm.SCRYPT;
import static google.registry.util.PasswordUtils.HashAlgorithm.SHA256;
import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.US_ASCII;
import com.google.common.base.Supplier; import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.common.flogger.FluentLogger;
import java.security.MessageDigest; import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException; import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom; import java.security.SecureRandom;
import java.util.Arrays;
import java.util.Optional;
import org.bouncycastle.crypto.generators.SCrypt;
/** Common utility class to handle password hashing and salting */ /** Common utility class to handle password hashing and salting */
public final class PasswordUtils { public final class PasswordUtils {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final Supplier<MessageDigest> SHA256_DIGEST_SUPPLIER =
Suppliers.memoize(
() -> {
try {
return MessageDigest.getInstance("SHA-256");
} catch (NoSuchAlgorithmException e) {
// All implementations of MessageDigest are required to support SHA-256.
throw new RuntimeException(
"All MessageDigest implementations are required to support SHA-256 but this one"
+ " didn't",
e);
}
});
private PasswordUtils() {}
/**
* Password hashing algorithm that takes a password and a salt (both as {@code byte[]}) and
* returns a hash.
*/
public enum HashAlgorithm {
/**
* SHA-2 that returns a 256-bit digest.
*
* @see <a href="https://en.wikipedia.org/wiki/SHA-2">SHA-2</a>
*/
@Deprecated
SHA256 {
@Override
byte[] hash(byte[] password, byte[] salt) {
return SHA256_DIGEST_SUPPLIER
.get()
.digest((new String(password, US_ASCII) + base64().encode(salt)).getBytes(US_ASCII));
}
},
/**
* Memory-hard hashing algorithm, preferred over SHA-256.
*
* @see <a href="https://en.wikipedia.org/wiki/Scrypt">Scrypt</a>
*/
SCRYPT {
@Override
byte[] hash(byte[] password, byte[] salt) {
return SCrypt.generate(password, salt, 32768, 8, 1, 256);
}
};
abstract byte[] hash(byte[] password, byte[] salt);
}
public static final Supplier<byte[]> SALT_SUPPLIER = public static final Supplier<byte[]> SALT_SUPPLIER =
() -> { () -> {
// There are 32 bytes in a SHA-256 hash, and the salt should generally be the same size. // The generated hashes are 256 bits, and the salt should generally be of the same size.
byte[] salt = new byte[32]; byte[] salt = new byte[32];
new SecureRandom().nextBytes(salt); new SecureRandom().nextBytes(salt);
return salt; return salt;
}; };
public static String hashPassword(String password, String salt) { public static String hashPassword(String password, byte[] salt) {
try { return hashPassword(password, salt, SCRYPT);
return base64() }
.encode(
MessageDigest.getInstance("SHA-256").digest((password + salt).getBytes(US_ASCII))); /** Returns the hash of the password using the provided salt and {@link HashAlgorithm}. */
} catch (NoSuchAlgorithmException e) { public static String hashPassword(String password, byte[] salt, HashAlgorithm algorithm) {
// All implementations of MessageDigest are required to support SHA-256. return base64().encode(algorithm.hash(password.getBytes(US_ASCII), salt));
throw new RuntimeException( }
"All MessageDigest implementations are required to support SHA-256 but this didn't", e);
/**
* Verifies a password by regenerating the hash with the provided salt and comparing it to the
* provided hash.
*
* <p>This method will first try to use {@link HashAlgorithm#SCRYPT} to verify the password, and
* falls back to {@link HashAlgorithm#SHA256} if the former fails.
*
* @return the {@link HashAlgorithm} used to successfully verify the password, or {@link
* Optional#empty()} if neither works.
*/
public static Optional<HashAlgorithm> verifyPassword(String password, String hash, String salt) {
byte[] decodedHash = base64().decode(hash);
byte[] decodedSalt = base64().decode(salt);
byte[] calculatedHash = SCRYPT.hash(password.getBytes(US_ASCII), decodedSalt);
if (Arrays.equals(decodedHash, calculatedHash)) {
logger.atInfo().log("Scrypt hash verified.");
return Optional.of(SCRYPT);
} }
calculatedHash = SHA256.hash(password.getBytes(US_ASCII), decodedSalt);
if (Arrays.equals(decodedHash, calculatedHash)) {
logger.atInfo().log("SHA256 hash verified.");
return Optional.of(SHA256);
}
return Optional.empty();
} }
} }

View file

@ -16,13 +16,16 @@ package google.registry.util;
import static com.google.common.io.BaseEncoding.base64; import static com.google.common.io.BaseEncoding.base64;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static google.registry.util.PasswordUtils.HashAlgorithm.SCRYPT;
import static google.registry.util.PasswordUtils.HashAlgorithm.SHA256;
import static google.registry.util.PasswordUtils.SALT_SUPPLIER; import static google.registry.util.PasswordUtils.SALT_SUPPLIER;
import static google.registry.util.PasswordUtils.hashPassword; import static google.registry.util.PasswordUtils.hashPassword;
import static google.registry.util.PasswordUtils.verifyPassword;
import java.util.Arrays; import java.util.Arrays;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
/** Unit tests for {@link google.registry.util.PasswordUtils}. */ /** Unit tests for {@link PasswordUtils}. */
final class PasswordUtilsTest { final class PasswordUtilsTest {
@Test @Test
@ -36,12 +39,40 @@ final class PasswordUtilsTest {
@Test @Test
void testHash() { void testHash() {
String salt = base64().encode(SALT_SUPPLIER.get()); byte[] salt = SALT_SUPPLIER.get();
String password = "mySuperSecurePassword"; String password = "mySuperSecurePassword";
String hashedPassword = hashPassword(password, salt); String hashedPassword = hashPassword(password, salt);
assertThat(hashedPassword).isEqualTo(hashPassword(password, salt)); assertThat(hashedPassword).isEqualTo(hashPassword(password, salt));
assertThat(hashedPassword).isNotEqualTo(hashPassword(password + "a", salt)); assertThat(hashedPassword).isNotEqualTo(hashPassword(password + "a", salt));
String secondSalt = base64().encode(SALT_SUPPLIER.get()); byte[] secondSalt = SALT_SUPPLIER.get();
assertThat(hashedPassword).isNotEqualTo(hashPassword(password, secondSalt)); assertThat(hashedPassword).isNotEqualTo(hashPassword(password, secondSalt));
} }
@Test
void testVerify_scrypt_default() {
byte[] salt = SALT_SUPPLIER.get();
String password = "mySuperSecurePassword";
String hashedPassword = hashPassword(password, salt);
assertThat(hashedPassword).isEqualTo(hashPassword(password, salt, SCRYPT));
assertThat(verifyPassword(password, hashedPassword, base64().encode(salt)).get())
.isEqualTo(SCRYPT);
}
@Test
void testVerify_sha256() {
byte[] salt = SALT_SUPPLIER.get();
String password = "mySuperSecurePassword";
String hashedPassword = hashPassword(password, salt, SHA256);
assertThat(verifyPassword(password, hashedPassword, base64().encode(salt)).get())
.isEqualTo(SHA256);
}
@Test
void testVerify_failure() {
byte[] salt = SALT_SUPPLIER.get();
String password = "mySuperSecurePassword";
String hashedPassword = hashPassword(password, salt);
assertThat(verifyPassword(password + "a", hashedPassword, base64().encode(salt)).isEmpty())
.isTrue();
}
} }