Update authentication to use HKDF_SHA256.

This commit is contained in:
gram-signal 2022-08-29 14:20:47 -06:00 committed by GitHub
parent cb6cc39679
commit 08db4ba54b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 134 additions and 25 deletions

View File

@ -477,8 +477,8 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
ReportedMessageMetricsListener reportedMessageMetricsListener = new ReportedMessageMetricsListener(accountsManager);
reportMessageManager.addListener(reportedMessageMetricsListener);
AccountAuthenticator accountAuthenticator = new AccountAuthenticator(accountsManager);
DisabledPermittedAccountAuthenticator disabledPermittedAccountAuthenticator = new DisabledPermittedAccountAuthenticator(accountsManager);
AccountAuthenticator accountAuthenticator = new AccountAuthenticator(accountsManager, experimentEnrollmentManager);
DisabledPermittedAccountAuthenticator disabledPermittedAccountAuthenticator = new DisabledPermittedAccountAuthenticator(accountsManager, experimentEnrollmentManager);
TwilioSmsSender twilioSmsSender = new TwilioSmsSender(config.getTwilioConfiguration(), dynamicConfigurationManager);
SmsSender smsSender = new SmsSender(twilioSmsSender);

View File

@ -7,13 +7,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 AccountAuthenticator extends BaseAccountAuthenticator implements
Authenticator<BasicCredentials, AuthenticatedAccount> {
public AccountAuthenticator(AccountsManager accountsManager) {
super(accountsManager);
public AccountAuthenticator(AccountsManager accountsManager, ExperimentEnrollmentManager enrollmentManager) {
super(accountsManager, enrollmentManager);
}
@Override

View File

@ -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);
}
}

View File

@ -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<String, Long> 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;
}
}

View File

@ -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<BasicCredentials, DisabledPermittedAuthenticatedAccount> {
public DisabledPermittedAccountAuthenticator(AccountsManager accountsManager) {
super(accountsManager);
public DisabledPermittedAccountAuthenticator(AccountsManager accountsManager, ExperimentEnrollmentManager enrollmentManager) {
super(accountsManager, enrollmentManager);
}
@Override

View File

@ -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<Device>() {
@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

View File

@ -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<AuthenticatedAccount> 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<AuthenticatedAccount> 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<AuthenticatedAccount> 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<AuthenticatedAccount> 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<AuthenticatedAccount> 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";

View File

@ -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

View File

@ -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

View File

@ -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 -> {});
});
}

View File

@ -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<BasicCredentials, AuthenticatedAccount> accountAuthFilter = new BasicCredentialAuthFilter.Builder<AuthenticatedAccount>().setAuthenticator(
new AccountAuthenticator(ACCOUNTS_MANAGER)).buildAuthFilter();
new AccountAuthenticator(ACCOUNTS_MANAGER, experimentEnrollmentManager)).buildAuthFilter();
AuthFilter<BasicCredentials, DisabledPermittedAuthenticatedAccount> disabledPermittedAccountAuthFilter = new BasicCredentialAuthFilter.Builder<DisabledPermittedAuthenticatedAccount>().setAuthenticator(
new DisabledPermittedAccountAuthenticator(ACCOUNTS_MANAGER)).buildAuthFilter();
new DisabledPermittedAccountAuthenticator(ACCOUNTS_MANAGER, experimentEnrollmentManager)).buildAuthFilter();
return new PolymorphicAuthDynamicFeature<>(ImmutableMap.of(AuthenticatedAccount.class, accountAuthFilter,
DisabledPermittedAuthenticatedAccount.class, disabledPermittedAccountAuthFilter));