From d3d68c2a602a0190d29002289500c93087372cc6 Mon Sep 17 00:00:00 2001 From: Katherine Yen Date: Thu, 9 Jan 2025 10:58:28 -0500 Subject: [PATCH] Enforce one entry position per identifier in monitor request --- .../KeyTransparencyController.java | 6 +- .../KeyTransparencyMonitorRequest.java | 23 ++--- .../main/proto/KeyTransparencyService.proto | 6 +- .../KeyTransparencyControllerTest.java | 92 ++++++++----------- 4 files changed, 54 insertions(+), 73 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyController.java index d47215458..2f1ac344f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyController.java @@ -151,21 +151,21 @@ public class KeyTransparencyController { try { final AciMonitorRequest aciMonitorRequest = AciMonitorRequest.newBuilder() .setAci(ByteString.copyFrom(request.aci().value().toCompactByteArray())) - .addAllEntries(request.aci().positions()) + .setEntryPosition(request.aci().entry_position()) .setCommitmentIndex(ByteString.copyFrom(request.aci().commitmentIndex())) .build(); final Optional usernameHashMonitorRequest = request.usernameHash().map(usernameHash -> UsernameHashMonitorRequest.newBuilder() .setUsernameHash(ByteString.copyFrom(usernameHash.value())) - .addAllEntries(usernameHash.positions()) + .setEntryPosition(usernameHash.entry_position()) .setCommitmentIndex(ByteString.copyFrom(usernameHash.commitmentIndex())) .build()); final Optional e164MonitorRequest = request.e164().map(e164 -> E164MonitorRequest.newBuilder() .setE164(e164.value()) - .addAllEntries(e164.positions()) + .setEntryPosition(e164.entry_position()) .setCommitmentIndex(ByteString.copyFrom(e164.commitmentIndex())) .build()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/KeyTransparencyMonitorRequest.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/KeyTransparencyMonitorRequest.java index 107de23cd..bc8771295 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/KeyTransparencyMonitorRequest.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/KeyTransparencyMonitorRequest.java @@ -49,11 +49,9 @@ public record KeyTransparencyMonitorRequest( @Schema(description = "The aci identifier to monitor") AciServiceIdentifier value, - @Schema(description = "A list of log tree positions maintained by the client for the aci search key.") - @Valid - @NotNull - @NotEmpty - List<@Positive Long> positions, + @Schema(description = "A log tree position maintained by the client for the aci.") + @Positive + long entry_position, @Schema(description = "The commitment index derived from a previous search request, encoded in standard unpadded base64") @JsonSerialize(using = ByteArrayAdapter.Serializing.class) @@ -68,11 +66,9 @@ public record KeyTransparencyMonitorRequest( @NotBlank String value, - @Schema(description = "A list of log tree positions maintained by the client for the e164 search key.") - @NotNull - @NotEmpty - @Valid - List<@Positive Long> positions, + @Schema(description = "A log tree position maintained by the client for the e164.") + @Positive + long entry_position, @Schema(description = "The commitment index derived from a previous search request, encoded in standard unpadded base64") @JsonSerialize(using = ByteArrayAdapter.Serializing.class) @@ -91,10 +87,9 @@ public record KeyTransparencyMonitorRequest( @NotEmpty byte[] value, - @Schema(description = "A list of log tree positions maintained by the client for the username hash search key.") - @NotNull - @NotEmpty - @Valid List<@Positive Long> positions, + @Schema(description = "A log tree position maintained by the client for the username hash.") + @Positive + long entry_position, @Schema(description = "The commitment index derived from a previous search request, encoded in standard unpadded base64") @JsonSerialize(using = ByteArrayAdapter.Serializing.class) diff --git a/service/src/main/proto/KeyTransparencyService.proto b/service/src/main/proto/KeyTransparencyService.proto index 3e1bd5d36..3ce17aa9e 100644 --- a/service/src/main/proto/KeyTransparencyService.proto +++ b/service/src/main/proto/KeyTransparencyService.proto @@ -295,19 +295,19 @@ message MonitorRequest { message AciMonitorRequest { bytes aci = 1; - repeated uint64 entries = 2; + uint64 entry_position = 2; bytes commitment_index = 3; } message UsernameHashMonitorRequest { bytes username_hash = 1; - repeated uint64 entries = 2; + uint64 entry_position = 2; bytes commitment_index = 3; } message E164MonitorRequest { string e164 = 1; - repeated uint64 entries = 2; + uint64 entry_position = 2; bytes commitment_index = 3; } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyControllerTest.java index 88c3ff9e1..7dc468b85 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeyTransparencyControllerTest.java @@ -305,7 +305,7 @@ public class KeyTransparencyControllerTest { try (Response response = request.post(Entity.json( createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(3L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI,3, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 4L))))) { assertEquals(200, response.getStatus()); @@ -327,7 +327,7 @@ public class KeyTransparencyControllerTest { try (Response response = request.post( Entity.json(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(3L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 3, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 4L))))) { assertEquals(400, response.getStatus()); verifyNoInteractions(keyTransparencyServiceClient); @@ -346,7 +346,7 @@ public class KeyTransparencyControllerTest { try (Response response = request.post( Entity.json(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(3L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 3, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 4L))))) { assertEquals(httpStatus, response.getStatus()); verify(keyTransparencyServiceClient, times(1)).monitor(any(), any(), any(), anyLong(), anyLong(), any()); @@ -381,115 +381,101 @@ public class KeyTransparencyControllerTest { new KeyTransparencyMonitorRequest(null, Optional.empty(), Optional.empty(), 3L, 4L))), // aci monitor fields can't be null Arguments.of(createRequestJson( - new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(null, null, null), + new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(null, 4, null), Optional.empty(), Optional.empty(), 3L, 4L))), Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(null, List.of(4L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(null, 4, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 4L))), Arguments.of(createRequestJson( - new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, null, COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, null), Optional.empty(), Optional.empty(), 3L, 4L))), - Arguments.of(createRequestJson( - new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), null), - Optional.empty(), Optional.empty(), 3L, 4L))), - // aciPositions list can't be empty + // aciPosition must be positive Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, Collections.emptyList(), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 0, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 4L))), // aci commitment index must be the correct size Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), new byte[0]), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, new byte[0]), Optional.empty(), Optional.empty(), 3L, 4L))), Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, Collections.emptyList(), new byte[33]), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 0, new byte[33]), Optional.empty(), Optional.empty(), 3L, 4L))), // username monitor fields cannot be null Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), - Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(null, null, null)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), + Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(null, 5, null)), 3L, 4L))), Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), - Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(null, List.of(5L), COMMITMENT_INDEX)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), + Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(null, 5, COMMITMENT_INDEX)), 3L, 4L))), Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), - Optional.of( - new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, null, COMMITMENT_INDEX)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), + Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, 5, null)), 3L, 4L))), + // usernameHashPosition must be positive Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), - Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, List.of(5L), null)), - 3L, 4L))), - // usernameHashPositions list cannot be empty - Arguments.of(createRequestJson( - new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, - Collections.emptyList(), COMMITMENT_INDEX)), 3L, 4L))), + 0, COMMITMENT_INDEX)), 3L, 4L))), // username commitment index must be the correct size Arguments.of(createRequestJson( new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), new byte[0]), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, new byte[0]), Optional.empty(), Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, - List.of(5L), new byte[0])), 3L, 4L))), + 5, new byte[0])), 3L, 4L))), Arguments.of(createRequestJson( - new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), null), + new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, null), Optional.empty(), Optional.of(new KeyTransparencyMonitorRequest.UsernameHashMonitor(USERNAME_HASH, - List.of(5L), new byte[33])), 3L, 4L))), + 5, new byte[33])), 3L, 4L))), // e164 fields cannot be null Arguments.of( createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), - Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(null, null, null)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), + Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(null, 5, null)), Optional.empty(), 3L, 4L))), Arguments.of( createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), - Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(null, List.of(5L), COMMITMENT_INDEX)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), + Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(null, 5, COMMITMENT_INDEX)), Optional.empty(), 3L, 4L))), Arguments.of( createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), - Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, null, COMMITMENT_INDEX)), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), + Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, 5, null)), Optional.empty(), 3L, 4L))), - Arguments.of( - createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), - Optional.of(new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, List.of(5L), null)), - Optional.empty(), 3L, 4L))), - // e164Positions list cannot empty + // e164Position must be positive Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.of( - new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, Collections.emptyList(), COMMITMENT_INDEX)), + new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, 0, COMMITMENT_INDEX)), Optional.empty(), 3L, 4L))), // e164 commitment index must be the correct size Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.of( - new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, List.of(5L), new byte[0])), + new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, 5, new byte[0])), Optional.empty(), 3L, 4L))), Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.of( - new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, List.of(5L), new byte[33])), + new KeyTransparencyMonitorRequest.E164Monitor(NUMBER, 5, new byte[33])), Optional.empty(), 3L, 4L))), // lastNonDistinguishedTreeHeadSize must be positive Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 0L, 4L))), // lastDistinguishedTreeHeadSize must be positive Arguments.of(createRequestJson(new KeyTransparencyMonitorRequest( - new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(4L), COMMITMENT_INDEX), Optional.empty(), + new KeyTransparencyMonitorRequest.AciMonitor(ACI, 4, COMMITMENT_INDEX), Optional.empty(), Optional.empty(), 3L, 0L))) ); } @@ -503,7 +489,7 @@ public class KeyTransparencyControllerTest { .request(); try (Response response = request.post( Entity.json(createRequestJson( - new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, List.of(3L), null), + new KeyTransparencyMonitorRequest(new KeyTransparencyMonitorRequest.AciMonitor(ACI, 3, null), Optional.empty(), Optional.empty(), 3L, 4L))))) { assertEquals(429, response.getStatus());