From a83378a44e19f10a7a96938fa5a1c33c3c82132e Mon Sep 17 00:00:00 2001 From: Jonathan Klabunde Tomer <125505367+jkt-signal@users.noreply.github.com> Date: Mon, 13 Nov 2023 09:01:54 -0800 Subject: [PATCH] add an option to replace username ciphertext without rotating the link handle --- .../controllers/AccountController.java | 16 +++++++++----- .../entities/EncryptedUsername.java | 13 ++++++++++- .../grpc/AccountsGrpcService.java | 7 +++++- .../main/proto/org/signal/chat/account.proto | 12 ++++++++-- .../controllers/AccountControllerTest.java | 22 +++++++++++++++++++ .../grpc/AccountsGrpcServiceTest.java | 9 ++++++-- .../tests/util/AccountsHelper.java | 1 + 7 files changed, 69 insertions(+), 11 deletions(-) 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 65f5b156c..25c893e84 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -410,9 +410,8 @@ public class AccountController { summary = "Set username link", description = """ Authenticated endpoint. For the given encrypted username generates a username link handle. - Username link handle could be used to lookup the encrypted username. - An account can only have one username link at a time. Calling this endpoint will reset previously stored - encrypted username and deactivate previous link handle. + The username link handle can be used to lookup the encrypted username. + An account can only have one username link at a time; this endpoint overwrites the previous encrypted username if there was one. """ ) @ApiResponse(responseCode = "200", description = "Username Link updated successfully.", useReturnTypeSchema = true) @@ -426,12 +425,19 @@ public class AccountController { // check ratelimiter for username link operations rateLimiters.forDescriptor(RateLimiters.For.USERNAME_LINK_OPERATION).validate(auth.getAccount().getUuid()); + final Account account = auth.getAccount(); + // check if username hash is set for the account - if (auth.getAccount().getUsernameHash().isEmpty()) { + if (account.getUsernameHash().isEmpty()) { throw new WebApplicationException(Status.CONFLICT); } - final UUID usernameLinkHandle = UUID.randomUUID(); + final UUID usernameLinkHandle; + if (encryptedUsername.keepLinkHandle() && account.getUsernameLinkHandle() != null) { + usernameLinkHandle = account.getUsernameLinkHandle(); + } else { + usernameLinkHandle = UUID.randomUUID(); + } updateUsernameLink(auth.getAccount(), usernameLinkHandle, encryptedUsername.usernameLinkEncryptedValue()); return new UsernameLinkHandle(usernameLinkHandle); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/EncryptedUsername.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/EncryptedUsername.java index 6e24c2dd4..8a696eb28 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/EncryptedUsername.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/EncryptedUsername.java @@ -5,6 +5,7 @@ package org.whispersystems.textsecuregcm.entities; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import io.swagger.v3.oas.annotations.media.Schema; @@ -18,7 +19,17 @@ public record EncryptedUsername( @NotNull @Size(min = 1, max = EncryptedUsername.MAX_SIZE) @Schema(type = "string", description = "the URL-safe base64 encoding of the encrypted username") - byte[] usernameLinkEncryptedValue) { + byte[] usernameLinkEncryptedValue, + + @JsonProperty + @Schema(type = "boolean", description = "if set and the account already has an encrypted-username link handle, reuse the same link handle rather than generating a new one. The response will still have the link handle.") + boolean keepLinkHandle +) { public static final int MAX_SIZE = 128; + + public EncryptedUsername(final byte[] usernameLinkEncryptedValue) { + this(usernameLinkEncryptedValue, false); + } + } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java index d4221418f..895139a7f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java @@ -267,7 +267,12 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { .asRuntimeException()); } - final UUID linkHandle = UUID.randomUUID(); + final UUID linkHandle; + if (request.getKeepLinkHandle() && account.getUsernameLinkHandle() != null) { + linkHandle = account.getUsernameLinkHandle(); + } else { + linkHandle = UUID.randomUUID(); + } return Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setUsernameLinkDetails(linkHandle, request.getUsernameCiphertext().toByteArray()))) .thenReturn(linkHandle); diff --git a/service/src/main/proto/org/signal/chat/account.proto b/service/src/main/proto/org/signal/chat/account.proto index 32f7df9fc..b2f8d3f47 100644 --- a/service/src/main/proto/org/signal/chat/account.proto +++ b/service/src/main/proto/org/signal/chat/account.proto @@ -61,8 +61,9 @@ service Accounts { rpc DeleteUsernameHash(DeleteUsernameHashRequest) returns (DeleteUsernameHashResponse) {} /** - * Generates a new link handle for the given username ciphertext, displacing - * any previously-existing link handle. + * Associates the given username ciphertext with the account, replacing any + * previously stored ciphertext. A new link handle will optionally be created, + * and the link handle to use will be returned in any event. * * This RPC may fail with a status of `FAILED_PRECONDITION` if the * authenticated account does not have a username. It may also fail with @@ -235,6 +236,13 @@ message SetUsernameLinkRequest { * The username ciphertext for which to generate a new link handle. */ bytes username_ciphertext = 1; + + /** + * If true and the account already had an encrypted username stored, the + * existing link handle will be reused. Otherwise a new link handle will be + * created. + */ + bool keep_link_handle = 2; } message SetUsernameLinkResponse { 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 ba9ba63f3..d39db373c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -52,6 +52,7 @@ import org.junit.jupiter.api.extension.ExtendWith; 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.mockito.ArgumentCaptor; import org.mockito.stubbing.Answer; import org.signal.libsignal.usernames.BaseUsernameException; @@ -71,6 +72,7 @@ import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashRequest; import org.whispersystems.textsecuregcm.entities.ReserveUsernameHashResponse; import org.whispersystems.textsecuregcm.entities.UsernameHashResponse; +import org.whispersystems.textsecuregcm.entities.UsernameLinkHandle; import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; import org.whispersystems.textsecuregcm.identity.PniServiceIdentifier; import org.whispersystems.textsecuregcm.limits.RateLimitByIpFilter; @@ -998,4 +1000,24 @@ class AccountControllerTest { .get() .getStatus()).isEqualTo(422); } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void testPutUsernameLink(boolean keepLink) { + when(rateLimiters.forDescriptor(eq(RateLimiters.For.USERNAME_LINK_OPERATION))).thenReturn(mock(RateLimiter.class)); + + final UUID oldLinkHandle = UUID.randomUUID(); + when(AuthHelper.VALID_ACCOUNT.getUsernameLinkHandle()).thenReturn(oldLinkHandle); + + final byte[] encryptedUsername = "some encrypted goop".getBytes(); + final UsernameLinkHandle newHandle = resources.getJerseyTest() + .target("/v1/accounts/username_link") + .request() + .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .put(Entity.json(new EncryptedUsername(encryptedUsername, keepLink)), UsernameLinkHandle.class); + + assertThat(newHandle.usernameLinkHandle().equals(oldLinkHandle)).isEqualTo(keepLink); + verify(AuthHelper.VALID_ACCOUNT).setUsernameLinkDetails(eq(newHandle.usernameLinkHandle()), eq(encryptedUsername)); + } + } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java index c4580385f..44b7fb8ac 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java @@ -521,10 +521,13 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest linkHandleCaptor = ArgumentCaptor.forClass(UUID.class); verify(account).setUsernameLinkDetails(linkHandleCaptor.capture(), eq(usernameCiphertext)); + assertEquals(keepLink, oldHandle.equals(linkHandleCaptor.getValue())); final SetUsernameLinkResponse expectedResponse = SetUsernameLinkResponse.newBuilder() .setUsernameLinkHandle(UUIDUtil.toByteString(linkHandleCaptor.getValue())) .build(); 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 33b9160e3..1efdbd6d0 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 @@ -116,6 +116,7 @@ public class AccountsHelper { case "getNumber" -> when(updatedAccount.getNumber()).thenAnswer(stubbing); case "getUsername" -> when(updatedAccount.getUsernameHash()).thenAnswer(stubbing); case "getUsernameHash" -> when(updatedAccount.getUsernameHash()).thenAnswer(stubbing); + case "getUsernameLinkHandle" -> when(updatedAccount.getUsernameLinkHandle()).thenAnswer(stubbing); case "getDevices" -> when(updatedAccount.getDevices()).thenAnswer(stubbing); case "getDevice" -> when(updatedAccount.getDevice(stubbing.getInvocation().getArgument(0))).thenAnswer(stubbing); case "getPrimaryDevice" -> when(updatedAccount.getPrimaryDevice()).thenAnswer(stubbing);