Skip to content

Commit 9dfe51e

Browse files
ravi-signalravi-signal
committed
Forbid linked devices from setting backup-ids
1 parent 5de848b commit 9dfe51e

File tree

6 files changed

+71
-17
lines changed

6 files changed

+71
-17
lines changed

‎service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupAuthManager.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.whispersystems.textsecuregcm.limits.RateLimiters;
3535
import org.whispersystems.textsecuregcm.storage.Account;
3636
import org.whispersystems.textsecuregcm.storage.AccountsManager;
37+
import org.whispersystems.textsecuregcm.storage.Device;
3738
import org.whispersystems.textsecuregcm.storage.RedeemedReceiptsManager;
3839
import org.whispersystems.textsecuregcm.util.Util;
3940

@@ -85,19 +86,25 @@ public BackupAuthManager(
8586
* Store credential requests containing blinded backup-ids for future use.
8687
*
8788
* @param account The account using the backup-id
89+
* @param device The device setting the account backup-id
8890
* @param messagesBackupCredentialRequest A request containing the blinded backup-id the client will use to upload
8991
* message backups
9092
* @param mediaBackupCredentialRequest A request containing the blinded backup-id the client will use to upload
9193
* media backups
9294
* @return A future that completes when the credentialRequest has been stored
9395
* @throws RateLimitExceededException If too many backup-ids have been committed
9496
*/
95-
public CompletableFuture<Void> commitBackupId(final Account account,
97+
public CompletableFuture<Void> commitBackupId(
98+
final Account account,
99+
final Device device,
96100
final BackupAuthCredentialRequest messagesBackupCredentialRequest,
97101
final BackupAuthCredentialRequest mediaBackupCredentialRequest) {
98102
if (configuredBackupLevel(account).isEmpty()) {
99103
throw Status.PERMISSION_DENIED.withDescription("Backups not allowed on account").asRuntimeException();
100104
}
105+
if (!device.isPrimary()) {
106+
throw Status.PERMISSION_DENIED.withDescription("Only primary device can set backup-id").asRuntimeException();
107+
}
101108
final byte[] serializedMessageCredentialRequest = messagesBackupCredentialRequest.serialize();
102109
final byte[] serializedMediaCredentialRequest = mediaBackupCredentialRequest.serialize();
103110

‎service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,14 @@ Set a (blinded) backup-id for the account. Each account may have a single active
135135
""")
136136
@ApiResponse(responseCode = "204", description = "The backup-id was set")
137137
@ApiResponse(responseCode = "400", description = "The provided backup auth credential request was invalid")
138+
@ApiResponse(responseCode = "403", description = "The device did not have permission to set the backup-id. Only the primary device can set the backup-id for an account")
138139
@ApiResponse(responseCode = "429", description = "Rate limited. Too many attempts to change the backup-id have been made")
139140
public CompletionStage<Response> setBackupId(
140141
@Mutable @Auth final AuthenticatedDevice account,
141142
@Valid @NotNull final SetBackupIdRequest setBackupIdRequest) throws RateLimitExceededException {
142-
143143
return this.backupAuthManager
144-
.commitBackupId(account.getAccount(), setBackupIdRequest.messagesBackupAuthCredentialRequest,
144+
.commitBackupId(account.getAccount(), account.getAuthenticatedDevice(),
145+
setBackupIdRequest.messagesBackupAuthCredentialRequest,
145146
setBackupIdRequest.mediaBackupAuthCredentialRequest)
146147
.thenApply(Util.ASYNC_EMPTY_RESPONSE);
147148
}

‎service/src/main/java/org/whispersystems/textsecuregcm/grpc/BackupsGrpcService.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
3333
import org.whispersystems.textsecuregcm.storage.Account;
3434
import org.whispersystems.textsecuregcm.storage.AccountsManager;
35+
import org.whispersystems.textsecuregcm.storage.Device;
3536
import reactor.core.publisher.Mono;
3637

3738
import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
@@ -60,9 +61,15 @@ public Mono<SetBackupIdResponse> setBackupId(SetBackupIdRequest request) {
6061
BackupAuthCredentialRequest::new,
6162
request.getMediaBackupAuthCredentialRequest().toByteArray());
6263

64+
final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice();
6365
return authenticatedAccount()
64-
.flatMap(account -> Mono.fromFuture(
65-
backupAuthManager.commitBackupId(account, messagesCredentialRequest, mediaCredentialRequest)))
66+
.flatMap(account -> {
67+
final Device device = account
68+
.getDevice(authenticatedDevice.deviceId())
69+
.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException);
70+
return Mono.fromFuture(
71+
backupAuthManager.commitBackupId(account, device, messagesCredentialRequest, mediaCredentialRequest));
72+
})
6673
.thenReturn(SetBackupIdResponse.getDefaultInstance());
6774
}
6875

‎service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupAuthManagerTest.java

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import org.whispersystems.textsecuregcm.limits.RateLimiters;
6262
import org.whispersystems.textsecuregcm.storage.Account;
6363
import org.whispersystems.textsecuregcm.storage.AccountsManager;
64+
import org.whispersystems.textsecuregcm.storage.Device;
6465
import org.whispersystems.textsecuregcm.storage.RedeemedReceiptsManager;
6566
import org.whispersystems.textsecuregcm.tests.util.ExperimentHelper;
6667
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
@@ -119,7 +120,7 @@ void commitBackupId() {
119120
final BackupAuthCredentialRequest messagesCredentialRequest = backupAuthTestUtil.getRequest(messagesBackupKey, aci);
120121
final BackupAuthCredentialRequest mediaCredentialRequest = backupAuthTestUtil.getRequest(mediaBackupKey, aci);
121122

122-
authManager.commitBackupId(account, messagesCredentialRequest, mediaCredentialRequest).join();
123+
authManager.commitBackupId(account, primaryDevice(), messagesCredentialRequest, mediaCredentialRequest).join();
123124

124125
verify(account).setBackupCredentialRequests(messagesCredentialRequest.serialize(), mediaCredentialRequest.serialize());
125126
}
@@ -135,6 +136,7 @@ void commitRequiresBackupLevel(final BackupLevel backupLevel) {
135136

136137
final ThrowableAssert.ThrowingCallable commit = () ->
137138
authManager.commitBackupId(account,
139+
primaryDevice(),
138140
backupAuthTestUtil.getRequest(messagesBackupKey, aci),
139141
backupAuthTestUtil.getRequest(mediaBackupKey, aci)).join();
140142
if (backupLevel == null) {
@@ -147,6 +149,24 @@ void commitRequiresBackupLevel(final BackupLevel backupLevel) {
147149
}
148150
}
149151

152+
@Test
153+
void commitRequiresPrimary() {
154+
final BackupAuthManager authManager = create(BackupLevel.FREE);
155+
final Account account = mock(Account.class);
156+
when(account.getUuid()).thenReturn(aci);
157+
when(accountsManager.updateAsync(any(), any())).thenReturn(CompletableFuture.completedFuture(account));
158+
159+
final ThrowableAssert.ThrowingCallable commit = () ->
160+
authManager.commitBackupId(account,
161+
linkedDevice(),
162+
backupAuthTestUtil.getRequest(messagesBackupKey, aci),
163+
backupAuthTestUtil.getRequest(mediaBackupKey, aci)).join();
164+
assertThatExceptionOfType(StatusRuntimeException.class)
165+
.isThrownBy(commit)
166+
.extracting(ex -> ex.getStatus().getCode())
167+
.isEqualTo(Status.Code.PERMISSION_DENIED);
168+
}
169+
150170
@CartesianTest
151171
void getBackupAuthCredentials(@CartesianTest.Enum final BackupLevel backupLevel,
152172
@CartesianTest.Enum final BackupCredentialType credentialType) {
@@ -504,7 +524,7 @@ void testChangeIdRateLimits(
504524
: storedMediaCredential;
505525

506526
final boolean expectRateLimit = (changeMedia || changeMessage) && rateLimitBackupId;
507-
final CompletableFuture<Void> future = authManager.commitBackupId(account, newMessagesCredential, newMediaCredential);
527+
final CompletableFuture<Void> future = authManager.commitBackupId(account, primaryDevice(), newMessagesCredential, newMediaCredential);
508528
if (expectRateLimit) {
509529
CompletableFutureTestUtil.assertFailsWithCause(RateLimitExceededException.class, future);
510530
} else {
@@ -538,7 +558,7 @@ void testChangePaidMediaIdRateLimits(
538558

539559
// We should get rate limited iff we are out of paid media changes and we changed the media backup-id
540560
final boolean expectRateLimit = changeMedia && paid && rateLimitPaidMedia;
541-
final CompletableFuture<Void> future = authManager.commitBackupId(account, newMessagesCredential, newMediaCredential);
561+
final CompletableFuture<Void> future = authManager.commitBackupId(account, primaryDevice(), newMessagesCredential, newMediaCredential);
542562
if (expectRateLimit) {
543563
CompletableFutureTestUtil.assertFailsWithCause(RateLimitExceededException.class, future);
544564
} else {
@@ -562,6 +582,17 @@ private Account mockAccount(final BackupAuthCredentialRequest storedMessagesCred
562582
return account;
563583
}
564584

585+
private Device primaryDevice() {
586+
final Device device = mock(Device.class);
587+
when(device.isPrimary()).thenReturn(true);
588+
return device;
589+
}
590+
591+
private Device linkedDevice() {
592+
final Device device = mock(Device.class);
593+
when(device.isPrimary()).thenReturn(false);
594+
return device;
595+
}
565596

566597
private static String experimentName(@Nullable BackupLevel backupLevel) {
567598
return switch (backupLevel) {

‎service/src/test/java/org/whispersystems/textsecuregcm/controllers/ArchiveControllerTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public void anonymousAuthOnly(final String method, final String path, final Stri
157157

158158
@Test
159159
public void setBackupId() {
160-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenReturn(CompletableFuture.completedFuture(null));
160+
when(backupAuthManager.commitBackupId(any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(null));
161161

162162
final Response response = resources.getJerseyTest()
163163
.target("v1/archives/backupid")
@@ -170,7 +170,7 @@ public void setBackupId() {
170170

171171
assertThat(response.getStatus()).isEqualTo(204);
172172

173-
verify(backupAuthManager).commitBackupId(AuthHelper.VALID_ACCOUNT,
173+
verify(backupAuthManager).commitBackupId(AuthHelper.VALID_ACCOUNT, AuthHelper.VALID_DEVICE,
174174
backupAuthTestUtil.getRequest(messagesBackupKey, aci),
175175
backupAuthTestUtil.getRequest(mediaBackupKey, aci));
176176
}
@@ -275,9 +275,9 @@ public static Stream<Arguments> setBackupIdException() {
275275
@MethodSource
276276
public void setBackupIdException(final Exception ex, final boolean sync, final int expectedStatus) {
277277
if (sync) {
278-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenThrow(ex);
278+
when(backupAuthManager.commitBackupId(any(), any(), any(), any())).thenThrow(ex);
279279
} else {
280-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenReturn(CompletableFuture.failedFuture(ex));
280+
when(backupAuthManager.commitBackupId(any(), any(), any(), any())).thenReturn(CompletableFuture.failedFuture(ex));
281281
}
282282
final Response response = resources.getJerseyTest()
283283
.target("v1/archives/backupid")

‎service/src/test/java/org/whispersystems/textsecuregcm/grpc/BackupsGrpcServiceTest.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.whispersystems.textsecuregcm.metrics.BackupMetrics;
5858
import org.whispersystems.textsecuregcm.storage.Account;
5959
import org.whispersystems.textsecuregcm.storage.AccountsManager;
60+
import org.whispersystems.textsecuregcm.storage.Device;
6061
import org.whispersystems.textsecuregcm.util.EnumMapUtil;
6162
import org.whispersystems.textsecuregcm.util.TestRandomUtil;
6263

@@ -69,7 +70,8 @@ class BackupsGrpcServiceTest extends SimpleBaseGrpcTest<BackupsGrpcService, Back
6970
backupAuthTestUtil.getRequest(mediaBackupKey, AUTHENTICATED_ACI);
7071
final BackupAuthCredentialRequest messagesAuthCredRequest =
7172
backupAuthTestUtil.getRequest(messagesBackupKey, AUTHENTICATED_ACI);
72-
private final Account account = mock(Account.class);
73+
private Account account;
74+
private Device device;
7375

7476
@Mock
7577
private BackupAuthManager backupAuthManager;
@@ -83,22 +85,27 @@ protected BackupsGrpcService createServiceBeforeEachTest() {
8385

8486
@BeforeEach
8587
void setup() {
88+
account = mock(Account.class);
89+
device = mock(Device.class);
90+
when(device.isPrimary()).thenReturn(true);
8691
when(accountsManager.getByAccountIdentifierAsync(AUTHENTICATED_ACI))
8792
.thenReturn(CompletableFuture.completedFuture(Optional.of(account)));
93+
when(account.getDevice(AUTHENTICATED_DEVICE_ID)).thenReturn(Optional.of(device));
8894
}
8995

9096

9197
@Test
9298
void setBackupId() {
93-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenReturn(CompletableFuture.completedFuture(null));
99+
when(backupAuthManager.commitBackupId(any(), any(), any(), any()))
100+
.thenReturn(CompletableFuture.completedFuture(null));
94101

95102
authenticatedServiceStub().setBackupId(
96103
SetBackupIdRequest.newBuilder()
97104
.setMediaBackupAuthCredentialRequest(ByteString.copyFrom(mediaAuthCredRequest.serialize()))
98105
.setMessagesBackupAuthCredentialRequest(ByteString.copyFrom(messagesAuthCredRequest.serialize()))
99106
.build());
100107

101-
verify(backupAuthManager).commitBackupId(account, messagesAuthCredRequest, mediaAuthCredRequest);
108+
verify(backupAuthManager).commitBackupId(account, device, messagesAuthCredRequest, mediaAuthCredRequest);
102109
}
103110

104111
@Test
@@ -147,9 +154,10 @@ public static Stream<Arguments> setBackupIdException() {
147154
@MethodSource
148155
void setBackupIdException(final Exception ex, final boolean sync, final Status expected) {
149156
if (sync) {
150-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenThrow(ex);
157+
when(backupAuthManager.commitBackupId(any(), any(), any(), any())).thenThrow(ex);
151158
} else {
152-
when(backupAuthManager.commitBackupId(any(), any(), any())).thenReturn(CompletableFuture.failedFuture(ex));
159+
when(backupAuthManager.commitBackupId(any(), any(), any(), any()))
160+
.thenReturn(CompletableFuture.failedFuture(ex));
153161
}
154162

155163
GrpcTestUtils.assertStatusException(

0 commit comments

Comments
 (0)