From 2c6b646d87053f5cb0447a934193ed20d68f538c Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 4 Sep 2020 18:06:05 -0400 Subject: [PATCH] Enforce no capability downgrade on device verification --- .../controllers/DeviceController.java | 10 ++++++++++ .../entities/AccountAttributes.java | 5 +++-- .../controllers/AccountControllerTest.java | 10 +++++----- .../controllers/DeviceControllerTest.java | 18 +++++++++++++++++- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java index 32bfc1e3d..142f39210 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -190,6 +190,11 @@ public class DeviceController { throw new DeviceLimitExceededException(account.get().getDevices().size(), MAX_DEVICES); } + final DeviceCapabilities capabilities = accountAttributes.getCapabilities(); + if (capabilities != null && isCapabilityDowngrade(account.get(), capabilities)) { + throw new WebApplicationException(Response.status(409).build()); + } + Device device = new Device(); device.setName(accountAttributes.getName()); device.setAuthenticationCredentials(new AuthenticationCredentials(password)); @@ -235,4 +240,9 @@ public class DeviceController { int randomInt = 100000 + random.nextInt(900000); return new VerificationCode(randomInt); } + + private boolean isCapabilityDowngrade(Account account, DeviceCapabilities capabilities) { + return (!capabilities.isGv2() && account.isGroupsV2Supported()) + || (!capabilities.isUuid() && account.isUuidAddressingSupported()); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java index 3581a85d1..d9aee1274 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/AccountAttributes.java @@ -67,11 +67,11 @@ public class AccountAttributes { @VisibleForTesting public AccountAttributes(String signalingKey, boolean fetchesMessages, int registrationId, String pin) { - this(signalingKey, fetchesMessages, registrationId, null, pin, null, null, true); + this(signalingKey, fetchesMessages, registrationId, null, pin, null, null, true, null); } @VisibleForTesting - public AccountAttributes(String signalingKey, boolean fetchesMessages, int registrationId, String name, String pin, String registrationLock, List payments, boolean discoverableByPhoneNumber) { + public AccountAttributes(String signalingKey, boolean fetchesMessages, int registrationId, String name, String pin, String registrationLock, List payments, boolean discoverableByPhoneNumber, final DeviceCapabilities capabilities) { this.signalingKey = signalingKey; this.fetchesMessages = fetchesMessages; this.registrationId = registrationId; @@ -80,6 +80,7 @@ public class AccountAttributes { this.registrationLock = registrationLock; this.payments = payments; this.discoverableByPhoneNumber = discoverableByPhoneNumber; + this.capabilities = capabilities; } public String getSignalingKey() { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index c24a43ac6..4c5b1bae4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -528,7 +528,7 @@ public class AccountControllerTest { .target(String.format("/v1/accounts/code/%s", "1234")) .request() .header("Authorization", AuthHelper.getAuthHeader(SENDER, "bar")) - .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 2222, null, null, null, null, false), + .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 2222, null, null, null, null, false, null), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -611,7 +611,7 @@ public class AccountControllerTest { .target(String.format("/v1/accounts/code/%s", "666666")) .request() .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), null, true), + .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), null, true, null), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -627,7 +627,7 @@ public class AccountControllerTest { .target(String.format("/v1/accounts/code/%s", "666666")) .request() .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), null, true), + .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, Hex.toStringCondensed(registration_lock_key), null, true, null), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -661,7 +661,7 @@ public class AccountControllerTest { .target(String.format("/v1/accounts/code/%s", "666666")) .request() .header("Authorization", AuthHelper.getAuthHeader(SENDER_REG_LOCK, "bar")) - .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, null, null, true), + .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 3333, null, null, null, null, true, null), MediaType.APPLICATION_JSON_TYPE), AccountCreationResult.class); assertThat(result.getUuid()).isNotNull(); @@ -1166,7 +1166,7 @@ public class AccountControllerTest { .target("/v1/accounts/attributes/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) - .put(Entity.json(new AccountAttributes("keykeykeykey", false, 2222, null, null, null, null, true))); + .put(Entity.json(new AccountAttributes("keykeykeykey", false, 2222, null, null, null, null, true, null))); assertThat(response.getStatus()).isEqualTo(204); verify(directoryQueue, times(1)).refreshRegisteredUser(AuthHelper.VALID_ACCOUNT); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java index d1bd4beba..026034516 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java @@ -117,6 +117,8 @@ public class DeviceControllerTest { // when(maxedAccount.getActiveDeviceCount()).thenReturn(6); when(account.getAuthenticatedDevice()).thenReturn(Optional.of(masterDevice)); when(account.isEnabled()).thenReturn(false); + when(account.isUuidAddressingSupported()).thenReturn(true); + when(account.isGroupsV2Supported()).thenReturn(true); when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(new StoredVerificationCode("5678901", System.currentTimeMillis(), null))); when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(new StoredVerificationCode("1112223", System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(31), null))); @@ -213,11 +215,25 @@ public class DeviceControllerTest { .target("/v1/devices/5678901") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, "password1")) - .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 1234, "this is a really long name that is longer than 80 characters it's so long that it's even longer than 204 characters. that's a lot of characters. we're talking lots and lots and lots of characters. 12345678", null, null, null, true), + .put(Entity.entity(new AccountAttributes("keykeykeykey", false, 1234, "this is a really long name that is longer than 80 characters it's so long that it's even longer than 204 characters. that's a lot of characters. we're talking lots and lots and lots of characters. 12345678", null, null, null, true, null), MediaType.APPLICATION_JSON_TYPE)); assertEquals(response.getStatus(), 422); verifyNoMoreInteractions(messagesManager); } + @Test + public void deviceDowngradeCapabilitiesTest() throws Exception { + Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(true, false, true, false); + AccountAttributes accountAttributes = new AccountAttributes("keykeykeykey", false, 1234, null, null, null, null, true, deviceCapabilities); + Response response = resources.getJerseyTest() + .target("/v1/devices/5678901") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, "password1")) + .put(Entity.entity(accountAttributes, MediaType.APPLICATION_JSON_TYPE)); + + assertThat(response.getStatus()).isEqualTo(409); + + verifyNoMoreInteractions(messagesManager); + } }