From 67343f6bdc66364120bc03de27362f9422f2ea03 Mon Sep 17 00:00:00 2001 From: Jonathan Klabunde Tomer <125505367+jkt-signal@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:54:11 -0700 Subject: [PATCH] accept encrypted username with confirm-username-hash requests --- .../org/signal/integration/AccountTest.java | 6 ++- .../controllers/AccountController.java | 18 +++---- .../entities/ConfirmUsernameHashRequest.java | 13 ++++- .../entities/UsernameHashResponse.java | 11 +++- .../textsecuregcm/storage/Accounts.java | 31 +++++++++--- .../storage/AccountsManager.java | 6 ++- .../workers/AssignUsernameCommand.java | 15 +++++- .../controllers/AccountControllerTest.java | 50 +++++++++++++++---- .../storage/AccountsManagerTest.java | 21 +++++--- ...ccountsManagerUsernameIntegrationTest.java | 20 ++++++-- .../textsecuregcm/storage/AccountsTest.java | 48 +++++++++++------- 11 files changed, 175 insertions(+), 64 deletions(-) diff --git a/integration-tests/src/test/java/org/signal/integration/AccountTest.java b/integration-tests/src/test/java/org/signal/integration/AccountTest.java index 4768ab87c..aecaf18a1 100644 --- a/integration-tests/src/test/java/org/signal/integration/AccountTest.java +++ b/integration-tests/src/test/java/org/signal/integration/AccountTest.java @@ -18,6 +18,7 @@ import org.whispersystems.textsecuregcm.entities.AccountIdentifierResponse; import org.whispersystems.textsecuregcm.entities.AccountIdentityResponse; import org.whispersystems.textsecuregcm.entities.ConfirmUsernameHashRequest; +import org.whispersystems.textsecuregcm.entities.EncryptedUsername; import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashRequest; import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; import org.whispersystems.textsecuregcm.entities.UsernameHashResponse; @@ -87,9 +88,10 @@ private static void verifyFullUsernameLifecycle(final TestUser user) throws Base .orElseThrow(); // confirm a username - final ConfirmUsernameHashRequest confirmUsernameHashRequest = new ConfirmUsernameHashRequest( + final ConfirmUsernameHashRequest confirmUsernameHashRequest = new ConfirmUsernameHashRequest( reservedUsername.getHash(), - reservedUsername.generateProof() + reservedUsername.generateProof(), + new EncryptedUsername("cluck cluck i'm a parrot".getBytes()) ); // try unauthorized Operations diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index fc6c09bde..6f1539e59 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -350,17 +350,15 @@ public UsernameHashResponse confirmUsernameHash( throw new WebApplicationException(Response.status(422).build()); } - // Whenever a valid request for a username change arrives, - // we're making sure to clear username link. This may happen before and username changes are written to the db - // but verifying zk proof means that request itself is valid from the client's perspective - clearUsernameLink(auth.getAccount()); - try { - final Account account = accounts.confirmReservedUsernameHash(auth.getAccount(), confirmRequest.usernameHash()); - return account - .getUsernameHash() - .map(UsernameHashResponse::new) - .orElseThrow(() -> new IllegalStateException("Could not get username after setting")); + final Account account = accounts.confirmReservedUsernameHash( + auth.getAccount(), + confirmRequest.usernameHash(), + Optional.ofNullable(confirmRequest.encryptedUsername()).map(EncryptedUsername::usernameLinkEncryptedValue).orElse(null)); + final UUID linkHandle = account.getUsernameLinkHandle(); + return new UsernameHashResponse( + account.getUsernameHash().orElseThrow(() -> new IllegalStateException("Could not get username after setting")), + linkHandle == null ? null : new UsernameLinkHandle(linkHandle)); } catch (final UsernameReservationNotFoundException e) { throw new WebApplicationException(Status.CONFLICT); } catch (final UsernameHashNotAvailableException e) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java index eb481aa3e..f1c75268e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/ConfirmUsernameHashRequest.java @@ -5,12 +5,18 @@ package org.whispersystems.textsecuregcm.entities; +import javax.validation.Valid; + +import javax.annotation.Nullable; + import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import org.whispersystems.textsecuregcm.controllers.AccountController; import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; import org.whispersystems.textsecuregcm.util.ExactlySize; +import io.swagger.v3.oas.annotations.media.Schema; + public record ConfirmUsernameHashRequest( @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @@ -19,5 +25,10 @@ public record ConfirmUsernameHashRequest( @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) - byte[] zkProof + byte[] zkProof, + + @Schema(description = "The encrypted username to be stored for username links") + @Nullable + @Valid + EncryptedUsername encryptedUsername ) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java index 2a7653181..527a4c9d6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/UsernameHashResponse.java @@ -7,9 +7,12 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import io.swagger.v3.oas.annotations.media.Schema; import org.whispersystems.textsecuregcm.controllers.AccountController; import org.whispersystems.textsecuregcm.util.ByteArrayBase64UrlAdapter; import org.whispersystems.textsecuregcm.util.ExactlySize; + +import javax.annotation.Nullable; import javax.validation.Valid; public record UsernameHashResponse( @@ -17,5 +20,11 @@ public record UsernameHashResponse( @JsonSerialize(using = ByteArrayBase64UrlAdapter.Serializing.class) @JsonDeserialize(using = ByteArrayBase64UrlAdapter.Deserializing.class) @ExactlySize(AccountController.USERNAME_HASH_LENGTH) - byte[] usernameHash + @Schema(description = "The hash of the confirmed username, as supplied in the request") + byte[] usernameHash, + + @Nullable + @Valid + @Schema(description = "A handle that can be included in username links to retrieve the stored encrypted username") + UsernameLinkHandle usernameLinkHandle ) {} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java index c6d7776e0..8018f68da 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -30,6 +30,8 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.util.AsyncTimerUtil; @@ -386,15 +388,18 @@ public void reserveUsernameHash( * @param usernameHash believed to be available * @throws ContestedOptimisticLockException if the account has been updated or the username has taken by someone else */ - public void confirmUsernameHash(final Account account, final byte[] usernameHash) + public void confirmUsernameHash(final Account account, final byte[] usernameHash, @Nullable final byte[] encryptedUsername) throws ContestedOptimisticLockException { final long startNanos = System.nanoTime(); final Optional maybeOriginalUsernameHash = account.getUsernameHash(); final Optional maybeOriginalReservationHash = account.getReservedUsernameHash(); + final Optional maybeOriginalUsernameLinkHandle = Optional.ofNullable(account.getUsernameLinkHandle()); + final Optional maybeOriginalEncryptedUsername = account.getEncryptedUsername(); account.setUsernameHash(usernameHash); account.setReservedUsernameHash(null); + account.setUsernameLinkDetails(encryptedUsername == null ? null : UUID.randomUUID(), encryptedUsername); boolean succeeded = false; @@ -420,21 +425,32 @@ public void confirmUsernameHash(final Account account, final byte[] usernameHash .build()) .build()); + final StringBuilder updateExpr = new StringBuilder("SET #data = :data, #username_hash = :username_hash"); + final Map expressionAttributeValues = new HashMap<>(Map.of( + ":data", AttributeValues.fromByteArray(SystemMapper.jsonMapper().writeValueAsBytes(account)), + ":username_hash", AttributeValues.fromByteArray(usernameHash), + ":version", AttributeValues.fromInt(account.getVersion()), + ":version_increment", AttributeValues.fromInt(1))); + if (account.getUsernameLinkHandle() != null) { + updateExpr.append(", #ul = :ul"); + expressionAttributeValues.put(":ul", AttributeValues.fromUUID(account.getUsernameLinkHandle())); + } else { + updateExpr.append(" REMOVE #ul"); + } + updateExpr.append(" ADD #version :version_increment"); + writeItems.add( TransactWriteItem.builder() .update(Update.builder() .tableName(accountsTableName) .key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) - .updateExpression("SET #data = :data, #username_hash = :username_hash ADD #version :version_increment") + .updateExpression(updateExpr.toString()) .conditionExpression("#version = :version") .expressionAttributeNames(Map.of("#data", ATTR_ACCOUNT_DATA, "#username_hash", ATTR_USERNAME_HASH, + "#ul", ATTR_USERNAME_LINK_UUID, "#version", ATTR_VERSION)) - .expressionAttributeValues(Map.of( - ":data", AttributeValues.fromByteArray(SystemMapper.jsonMapper().writeValueAsBytes(account)), - ":username_hash", AttributeValues.fromByteArray(usernameHash), - ":version", AttributeValues.fromInt(account.getVersion()), - ":version_increment", AttributeValues.fromInt(1))) + .expressionAttributeValues(expressionAttributeValues) .build()) .build()); @@ -460,6 +476,7 @@ public void confirmUsernameHash(final Account account, final byte[] usernameHash if (!succeeded) { account.setUsernameHash(maybeOriginalUsernameHash.orElse(null)); account.setReservedUsernameHash(maybeOriginalReservationHash.orElse(null)); + account.setUsernameLinkDetails(maybeOriginalUsernameLinkHandle.orElse(null), maybeOriginalEncryptedUsername.orElse(null)); } SET_USERNAME_TIMER.record(System.nanoTime() - startNanos, TimeUnit.NANOSECONDS); } 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 267874a2b..27899b94b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -503,11 +503,12 @@ public void persistAccount(final Account account) throws UsernameHashNotAvailabl * * @param account the account to update * @param reservedUsernameHash the previously reserved username hash + * @param encryptedUsername the encrypted form of the previously reserved username for the username link * @return the updated account with the username hash field set * @throws UsernameHashNotAvailableException if the reserved username hash has been taken (because the reservation expired) * @throws UsernameReservationNotFoundException if `reservedUsernameHash` was not reserved in the account */ - public Account confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash) throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { + public Account confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash, @Nullable final byte[] encryptedUsername) throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { if (!experimentEnrollmentManager.isEnrolled(account.getUuid(), USERNAME_EXPERIMENT_NAME)) { throw new UsernameHashNotAvailableException(); } @@ -532,7 +533,7 @@ public Account confirmReservedUsernameHash(final Account account, final byte[] r if (!accounts.usernameHashAvailable(Optional.of(account.getUuid()), reservedUsernameHash)) { throw new UsernameHashNotAvailableException(); } - accounts.confirmUsernameHash(a, reservedUsernameHash); + accounts.confirmUsernameHash(a, reservedUsernameHash, encryptedUsername); }, () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); @@ -731,6 +732,7 @@ private static Account cloneAccount(final Account account) { try { final Account clone = mapper.readValue(mapper.writeValueAsBytes(account), Account.class); clone.setUuid(account.getUuid()); + clone.setUsernameLinkHandle(account.getUsernameLinkHandle()); return clone; } catch (final IOException e) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java index a909c5b8a..a8c0d569e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/AssignUsernameCommand.java @@ -82,6 +82,12 @@ public void configure(Subparser subparser) { .required(true) .help("The username hash to assign"); + subparser.addArgument("-e", "--encryptedUsername") + .dest("encryptedUsername") + .type(String.class) + .required(false) + .help("The encrypted username for the username link"); + subparser.addArgument("-a", "--aci") .dest("aci") .type(String.class) @@ -210,14 +216,19 @@ protected void run(Environment environment, Namespace namespace, experimentEnrollmentManager, registrationRecoveryPasswordsManager, Clock.systemUTC()); final String usernameHash = namespace.getString("usernameHash"); + final String encryptedUsername = namespace.getString("encryptedUsername"); final UUID accountIdentifier = UUID.fromString(namespace.getString("aci")); accountsManager.getByAccountIdentifier(accountIdentifier).ifPresentOrElse(account -> { try { final AccountsManager.UsernameReservation reservation = accountsManager.reserveUsernameHash(account, List.of(Base64.getUrlDecoder().decode(usernameHash))); - final Account result = accountsManager.confirmReservedUsernameHash(account, Base64.getUrlDecoder().decode(usernameHash)); - System.out.println("New username hash: " + usernameHash); + final Account result = accountsManager.confirmReservedUsernameHash( + account, + reservation.reservedUsernameHash(), + encryptedUsername == null ? null : Base64.getUrlDecoder().decode(encryptedUsername)); + System.out.println("New username hash: " + Base64.getUrlEncoder().encodeToString(result.getUsernameHash().orElseThrow())); + System.out.println("New username link handle: " + result.getUsernameLinkHandle().toString()); } catch (final UsernameHashNotAvailableException e) { throw new IllegalArgumentException("Username hash already taken"); } catch (final UsernameReservationNotFoundException e) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java index f921dac88..539915b70 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -8,6 +8,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.anyLong; @@ -102,11 +103,15 @@ class AccountControllerTest { private static final String SENDER_TRANSFER = "+14151111112"; private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_1 = "md1votbj9r794DsqTNrBqA"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_2 = "9hrqVLy59bzgPse-S9NUsA"; private static final String INVALID_BASE_64_URL_USERNAME_HASH = "fA+VkNbvB6dVfx/6NpaRSK6mvhhAUBgDNWFaD7+7gvs="; private static final String TOO_SHORT_BASE_64_URL_USERNAME_HASH = "P2oMuxx0xgGxSpTO0ACq3IztEOBDaV9t9YFu4bAGpQ"; private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); + private static final byte[] ENCRYPTED_USERNAME_1 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_1); + private static final byte[] ENCRYPTED_USERNAME_2 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_2); private static final byte[] INVALID_USERNAME_HASH = Base64.getDecoder().decode(INVALID_BASE_64_URL_USERNAME_HASH); private static final byte[] TOO_SHORT_USERNAME_HASH = Base64.getUrlDecoder().decode(TOO_SHORT_BASE_64_URL_USERNAME_HASH); private static final String BASE_64_URL_ZK_PROOF = "2kambOgmdeeIO0faCMgR6HR4G2BQ5bnhXdIe9ZuZY0NmQXSra5BzDBQ7jzy1cvoEqUHYLpBYMrXudkYPJaWoQg"; @@ -625,30 +630,57 @@ void testReserveUsernameHashInvalidBase64UrlEncoding() { void testConfirmUsernameHash() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException, BaseUsernameException { Account account = mock(Account.class); + final UUID uuid = UUID.randomUUID(); when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))).thenReturn(account); + when(account.getUsernameLinkHandle()).thenReturn(uuid); + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), eq(ENCRYPTED_USERNAME_1))).thenReturn(account); Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") .request() .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF, new EncryptedUsername(ENCRYPTED_USERNAME_1)))); assertThat(response.getStatus()).isEqualTo(200); - assertArrayEquals(response.readEntity(UsernameHashResponse.class).usernameHash(), USERNAME_HASH_1); + + final UsernameHashResponse respEntity = response.readEntity(UsernameHashResponse.class); + assertArrayEquals(respEntity.usernameHash(), USERNAME_HASH_1); + assertEquals(respEntity.usernameLinkHandle().usernameLinkHandle(), uuid); + verify(usernameZkProofVerifier).verifyProof(ZK_PROOF, USERNAME_HASH_1); + } + + @Test + void testConfirmUsernameHashOld() + throws UsernameHashNotAvailableException, UsernameReservationNotFoundException, BaseUsernameException { + Account account = mock(Account.class); + final UUID uuid = UUID.randomUUID(); + when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); + when(account.getUsernameLinkHandle()).thenReturn(null); + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), eq(null))).thenReturn(account); + Response response = + resources.getJerseyTest() + .target("/v1/accounts/username_hash/confirm") + .request() + .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF, null))); + assertThat(response.getStatus()).isEqualTo(200); + + final UsernameHashResponse respEntity = response.readEntity(UsernameHashResponse.class); + assertArrayEquals(respEntity.usernameHash(), USERNAME_HASH_1); + assertNull(respEntity.usernameLinkHandle()); verify(usernameZkProofVerifier).verifyProof(ZK_PROOF, USERNAME_HASH_1); } @Test void testConfirmUnreservedUsernameHash() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException, BaseUsernameException { - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), any())) .thenThrow(new UsernameReservationNotFoundException()); Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") .request() .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF, new EncryptedUsername(ENCRYPTED_USERNAME_1)))); assertThat(response.getStatus()).isEqualTo(409); verify(usernameZkProofVerifier).verifyProof(ZK_PROOF, USERNAME_HASH_1); } @@ -656,14 +688,14 @@ void testConfirmUnreservedUsernameHash() @Test void testConfirmLapsedUsernameHash() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException, BaseUsernameException { - when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1))) + when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), any())) .thenThrow(new UsernameHashNotAvailableException()); Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") .request() .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF, new EncryptedUsername(ENCRYPTED_USERNAME_1)))); assertThat(response.getStatus()).isEqualTo(410); verify(usernameZkProofVerifier).verifyProof(ZK_PROOF, USERNAME_HASH_1); } @@ -695,7 +727,7 @@ void testConfirmUsernameHashInvalidHashSize() { .target("/v1/accounts/username_hash/confirm") .request() .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(usernameHash, ZK_PROOF))); + .put(Entity.json(new ConfirmUsernameHashRequest(usernameHash, ZK_PROOF, new EncryptedUsername(ENCRYPTED_USERNAME_1)))); assertThat(response.getStatus()).isEqualTo(422); verifyNoInteractions(usernameZkProofVerifier); } @@ -708,7 +740,7 @@ void testCommitUsernameHashWithInvalidProof() throws BaseUsernameException { .target("/v1/accounts/username_hash/confirm") .request() .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF))); + .put(Entity.json(new ConfirmUsernameHashRequest(USERNAME_HASH_1, ZK_PROOF, new EncryptedUsername(ENCRYPTED_USERNAME_1)))); assertThat(response.getStatus()).isEqualTo(422); verify(usernameZkProofVerifier).verifyProof(ZK_PROOF, USERNAME_HASH_1); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index 76c1fd908..d4b00a8d7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -76,8 +76,13 @@ class AccountsManagerTest { private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_1 = "md1votbj9r794DsqTNrBqA"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_2 = "9hrqVLy59bzgPse-S9NUsA"; + private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); + private static final byte[] ENCRYPTED_USERNAME_1 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_1); + private static final byte[] ENCRYPTED_USERNAME_2 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_2); private Accounts accounts; private DeletedAccounts deletedAccounts; @@ -1217,8 +1222,8 @@ void testConfirmReservedUsernameHash() throws UsernameHashNotAvailableException, final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); setReservationHash(account, USERNAME_HASH_1); when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(true); - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1); - verify(accounts).confirmUsernameHash(eq(account), eq(USERNAME_HASH_1)); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); + verify(accounts).confirmUsernameHash(eq(account), eq(USERNAME_HASH_1), eq(ENCRYPTED_USERNAME_1)); } @Test @@ -1227,7 +1232,7 @@ void testConfirmReservedHashNameMismatch() { setReservationHash(account, USERNAME_HASH_1); when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(true); assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2)); + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2)); } @Test @@ -1237,8 +1242,8 @@ void testConfirmReservedLapsed() { setReservationHash(account, USERNAME_HASH_1); when(accounts.usernameHashAvailable(eq(Optional.of(account.getUuid())), eq(USERNAME_HASH_1))).thenReturn(false); assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.confirmReservedUsernameHash(account, - USERNAME_HASH_1)); - verify(accounts, never()).confirmUsernameHash(any(), any()); + USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + verify(accounts, never()).confirmUsernameHash(any(), any(), any()); } @Test @@ -1247,7 +1252,7 @@ void testConfirmReservedRetry() throws UsernameHashNotAvailableException, Userna account.setUsernameHash(USERNAME_HASH_1); // reserved username already set, should be treated as a replay - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); verifyNoInteractions(accounts); } @@ -1256,8 +1261,8 @@ void testConfirmReservedUsernameHashWithNoReservation() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[16]); assertThrows(UsernameReservationNotFoundException.class, - () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1)); - verify(accounts, never()).confirmUsernameHash(any(), any()); + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + verify(accounts, never()).confirmUsernameHash(any(), any(), any()); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java index 05e94ca4a..9f157269d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -52,9 +52,13 @@ class AccountsManagerUsernameIntegrationTest { private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_1 = "md1votbj9r794DsqTNrBqA"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_2 = "9hrqVLy59bzgPse-S9NUsA"; private static final int SCAN_PAGE_SIZE = 1; private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); + private static final byte[] ENCRYPTED_USERNAME_1 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_1); + private static final byte[] ENCRYPTED_USERNAME_2 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_2); @RegisterExtension static final DynamoDbExtension DYNAMO_DB_EXTENSION = new DynamoDbExtension( @@ -203,10 +207,14 @@ public void testReserveConfirmClear() // confirm account = accountsManager.confirmReservedUsernameHash( reservation.account(), - reservation.reservedUsernameHash()); + reservation.reservedUsernameHash(), + ENCRYPTED_USERNAME_1); assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).orElseThrow().getUuid()).isEqualTo( account.getUuid()); + assertThat(account.getUsernameLinkHandle()).isNotNull(); + assertThat(accountsManager.getByUsernameLinkHandle(account.getUsernameLinkHandle()).orElseThrow().getUuid()) + .isEqualTo(account.getUuid()); // clear account = accountsManager.clearUsernameHash(account); @@ -241,8 +249,8 @@ public void testReservationLapsed() assertArrayEquals(reservation2.reservedUsernameHash(), USERNAME_HASH_1); assertThrows(UsernameHashNotAvailableException.class, - () -> accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1)); - account2 = accountsManager.confirmReservedUsernameHash(reservation2.account(), USERNAME_HASH_1); + () -> accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + account2 = accountsManager.confirmReservedUsernameHash(reservation2.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertEquals(accountsManager.getByUsernameHash(USERNAME_HASH_1).orElseThrow().getUuid(), account2.getUuid()); assertArrayEquals(account2.getUsernameHash().orElseThrow(), USERNAME_HASH_1); } @@ -256,7 +264,7 @@ void testUsernameSetReserveAnotherClearSetReserved() // Set username hash final AccountsManager.UsernameReservation reservation1 = accountsManager.reserveUsernameHash(account, List.of( USERNAME_HASH_1)); - account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1); + account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1); // Reserve another hash on the same account final AccountsManager.UsernameReservation reservation2 = accountsManager.reserveUsernameHash(account, List.of( @@ -265,6 +273,7 @@ void testUsernameSetReserveAnotherClearSetReserved() assertArrayEquals(account.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_2); assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); + assertArrayEquals(account.getEncryptedUsername().orElseThrow(), ENCRYPTED_USERNAME_1); // Clear the set username hash but not the reserved one account = accountsManager.clearUsernameHash(account); @@ -272,7 +281,8 @@ void testUsernameSetReserveAnotherClearSetReserved() assertThat(account.getUsernameHash()).isEmpty(); // Confirm second reservation - account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash()); + account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash(), ENCRYPTED_USERNAME_2); assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_2); + assertArrayEquals(account.getEncryptedUsername().orElseThrow(), ENCRYPTED_USERNAME_2); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java index 83e396ab2..c50445f0d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -79,8 +79,12 @@ class AccountsTest { private static final String BASE_64_URL_USERNAME_HASH_1 = "9p6Tip7BFefFOJzv4kv4GyXEYsBVfk_WbjNejdlOvQE"; private static final String BASE_64_URL_USERNAME_HASH_2 = "NLUom-CHwtemcdvOTTXdmXmzRIV7F05leS8lwkVK_vc"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_1 = "md1votbj9r794DsqTNrBqA"; + private static final String BASE_64_URL_ENCRYPTED_USERNAME_2 = "9hrqVLy59bzgPse-S9NUsA"; private static final byte[] USERNAME_HASH_1 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_1); private static final byte[] USERNAME_HASH_2 = Base64.getUrlDecoder().decode(BASE_64_URL_USERNAME_HASH_2); + private static final byte[] ENCRYPTED_USERNAME_1 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_1); + private static final byte[] ENCRYPTED_USERNAME_2 = Base64.getUrlDecoder().decode(BASE_64_URL_ENCRYPTED_USERNAME_2); private static final int SCAN_PAGE_SIZE = 1; @@ -350,9 +354,11 @@ void testOverwrite() { final SecureRandom byteGenerator = new SecureRandom(); final byte[] usernameHash = new byte[32]; byteGenerator.nextBytes(usernameHash); + final byte[] encryptedUsername = new byte[16]; + byteGenerator.nextBytes(encryptedUsername); // Set up the existing account to have a username hash - accounts.confirmUsernameHash(account, usernameHash); + accounts.confirmUsernameHash(account, usernameHash, encryptedUsername); verifyStoredState("+14151112222", account.getUuid(), account.getPhoneNumberIdentifier(), usernameHash, account, true); @@ -738,16 +744,20 @@ void testSwitchUsernameHashes() { assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); - + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); + final UUID oldHandle = account.getUsernameLinkHandle(); + { final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), USERNAME_HASH_1, maybeAccount.orElseThrow(), account); + final Optional maybeAccount2 = accounts.getByUsernameLinkHandle(oldHandle); + verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), USERNAME_HASH_1, maybeAccount2.orElseThrow(), account); } accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_2); + accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2); + final UUID newHandle = account.getUsernameLinkHandle(); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); assertThat(DYNAMO_DB_EXTENSION.getDynamoDbClient() @@ -756,6 +766,7 @@ void testSwitchUsernameHashes() { .key(Map.of(Accounts.ATTR_USERNAME_HASH, AttributeValues.fromByteArray(USERNAME_HASH_1))) .build()) .item()).isEmpty(); + assertThat(accounts.getByUsernameLinkHandle(oldHandle)).isEmpty(); { final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_2); @@ -763,6 +774,9 @@ void testSwitchUsernameHashes() { assertThat(maybeAccount).isPresent(); verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), USERNAME_HASH_2, maybeAccount.get(), account); + final Optional maybeAccount2 = accounts.getByUsernameLinkHandle(newHandle); + verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), + USERNAME_HASH_2, maybeAccount2.get(), account); } } @@ -777,7 +791,7 @@ void testUsernameHashConflict() { // first account reserves and confirms username hash assertThatNoException().isThrownBy(() -> { accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1); + accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); }); final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1); @@ -789,13 +803,13 @@ void testUsernameHashConflict() { assertThatExceptionOfType(ContestedOptimisticLockException.class) .isThrownBy(() -> accounts.reserveUsernameHash(secondAccount, USERNAME_HASH_1, Duration.ofDays(1))); assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1)); + .isThrownBy(() -> accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); // throw an error if first account tries to reserve or confirm the username hash that it has already confirmed assertThatExceptionOfType(ContestedOptimisticLockException.class) .isThrownBy(() -> accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1))); assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1)); + .isThrownBy(() -> accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(secondAccount.getReservedUsernameHash()).isEmpty(); assertThat(secondAccount.getUsernameHash()).isEmpty(); @@ -809,7 +823,7 @@ void testConfirmUsernameHashVersionMismatch() { account.setVersion(account.getVersion() + 77); assertThatExceptionOfType(ContestedOptimisticLockException.class) - .isThrownBy(() -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); + .isThrownBy(() -> accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(account.getUsernameHash()).isEmpty(); } @@ -820,7 +834,7 @@ void testClearUsername() { accounts.create(account); accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isPresent(); accounts.clearUsernameHash(account); @@ -844,7 +858,7 @@ void testClearUsernameVersionMismatch() { accounts.create(account); accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); - accounts.confirmUsernameHash(account, USERNAME_HASH_1); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); account.setVersion(account.getVersion() + 12); @@ -868,10 +882,10 @@ void testReservedUsernameHash() { assertThrows(ContestedOptimisticLockException.class, () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1)).isEmpty(); - accounts.confirmUsernameHash(account1, USERNAME_HASH_1); + accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(account1.getReservedUsernameHash()).isEmpty(); assertArrayEquals(account1.getUsernameHash().orElseThrow(), USERNAME_HASH_1); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account1.getUuid()); @@ -898,7 +912,7 @@ void testUsernameHashAvailable() { assertThat(accounts.usernameHashAvailable(Optional.of(UUID.randomUUID()), USERNAME_HASH_1)).isFalse(); assertThat(accounts.usernameHashAvailable(Optional.of(account1.getUuid()), USERNAME_HASH_1)).isTrue(); - accounts.confirmUsernameHash(account1, USERNAME_HASH_1); + accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(accounts.usernameHashAvailable(USERNAME_HASH_1)).isFalse(); assertThat(accounts.usernameHashAvailable(Optional.empty(), USERNAME_HASH_1)).isFalse(); assertThat(accounts.usernameHashAvailable(Optional.of(UUID.randomUUID()), USERNAME_HASH_1)).isFalse(); @@ -918,7 +932,7 @@ void testConfirmReservedUsernameHashWrongAccountUuid() { // only account1 should be able to confirm the reserved hash assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1)); + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); } @Test @@ -942,12 +956,12 @@ void testConfirmExpiredReservedUsernameHash() { runnable.run(); assertEquals(account2.getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); - accounts.confirmUsernameHash(account2, USERNAME_HASH_1); + accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThrows(ContestedOptimisticLockException.class, () -> accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account1, USERNAME_HASH_1)); + () -> accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).get().getUuid()).isEqualTo(account2.getUuid()); } @@ -970,7 +984,7 @@ void testReserveConfirmUsernameHashVersionConflict() { assertThrows(ContestedOptimisticLockException.class, () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); assertThrows(ContestedOptimisticLockException.class, - () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1)); + () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(account.getReservedUsernameHash()).isEmpty(); assertThat(account.getUsernameHash()).isEmpty(); }