From 417d48c452221d3bbfbfc9ecb2ffc147bdd8f8cd Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Mon, 24 May 2021 22:53:30 -0500 Subject: [PATCH] Block downgrading sender key support Disallow linking an additional device to an account that has already upgraded to having sender key support where the linked device does not have sender key support. This should prompt the person attempting to link the older application to upgrade in order to complete the linking process. --- .../controllers/DeviceController.java | 4 ++ .../controllers/DeviceControllerTest.java | 35 +++++++++-- .../tests/storage/AccountTest.java | 62 ++++++++++++++----- 3 files changed, 79 insertions(+), 22 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 98b4314c9..a0cd2358d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -234,6 +234,10 @@ public class DeviceController { private boolean isCapabilityDowngrade(Account account, DeviceCapabilities capabilities, String userAgent) { boolean isDowngrade = false; + if (account.isSenderKeySupported() && !capabilities.isSenderKey()) { + isDowngrade = true; + } + if (account.isGv1MigrationSupported() && !capabilities.isGv1Migration()) { isDowngrade = true; } 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 c30631ba0..af871d26c 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 @@ -42,6 +42,7 @@ import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.PendingDevicesManager; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; @@ -114,6 +115,7 @@ public class DeviceControllerTest { when(account.isEnabled()).thenReturn(false); when(account.isGroupsV2Supported()).thenReturn(true); when(account.isGv1MigrationSupported()).thenReturn(true); + when(account.isSenderKeySupported()).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))); @@ -221,8 +223,7 @@ public class DeviceControllerTest { @Test @Parameters(method = "argumentsForDeviceDowngradeCapabilitiesTest") public void deviceDowngradeCapabilitiesTest(final String userAgent, final boolean gv2, final boolean gv2_2, final boolean gv2_3, final int expectedStatus) throws Exception { - Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(gv2, gv2_2, gv2_3, true, false, true, - false); + DeviceCapabilities deviceCapabilities = new DeviceCapabilities(gv2, gv2_2, gv2_3, true, false, true, true); AccountAttributes accountAttributes = new AccountAttributes(false, 1234, null, null, null, true, deviceCapabilities); Response response = resources.getJerseyTest() .target("/v1/devices/5678901") @@ -262,8 +263,7 @@ public class DeviceControllerTest { @Test public void deviceDowngradeGv1MigrationTest() { - Device.DeviceCapabilities deviceCapabilities = new Device.DeviceCapabilities(true, true, true, true, false, false, - false); + DeviceCapabilities deviceCapabilities = new DeviceCapabilities(true, true, true, true, false, false, true); AccountAttributes accountAttributes = new AccountAttributes(false, 1234, null, null, null, true, deviceCapabilities); Response response = resources.getJerseyTest() .target("/v1/devices/5678901") @@ -274,7 +274,7 @@ public class DeviceControllerTest { assertThat(response.getStatus()).isEqualTo(409); - deviceCapabilities = new Device.DeviceCapabilities(true, true, true, true, false, true, false); + deviceCapabilities = new DeviceCapabilities(true, true, true, true, false, true, true); accountAttributes = new AccountAttributes(false, 1234, null, null, null, true, deviceCapabilities); response = resources.getJerseyTest() .target("/v1/devices/5678901") @@ -284,6 +284,31 @@ public class DeviceControllerTest { .put(Entity.entity(accountAttributes, MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatus()).isEqualTo(200); + } + @Test + public void deviceDowngradeSenderKeyTest() { + DeviceCapabilities deviceCapabilities = new DeviceCapabilities(true, true, true, true, true, true, false); + AccountAttributes accountAttributes = + new AccountAttributes(false, 1234, null, null, null, true, deviceCapabilities); + Response response = resources + .getJerseyTest() + .target("/v1/devices/5678901") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .header("User-Agent", "Signal-Android/5.42.8675309 Android/30") + .put(Entity.entity(accountAttributes, MediaType.APPLICATION_JSON_TYPE)); + assertThat(response.getStatus()).isEqualTo(409); + + deviceCapabilities = new DeviceCapabilities(true, true, true, true, true, true, true); + accountAttributes = new AccountAttributes(false, 1234, null, null, null, true, deviceCapabilities); + response = resources + .getJerseyTest() + .target("/v1/devices/5678901") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .header("User-Agent", "Signal-Android/5.42.8675309 Android/30") + .put(Entity.entity(accountAttributes, MediaType.APPLICATION_JSON_TYPE)); + assertThat(response.getStatus()).isEqualTo(200); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountTest.java index 9956576fb..ebdc3659e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountTest.java @@ -5,10 +5,11 @@ package org.whispersystems.textsecuregcm.tests.storage; -import org.junit.Before; -import org.junit.Test; -import org.whispersystems.textsecuregcm.storage.Account; -import org.whispersystems.textsecuregcm.storage.Device; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.nio.charset.StandardCharsets; import java.util.Collections; @@ -16,11 +17,11 @@ import java.util.HashSet; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import org.junit.Before; +import org.junit.Test; +import org.whispersystems.textsecuregcm.storage.Account; +import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities; public class AccountTest { @@ -38,6 +39,10 @@ public class AccountTest { private final Device gv1MigrationIncapableDevice = mock(Device.class); private final Device gv1MigrationIncapableExpiredDevice = mock(Device.class); + private final Device senderKeyCapableDevice = mock(Device.class); + private final Device senderKeyIncapableDevice = mock(Device.class); + private final Device senderKeyIncapableExpiredDevice = mock(Device.class); + @Before public void setup() { when(oldMasterDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(366)); @@ -72,17 +77,29 @@ public class AccountTest { when(gv2IncapableExpiredDevice.getLastSeen()).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31)); when(gv2IncapableExpiredDevice.isEnabled()).thenReturn(false); - when(gv1MigrationCapableDevice.getCapabilities()).thenReturn(new Device.DeviceCapabilities(true, true, true, true, true, true, - false)); + when(gv1MigrationCapableDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, true, false)); when(gv1MigrationCapableDevice.isEnabled()).thenReturn(true); - when(gv1MigrationIncapableDevice.getCapabilities()).thenReturn(new Device.DeviceCapabilities(true, true, true, true, true, false, - false)); + when(gv1MigrationIncapableDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, false, false)); when(gv1MigrationIncapableDevice.isEnabled()).thenReturn(true); - when(gv1MigrationIncapableExpiredDevice.getCapabilities()).thenReturn(new Device.DeviceCapabilities(true, true, true, true, true, false, - false)); + when(gv1MigrationIncapableExpiredDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, false, false)); when(gv1MigrationIncapableExpiredDevice.isEnabled()).thenReturn(false); + + when(senderKeyCapableDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, true, true)); + when(senderKeyCapableDevice.isEnabled()).thenReturn(true); + + when(senderKeyIncapableDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, true, false)); + when(senderKeyIncapableDevice.isEnabled()).thenReturn(true); + + when(senderKeyIncapableExpiredDevice.getCapabilities()).thenReturn( + new DeviceCapabilities(true, true, true, true, true, true, false)); + when(senderKeyIncapableExpiredDevice.isEnabled()).thenReturn(false); } @Test @@ -137,8 +154,8 @@ public class AccountTest { final Device nonTransferCapableMasterDevice = mock(Device.class); final Device transferCapableLinkedDevice = mock(Device.class); - final Device.DeviceCapabilities transferCapabilities = mock(Device.DeviceCapabilities.class); - final Device.DeviceCapabilities nonTransferCapabilities = mock(Device.DeviceCapabilities.class); + final DeviceCapabilities transferCapabilities = mock(DeviceCapabilities.class); + final DeviceCapabilities nonTransferCapabilities = mock(DeviceCapabilities.class); when(transferCapableMasterDevice.getId()).thenReturn(1L); when(transferCapableMasterDevice.isMaster()).thenReturn(true); @@ -205,4 +222,15 @@ public class AccountTest { assertFalse(new Account("+18005551234", UUID.randomUUID(), Set.of(gv1MigrationCapableDevice, gv1MigrationIncapableDevice), "1234".getBytes(StandardCharsets.UTF_8)).isGv1MigrationSupported()); assertTrue(new Account("+18005551234", UUID.randomUUID(), Set.of(gv1MigrationCapableDevice, gv1MigrationIncapableExpiredDevice), "1234".getBytes(StandardCharsets.UTF_8)).isGv1MigrationSupported()); } + + @Test + public void isSenderKeySupported() { + assertThat(new Account("+18005551234", UUID.randomUUID(), Set.of(senderKeyCapableDevice), + "1234".getBytes(StandardCharsets.UTF_8)).isSenderKeySupported()).isTrue(); + assertThat(new Account("+18005551234", UUID.randomUUID(), Set.of(senderKeyCapableDevice, senderKeyIncapableDevice), + "1234".getBytes(StandardCharsets.UTF_8)).isSenderKeySupported()).isFalse(); + assertThat(new Account("+18005551234", UUID.randomUUID(), + Set.of(senderKeyCapableDevice, senderKeyIncapableExpiredDevice), + "1234".getBytes(StandardCharsets.UTF_8)).isSenderKeySupported()).isTrue(); + } }