diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 000c32f7f..ae235bd4c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -477,8 +477,8 @@ public class WhisperServerService extends Application { - public AccountAuthenticator(AccountsManager accountsManager) { - super(accountsManager); + public AccountAuthenticator(AccountsManager accountsManager, ExperimentEnrollmentManager enrollmentManager) { + super(accountsManager, enrollmentManager); } @Override diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java index 328ff9168..714e447e8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java @@ -4,7 +4,9 @@ */ package org.whispersystems.textsecuregcm.auth; +import com.google.common.annotations.VisibleForTesting; import org.apache.commons.codec.binary.Hex; +import org.signal.libsignal.protocol.kdf.HKDF; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; @@ -12,10 +14,18 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; public class AuthenticationCredentials { + private static final String V2_PREFIX = "2."; private final String hashedAuthenticationToken; private final String salt; + public enum Version { + V1, + V2, + } + + public static final Version CURRENT_VERSION = Version.V2; + public AuthenticationCredentials(String hashedAuthenticationToken, String salt) { this.hashedAuthenticationToken = hashedAuthenticationToken; this.salt = salt; @@ -23,7 +33,20 @@ public class AuthenticationCredentials { public AuthenticationCredentials(String authenticationToken) { this.salt = String.valueOf(Math.abs(new SecureRandom().nextInt())); - this.hashedAuthenticationToken = getHashedValue(salt, authenticationToken); + this.hashedAuthenticationToken = getV2HashedValue(salt, authenticationToken); + } + + @VisibleForTesting + public AuthenticationCredentials v1ForTesting(String authenticationToken) { + String salt = String.valueOf(Math.abs(new SecureRandom().nextInt())); + return new AuthenticationCredentials(getV1HashedValue(salt, authenticationToken), salt); + } + + public Version getVersion() { + if (this.hashedAuthenticationToken.startsWith(V2_PREFIX)) { + return Version.V2; + } + return Version.V1; } public String getHashedAuthenticationToken() { @@ -35,11 +58,14 @@ public class AuthenticationCredentials { } public boolean verify(String authenticationToken) { - String theirValue = getHashedValue(salt, authenticationToken); + final String theirValue = switch (getVersion()) { + case V1 -> getV1HashedValue(salt, authenticationToken); + case V2 -> getV2HashedValue(salt, authenticationToken); + }; return MessageDigest.isEqual(theirValue.getBytes(StandardCharsets.UTF_8), this.hashedAuthenticationToken.getBytes(StandardCharsets.UTF_8)); } - private static String getHashedValue(String salt, String token) { + private static String getV1HashedValue(String salt, String token) { try { return new String(Hex.encodeHex(MessageDigest.getInstance("SHA1").digest((salt + token).getBytes(StandardCharsets.UTF_8)))); } catch (NoSuchAlgorithmException e) { @@ -47,4 +73,13 @@ public class AuthenticationCredentials { } } + private static final byte[] AUTH_TOKEN_HKDF_INFO = "authtoken".getBytes(StandardCharsets.UTF_8); + private static String getV2HashedValue(String salt, String token) { + byte[] secret = HKDF.deriveSecrets( + token.getBytes(StandardCharsets.UTF_8), // key + salt.getBytes(StandardCharsets.UTF_8), // salt + AUTH_TOKEN_HKDF_INFO, + 32); + return V2_PREFIX + Hex.encodeHexString(secret); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java index f50511bf6..0847bfa7f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java @@ -17,6 +17,7 @@ import java.time.temporal.ChronoUnit; import java.util.Optional; import java.util.UUID; import org.apache.commons.lang3.StringUtils; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -33,18 +34,21 @@ public class BaseAccountAuthenticator { private static final String DAYS_SINCE_LAST_SEEN_DISTRIBUTION_NAME = name(BaseAccountAuthenticator.class, "daysSinceLastSeen"); private static final String IS_PRIMARY_DEVICE_TAG = "isPrimary"; + private static final String AUTH_V2_REWRITE_EXPERIMENT_NAME = "authv2-rewrite"; private final AccountsManager accountsManager; private final Clock clock; + private final ExperimentEnrollmentManager enrollmentManager; - public BaseAccountAuthenticator(AccountsManager accountsManager) { - this(accountsManager, Clock.systemUTC()); + public BaseAccountAuthenticator(AccountsManager accountsManager, ExperimentEnrollmentManager enrollmentManager) { + this(accountsManager, Clock.systemUTC(), enrollmentManager); } @VisibleForTesting - public BaseAccountAuthenticator(AccountsManager accountsManager, Clock clock) { - this.accountsManager = accountsManager; - this.clock = clock; + public BaseAccountAuthenticator(AccountsManager accountsManager, Clock clock, ExperimentEnrollmentManager enrollmentManager) { + this.accountsManager = accountsManager; + this.clock = clock; + this.enrollmentManager = enrollmentManager; } static Pair getIdentifierAndDeviceId(final String basicUsername) { @@ -104,9 +108,17 @@ public class BaseAccountAuthenticator { } } - if (device.get().getAuthenticationCredentials().verify(basicCredentials.getPassword())) { + AuthenticationCredentials deviceAuthenticationCredentials = device.get().getAuthenticationCredentials(); + if (deviceAuthenticationCredentials.verify(basicCredentials.getPassword())) { succeeded = true; - final Account authenticatedAccount = updateLastSeen(account.get(), device.get()); + Account authenticatedAccount = updateLastSeen(account.get(), device.get()); + if (deviceAuthenticationCredentials.getVersion() != AuthenticationCredentials.CURRENT_VERSION + && enrollmentManager.isEnrolled(accountUuid, AUTH_V2_REWRITE_EXPERIMENT_NAME)) { + authenticatedAccount = accountsManager.updateDeviceAuthentication( + authenticatedAccount, + device.get(), + new AuthenticationCredentials(basicCredentials.getPassword())); // new credentials have current version + } return Optional.of(new AuthenticatedAccount( new RefreshingAccountAndDeviceSupplier(authenticatedAccount, device.get().getId(), accountsManager))); } @@ -142,5 +154,4 @@ public class BaseAccountAuthenticator { return account; } - } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/DisabledPermittedAccountAuthenticator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/DisabledPermittedAccountAuthenticator.java index cb7b8b78b..658408435 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/DisabledPermittedAccountAuthenticator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/DisabledPermittedAccountAuthenticator.java @@ -8,13 +8,14 @@ package org.whispersystems.textsecuregcm.auth; import io.dropwizard.auth.Authenticator; import io.dropwizard.auth.basic.BasicCredentials; import java.util.Optional; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.AccountsManager; public class DisabledPermittedAccountAuthenticator extends BaseAccountAuthenticator implements Authenticator { - public DisabledPermittedAccountAuthenticator(AccountsManager accountsManager) { - super(accountsManager); + public DisabledPermittedAccountAuthenticator(AccountsManager accountsManager, ExperimentEnrollmentManager enrollmentManager) { + super(accountsManager, enrollmentManager); } @Override diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index 1faa0ff1e..f50b8d67c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -13,6 +13,7 @@ import com.codahale.metrics.Timer; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import io.micrometer.core.instrument.Metrics; @@ -391,6 +392,16 @@ public class AccountsManager { }); } + public Account updateDeviceAuthentication(final Account account, final Device device, final AuthenticationCredentials credentials) { + Preconditions.checkArgument(credentials.getVersion() == AuthenticationCredentials.CURRENT_VERSION); + return updateDevice(account, device.getId(), new Consumer() { + @Override + public void accept(final Device device) { + device.setAuthenticationCredentials(credentials); + } + }); + } + /** * @param account account to update * @param updater must return {@code true} if the account was actually updated diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticatorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticatorTest.java index a3973f27f..79841a4f0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticatorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticatorTest.java @@ -14,6 +14,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -30,6 +31,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -54,7 +56,9 @@ class BaseAccountAuthenticatorTest { void setup() { accountsManager = mock(AccountsManager.class); clock = mock(Clock.class); - baseAccountAuthenticator = new BaseAccountAuthenticator(accountsManager, clock); + ExperimentEnrollmentManager enrollmentManager = mock(ExperimentEnrollmentManager.class); + when(enrollmentManager.isEnrolled(any(UUID.class), any())).thenReturn(true); + baseAccountAuthenticator = new BaseAccountAuthenticator(accountsManager, clock, enrollmentManager); // We use static UUIDs here because the UUID affects the "date last seen" offset acct1 = AccountsHelper.generateTestAccount("+14088675309", UUID.fromString("c139cb3e-f70c-4460-b221-815e8bdf778f"), UUID.randomUUID(), List.of(generateTestDevice(yesterday)), null); @@ -164,6 +168,7 @@ class BaseAccountAuthenticatorTest { when(device.isEnabled()).thenReturn(true); when(device.getAuthenticationCredentials()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION); final Optional maybeAuthenticatedAccount = baseAccountAuthenticator.authenticate(new BasicCredentials(uuid.toString(), password), true); @@ -171,6 +176,7 @@ class BaseAccountAuthenticatorTest { assertThat(maybeAuthenticatedAccount).isPresent(); assertThat(maybeAuthenticatedAccount.get().getAccount().getUuid()).isEqualTo(uuid); assertThat(maybeAuthenticatedAccount.get().getAuthenticatedDevice()).isEqualTo(device); + verify(accountsManager, never()).updateDeviceAuthentication(any(), any(), any());; } @Test @@ -192,6 +198,7 @@ class BaseAccountAuthenticatorTest { when(device.isEnabled()).thenReturn(true); when(device.getAuthenticationCredentials()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION); final Optional maybeAuthenticatedAccount = baseAccountAuthenticator.authenticate(new BasicCredentials(uuid + "." + deviceId, password), true); @@ -199,6 +206,7 @@ class BaseAccountAuthenticatorTest { assertThat(maybeAuthenticatedAccount).isPresent(); assertThat(maybeAuthenticatedAccount.get().getAccount().getUuid()).isEqualTo(uuid); assertThat(maybeAuthenticatedAccount.get().getAuthenticatedDevice()).isEqualTo(device); + verify(accountsManager, never()).updateDeviceAuthentication(any(), any(), any()); } @ParameterizedTest @@ -221,6 +229,7 @@ class BaseAccountAuthenticatorTest { when(device.isEnabled()).thenReturn(false); when(device.getAuthenticationCredentials()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION); final Optional maybeAuthenticatedAccount = baseAccountAuthenticator.authenticate(new BasicCredentials(uuid.toString(), password), enabledRequired); @@ -234,6 +243,37 @@ class BaseAccountAuthenticatorTest { } } + @Test + void testAuthenticateV1() { + final UUID uuid = UUID.randomUUID(); + final long deviceId = 1; + final String password = "12345"; + + final Account account = mock(Account.class); + final Device device = mock(Device.class); + final AuthenticationCredentials credentials = mock(AuthenticationCredentials.class); + + when(clock.instant()).thenReturn(Instant.now()); + when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account)); + when(account.getUuid()).thenReturn(uuid); + when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); + when(account.isEnabled()).thenReturn(true); + when(device.getId()).thenReturn(deviceId); + when(device.isEnabled()).thenReturn(true); + when(device.getAuthenticationCredentials()).thenReturn(credentials); + when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.Version.V1); + + final Optional maybeAuthenticatedAccount = + baseAccountAuthenticator.authenticate(new BasicCredentials(uuid.toString(), password), true); + + assertThat(maybeAuthenticatedAccount).isPresent(); + assertThat(maybeAuthenticatedAccount.get().getAccount().getUuid()).isEqualTo(uuid); + assertThat(maybeAuthenticatedAccount.get().getAuthenticatedDevice()).isEqualTo(device); + verify(accountsManager, times(1)).updateDeviceAuthentication( + any(), // this won't be 'account', because it'll already be updated by updateDeviceLastSeen + eq(device), any()); + } @Test void testAuthenticateAccountNotFound() { assertThat(baseAccountAuthenticator.authenticate(new BasicCredentials(UUID.randomUUID().toString(), "password"), true)) @@ -259,6 +299,7 @@ class BaseAccountAuthenticatorTest { when(device.isEnabled()).thenReturn(true); when(device.getAuthenticationCredentials()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION); final Optional maybeAuthenticatedAccount = baseAccountAuthenticator.authenticate(new BasicCredentials(uuid + "." + (deviceId + 1), password), true); @@ -286,6 +327,7 @@ class BaseAccountAuthenticatorTest { when(device.isEnabled()).thenReturn(true); when(device.getAuthenticationCredentials()).thenReturn(credentials); when(credentials.verify(password)).thenReturn(true); + when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION); final String incorrectPassword = password + "incorrect"; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/AuthenticationCredentialsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/AuthenticationCredentialsTest.java index b5ca8bb1a..37970fa1b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/AuthenticationCredentialsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/AuthenticationCredentialsTest.java @@ -17,7 +17,7 @@ class AuthenticationCredentialsTest { AuthenticationCredentials credentials = new AuthenticationCredentials("mypassword"); assertThat(credentials.getSalt()).isNotEmpty(); assertThat(credentials.getHashedAuthenticationToken()).isNotEmpty(); - assertThat(credentials.getHashedAuthenticationToken().length()).isEqualTo(40); + assertThat(credentials.getHashedAuthenticationToken().length()).isEqualTo(66); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index 813a5154b..1d177af18 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -1630,7 +1630,7 @@ class AccountControllerTest { assertThat(pinCapture.getValue()).isNotEmpty(); assertThat(pinSaltCapture.getValue()).isNotEmpty(); - assertThat(pinCapture.getValue().length()).isEqualTo(40); + assertThat(pinCapture.getValue().length()).isEqualTo(66); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index e3f99cbd7..e669a48f8 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -20,6 +20,7 @@ import java.util.UUID; import java.util.function.Consumer; import org.mockito.MockingDetails; import org.mockito.stubbing.Stubbing; +import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -69,10 +70,13 @@ public class AccountsHelper { }); when(mockAccountsManager.updateDeviceLastSeen(any(), any(), anyLong())).thenAnswer(answer -> { - answer.getArgument(1, Device.class).setLastSeen(answer.getArgument(2, Long.class)); - return mockAccountsManager.update(answer.getArgument(0, Account.class), account -> { - }); + return mockAccountsManager.update(answer.getArgument(0, Account.class), account -> {}); + }); + + when(mockAccountsManager.updateDeviceAuthentication(any(), any(), any())).thenAnswer(answer -> { + answer.getArgument(1, Device.class).setAuthenticationCredentials(answer.getArgument(2, AuthenticationCredentials.class)); + return mockAccountsManager.update(answer.getArgument(0, Account.class), account -> {}); }); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java index 5c2ee6c71..c219ab112 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AuthHelper.java @@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.tests.util; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; @@ -25,6 +26,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials; import org.whispersystems.textsecuregcm.auth.DisabledPermittedAccountAuthenticator; import org.whispersystems.textsecuregcm.auth.DisabledPermittedAuthenticatedAccount; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -189,10 +191,12 @@ public class AuthHelper { testAccount.setup(ACCOUNTS_MANAGER); } + ExperimentEnrollmentManager experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); + when(experimentEnrollmentManager.isEnrolled(any(UUID.class), any())).thenReturn(true); AuthFilter accountAuthFilter = new BasicCredentialAuthFilter.Builder().setAuthenticator( - new AccountAuthenticator(ACCOUNTS_MANAGER)).buildAuthFilter(); + new AccountAuthenticator(ACCOUNTS_MANAGER, experimentEnrollmentManager)).buildAuthFilter(); AuthFilter disabledPermittedAccountAuthFilter = new BasicCredentialAuthFilter.Builder().setAuthenticator( - new DisabledPermittedAccountAuthenticator(ACCOUNTS_MANAGER)).buildAuthFilter(); + new DisabledPermittedAccountAuthenticator(ACCOUNTS_MANAGER, experimentEnrollmentManager)).buildAuthFilter(); return new PolymorphicAuthDynamicFeature<>(ImmutableMap.of(AuthenticatedAccount.class, accountAuthFilter, DisabledPermittedAuthenticatedAccount.class, disabledPermittedAccountAuthFilter));