Skip to content

Commit b1266c9

Browse files
authored
Add and default to Argon2 hashing (#2877)
We've previously been using Scrypt since PR #2191 which, while being a memory-hard slow function, isn't the optimal solution according to the OWASP recommendations. While we could get away with increasing the parallelization parameter to 3, it's better to just switch to the most-recommended solution if we're switching things up anyway. For the transition, we do something similar to PR #2191 where if the previous-algorithm's hash is successful, we re-hash with Argo2id and store that version. By doing this, we should not need any intervention for registrars who log in at any point during the transition period. Much of this PR, especially the parts where we re-hash the passwords in Argon2 instead of Scrypt upon login, is based on the code that was eventually removed in #2310.
1 parent bc9aab6 commit b1266c9

File tree

8 files changed

+241
-32
lines changed

8 files changed

+241
-32
lines changed

core/src/main/java/google/registry/flows/session/LoginFlow.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import google.registry.model.eppinput.EppInput.Services;
4848
import google.registry.model.eppoutput.EppResponse;
4949
import google.registry.model.registrar.Registrar;
50+
import google.registry.util.PasswordUtils;
5051
import google.registry.util.StopwatchLogger;
5152
import jakarta.inject.Inject;
5253
import java.util.Optional;
@@ -150,8 +151,19 @@ private EppResponse runWithoutLogging() throws EppException {
150151
throw new RegistrarAccountNotActiveException();
151152
}
152153

153-
if (login.getNewPassword().isPresent()) {
154-
String newPassword = login.getNewPassword().get();
154+
// TODO(b/458423787): Remove this circa March 2026 after enough time has passed for the logins
155+
// to have transitioned to Argon2 hashing.
156+
if (login.getNewPassword().isPresent()
157+
|| registrar.get().getCurrentHashAlgorithm(login.getPassword()).orElse(null)
158+
!= PasswordUtils.HashAlgorithm.ARGON_2_ID) {
159+
String newPassword =
160+
login
161+
.getNewPassword()
162+
.orElseGet(
163+
() -> {
164+
logger.atInfo().log("Rehashing existing registrar password with ARGON_2_ID");
165+
return login.getPassword();
166+
});
155167
// Load fresh from database (bypassing the cache) to ensure we don't save stale data.
156168
Optional<Registrar> freshRegistrar = Registrar.loadByRegistrarId(login.getClientId());
157169
stopwatch.tick("LoginFlow reload freshRegistrar");

core/src/main/java/google/registry/model/console/User.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import google.registry.tools.ServiceConnection;
3838
import google.registry.tools.server.UpdateUserGroupAction;
3939
import google.registry.util.PasswordUtils;
40+
import google.registry.util.PasswordUtils.HashAlgorithm;
4041
import google.registry.util.RegistryEnvironment;
4142
import jakarta.persistence.Column;
4243
import jakarta.persistence.Embeddable;
@@ -229,6 +230,10 @@ public boolean verifyRegistryLockPassword(String registryLockPassword) {
229230
|| isNullOrEmpty(registryLockPasswordHash)) {
230231
return false;
231232
}
233+
return getCurrentHashAlgorithm(registryLockPassword).isPresent();
234+
}
235+
236+
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String registryLockPassword) {
232237
return PasswordUtils.verifyPassword(
233238
registryLockPassword, registryLockPasswordHash, registryLockPasswordSalt);
234239
}

core/src/main/java/google/registry/model/registrar/Registrar.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import google.registry.persistence.transaction.TransactionManager;
6868
import google.registry.util.CidrAddressBlock;
6969
import google.registry.util.PasswordUtils;
70+
import google.registry.util.PasswordUtils.HashAlgorithm;
7071
import jakarta.mail.internet.AddressException;
7172
import jakarta.mail.internet.InternetAddress;
7273
import jakarta.persistence.AttributeOverride;
@@ -672,6 +673,10 @@ private static String checkValidPhoneNumber(String phoneNumber) {
672673
}
673674

674675
public boolean verifyPassword(String password) {
676+
return getCurrentHashAlgorithm(password).isPresent();
677+
}
678+
679+
public Optional<HashAlgorithm> getCurrentHashAlgorithm(String password) {
675680
return PasswordUtils.verifyPassword(password, passwordHash, salt);
676681
}
677682

core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static jakarta.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
2424

2525
import com.google.common.collect.ImmutableList;
26+
import com.google.common.flogger.FluentLogger;
2627
import com.google.gson.annotations.Expose;
2728
import google.registry.flows.EppException;
2829
import google.registry.flows.domain.DomainFlowUtils;
@@ -40,6 +41,7 @@
4041
import google.registry.request.auth.Auth;
4142
import google.registry.tools.DomainLockUtils;
4243
import google.registry.util.EmailMessage;
44+
import google.registry.util.PasswordUtils.HashAlgorithm;
4345
import jakarta.inject.Inject;
4446
import jakarta.mail.internet.AddressException;
4547
import jakarta.mail.internet.InternetAddress;
@@ -61,13 +63,16 @@
6163
auth = Auth.AUTH_PUBLIC_LOGGED_IN)
6264
public class ConsoleRegistryLockAction extends ConsoleApiAction {
6365

66+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
67+
6468
static final String PATH = "/console-api/registry-lock";
6569
static final String VERIFICATION_EMAIL_TEMPLATE =
6670
"""
6771
Please click the link below to perform the lock / unlock action on domain %s. Note: this\
6872
code will expire in one hour.
6973
70-
%s""";
74+
%s\
75+
""";
7176

7277
private final DomainLockUtils domainLockUtils;
7378
private final GmailClient gmailClient;
@@ -114,7 +119,6 @@ protected void postHandler(User user) {
114119
optionalPostInput.orElseThrow(() -> new IllegalArgumentException("No POST input provided"));
115120
String domainName = postInput.domainName();
116121
boolean isLock = postInput.isLock();
117-
Optional<String> maybePassword = Optional.ofNullable(postInput.password());
118122
Optional<Long> relockDurationMillis = Optional.ofNullable(postInput.relockDurationMillis());
119123

120124
try {
@@ -126,11 +130,25 @@ protected void postHandler(User user) {
126130
// Passwords aren't required for admin users, otherwise we need to validate it
127131
boolean isAdmin = user.getUserRoles().isAdmin();
128132
if (!isAdmin) {
129-
checkArgument(maybePassword.isPresent(), "No password provided");
130-
if (!user.verifyRegistryLockPassword(maybePassword.get())) {
133+
checkArgument(postInput.password != null, "No password provided");
134+
Optional<HashAlgorithm> hashAlgorithm = user.getCurrentHashAlgorithm(postInput.password);
135+
if (hashAlgorithm.isEmpty()) {
131136
setFailedResponse("Incorrect registry lock password", SC_UNAUTHORIZED);
132137
return;
133138
}
139+
// TODO(b/458423787): Remove this circa March 2026 after enough time has passed for the logins
140+
// to have transitioned to Argon2 hashing.
141+
if (hashAlgorithm.get() != HashAlgorithm.ARGON_2_ID) {
142+
logger.atInfo().log("Rehashing existing registry lock password with ARGON_2_ID.");
143+
tm().transact(
144+
() ->
145+
tm().update(
146+
tm().loadByEntity(user)
147+
.asBuilder()
148+
.removeRegistryLockPassword()
149+
.setRegistryLockPassword(postInput.password)
150+
.build()));
151+
}
134152
}
135153

136154
String registryLockEmail =

core/src/test/java/google/registry/flows/session/LoginFlowTestCase.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
package google.registry.flows.session;
1616

17+
import static com.google.common.io.BaseEncoding.base64;
1718
import static com.google.common.truth.Truth.assertThat;
19+
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
1820
import static google.registry.testing.DatabaseHelper.deleteResource;
1921
import static google.registry.testing.DatabaseHelper.loadRegistrar;
2022
import static google.registry.testing.DatabaseHelper.persistResource;
@@ -38,6 +40,8 @@
3840
import google.registry.model.registrar.Registrar;
3941
import google.registry.model.registrar.Registrar.State;
4042
import google.registry.testing.DatabaseHelper;
43+
import google.registry.util.PasswordUtils;
44+
import google.registry.util.PasswordUtils.HashAlgorithm;
4145
import org.junit.jupiter.api.BeforeEach;
4246
import org.junit.jupiter.api.Test;
4347

@@ -186,6 +190,32 @@ void testFailure_disabledRegistrar() {
186190
doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class);
187191
}
188192

193+
@Test
194+
void testSuccess_scryptPasswordToArgon2() throws Exception {
195+
String password = "foo-BAR2";
196+
tm().transact(
197+
() -> {
198+
// The salt is not exposed by Registrar (nor should it be), so we query it directly.
199+
String encodedSalt =
200+
tm().query("SELECT salt FROM Registrar WHERE registrarId = :id", String.class)
201+
.setParameter("id", registrar.getRegistrarId())
202+
.getSingleResult();
203+
byte[] salt = base64().decode(encodedSalt);
204+
String newHash = PasswordUtils.hashPassword(password, salt, HashAlgorithm.SCRYPT_P_1);
205+
// Set password directly, as the Java method would have used Argon2.
206+
tm().query("UPDATE Registrar SET passwordHash = :hash WHERE registrarId = :id")
207+
.setParameter("id", registrar.getRegistrarId())
208+
.setParameter("hash", newHash)
209+
.executeUpdate();
210+
});
211+
assertThat(loadRegistrar("NewRegistrar").getCurrentHashAlgorithm(password).get())
212+
.isEqualTo(HashAlgorithm.SCRYPT_P_1);
213+
doSuccessfulTest("login_valid.xml");
214+
// Verifies that after successfully login, the password is re-hased with Scrypt.
215+
assertThat(loadRegistrar("NewRegistrar").getCurrentHashAlgorithm(password).get())
216+
.isEqualTo(HashAlgorithm.ARGON_2_ID);
217+
}
218+
189219
@Test
190220
void testFailure_incorrectPassword() {
191221
persistResource(getRegistrarBuilder().setPassword("diff password").build());

core/src/test/java/google/registry/ui/server/console/ConsoleRegistryLockActionTest.java

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
package google.registry.ui.server.console;
1616

17+
import static com.google.common.io.BaseEncoding.base64;
1718
import static com.google.common.truth.Truth.assertThat;
19+
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
1820
import static google.registry.testing.DatabaseHelper.createTld;
1921
import static google.registry.testing.DatabaseHelper.loadByEntity;
2022
import static google.registry.testing.DatabaseHelper.loadRegistrar;
@@ -49,6 +51,8 @@
4951
import google.registry.testing.FakeResponse;
5052
import google.registry.tools.DomainLockUtils;
5153
import google.registry.util.EmailMessage;
54+
import google.registry.util.PasswordUtils;
55+
import google.registry.util.PasswordUtils.HashAlgorithm;
5256
import google.registry.util.StringGenerator;
5357
import jakarta.mail.internet.InternetAddress;
5458
import java.io.IOException;
@@ -88,16 +92,17 @@ void beforeEach() throws Exception {
8892
createTld("test");
8993
defaultDomain = persistActiveDomain("example.test");
9094
user =
91-
new User.Builder()
92-
.setEmailAddress("[email protected]")
93-
.setRegistryLockEmailAddress("[email protected]")
94-
.setUserRoles(
95-
new UserRoles.Builder()
96-
.setRegistrarRoles(
97-
ImmutableMap.of("TheRegistrar", RegistrarRole.PRIMARY_CONTACT))
98-
.build())
99-
.setRegistryLockPassword("registryLockPassword")
100-
.build();
95+
persistResource(
96+
new User.Builder()
97+
.setEmailAddress("[email protected]")
98+
.setRegistryLockEmailAddress("[email protected]")
99+
.setUserRoles(
100+
new UserRoles.Builder()
101+
.setRegistrarRoles(
102+
ImmutableMap.of("TheRegistrar", RegistrarRole.PRIMARY_CONTACT))
103+
.build())
104+
.setRegistryLockPassword("registryLockPassword")
105+
.build());
101106
action = createGetAction();
102107
}
103108

@@ -277,6 +282,40 @@ void testPost_lock() throws Exception {
277282
assertThat(loadByEntity(defaultDomain).getStatusValues()).containsExactly(StatusValue.INACTIVE);
278283
}
279284

285+
@Test
286+
void testPost_lock_scryptPasswordToArgon2() throws Exception {
287+
tm().transact(
288+
() -> {
289+
// The salt is not exposed by User (nor should it be), so we query it directly.
290+
String encodedSalt =
291+
tm().query(
292+
"SELECT registryLockPasswordSalt FROM User WHERE emailAddress ="
293+
+ " :emailAddress",
294+
String.class)
295+
.setParameter("emailAddress", user.getEmailAddress())
296+
.getSingleResult();
297+
byte[] salt = base64().decode(encodedSalt);
298+
String newHash =
299+
PasswordUtils.hashPassword(
300+
"registryLockPassword", salt, HashAlgorithm.SCRYPT_P_1);
301+
// Set password directly, as the Java method would have used Argon2.
302+
tm().query("UPDATE User SET registryLockPasswordHash = :hash")
303+
.setParameter("hash", newHash)
304+
.executeUpdate();
305+
});
306+
user = loadByEntity(user);
307+
assertThat(user.getCurrentHashAlgorithm("registryLockPassword").get())
308+
.isEqualTo(HashAlgorithm.SCRYPT_P_1);
309+
action = createDefaultPostAction(true);
310+
action.run();
311+
verifyEmail();
312+
assertThat(response.getStatus()).isEqualTo(SC_OK);
313+
assertThat(getMostRecentRegistryLockByRepoId(defaultDomain.getRepoId())).isPresent();
314+
user = loadByEntity(user);
315+
assertThat(user.getCurrentHashAlgorithm("registryLockPassword").get())
316+
.isEqualTo(HashAlgorithm.ARGON_2_ID);
317+
}
318+
280319
@Test
281320
void testPost_unlock() throws Exception {
282321
saveRegistryLock(createDefaultLockBuilder().setLockCompletionTime(clock.nowUtc()).build());

0 commit comments

Comments
 (0)