diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/CertificateGenerator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/CertificateGenerator.java index 6e376d2f9..caa1cd0a2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/CertificateGenerator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/CertificateGenerator.java @@ -35,6 +35,10 @@ public class CertificateGenerator { } public byte[] createFor(final Account account, final byte deviceId, boolean includeE164) { + if (includeE164 && account.getNumberOptional().isEmpty()) { + throw new IllegalArgumentException(); + } + SenderCertificate.Certificate.Builder builder = SenderCertificate.Certificate.newBuilder() .setSenderDevice(Math.toIntExact(deviceId)) .setExpires(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(expiresDays)) @@ -42,7 +46,7 @@ public class CertificateGenerator { .setSenderUuid(UUIDUtil.toByteString(account.getAccountIdentifier())); if (includeE164) { - builder.setSenderE164(account.getNumber()); + builder.setSenderE164(account.getNumberOptional().get()); } if (embedSigner) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/CertificateController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/CertificateController.java index ebe9f6ab5..9a9234a7b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/CertificateController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/CertificateController.java @@ -82,8 +82,11 @@ public class CertificateController { final Account account = accountsManager.getByAccountIdentifier(auth.accountIdentifier()) .orElseThrow(() -> new WebApplicationException(Response.Status.UNAUTHORIZED)); - return new DeliveryCertificate( - certificateGenerator.createFor(account, auth.deviceId(), includeE164)); + try { + return new DeliveryCertificate(certificateGenerator.createFor(account, auth.deviceId(), includeE164)); + } catch (final IllegalArgumentException _) { + throw new BadRequestException(); + } } @GET diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcService.java index ced3bfb7e..e3db95581 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcService.java @@ -93,12 +93,16 @@ public class CredentialsGrpcService extends SimpleCredentialsGrpc.CredentialsImp final Account account = accountsManager.getByAccountIdentifier(authenticatedDevice.accountIdentifier()) .orElseThrow(() -> GrpcExceptions.invalidCredentials("invalid credentials")); - return GetDeliveryCertificateResponse.newBuilder() - .setCertificateWithE164(ByteString.copyFrom( - certificateGenerator.createFor(account, authenticatedDevice.deviceId(), true))) + final GetDeliveryCertificateResponse.Builder responseBuilder = GetDeliveryCertificateResponse.newBuilder() .setCertificateWithoutE164(ByteString.copyFrom( - certificateGenerator.createFor(account, authenticatedDevice.deviceId(), false))) - .build(); + certificateGenerator.createFor(account, authenticatedDevice.deviceId(), false))); + + if (account.getNumberOptional().isPresent()) { + responseBuilder.setCertificateWithE164(ByteString.copyFrom( + certificateGenerator.createFor(account, authenticatedDevice.deviceId(), true))); + } + + return responseBuilder.build(); } @Override diff --git a/service/src/main/proto/org/signal/chat/credentials.proto b/service/src/main/proto/org/signal/chat/credentials.proto index 644f7a514..a1568c8b8 100644 --- a/service/src/main/proto/org/signal/chat/credentials.proto +++ b/service/src/main/proto/org/signal/chat/credentials.proto @@ -109,7 +109,8 @@ message GetDeliveryCertificateRequest { // server never learns anything about the caller's intent to share their phone // number with their contacts. message GetDeliveryCertificateResponse { - // A delivery receipt that includes the caller's phone number + // A delivery receipt that includes the caller's phone number; may be empty if + // the authenticated account does not have a phone number bytes certificate_with_e164 = 1; // A delivery receipt that does not include the caller's phone number diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/CertificateGeneratorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/CertificateGeneratorTest.java index e4598219f..e496665d5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/CertificateGeneratorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/CertificateGeneratorTest.java @@ -6,17 +6,21 @@ package org.whispersystems.textsecuregcm.auth; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.i18n.phonenumbers.PhoneNumberUtil; +import com.google.protobuf.InvalidProtocolBufferException; import java.io.IOException; import java.util.Base64; +import java.util.Optional; import java.util.UUID; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.api.function.Executable; import org.junitpioneer.jupiter.cartesian.CartesianTest; import org.signal.libsignal.protocol.IdentityKey; import org.signal.libsignal.protocol.ecc.ECPrivateKey; @@ -49,7 +53,6 @@ class CertificateGeneratorTest { } @CartesianTest - @ValueSource(booleans = {true, false}) void testCreateFor(@CartesianTest.Values(booleans = {true, false}) boolean includeE164, @CartesianTest.Values(booleans = {true, false}) boolean embedSigner) throws IOException, org.signal.libsignal.protocol.InvalidKeyException { @@ -60,7 +63,7 @@ class CertificateGeneratorTest { when(account.getIdentityKey(IdentityType.ACI)).thenReturn(IDENTITY_KEY); when(account.getAccountIdentifier()).thenReturn(ACI); - when(account.getNumber()).thenReturn(E164); + when(account.getNumberOptional()).thenReturn(Optional.of(E164)); final byte[] contents = certificateGenerator.createFor(account, deviceId, includeE164); final SenderCertificate fullCertificate = SenderCertificate.parseFrom(contents); @@ -89,4 +92,25 @@ class CertificateGeneratorTest { assertTrue(signingKey .verifySignature(fullCertificate.getCertificate().toByteArray(), fullCertificate.getSignature().toByteArray())); } + + @CartesianTest + void createForNoNumber(@CartesianTest.Values(booleans = {true, false}) final boolean includeE164, + @CartesianTest.Values(booleans = {true, false}) final boolean embedSigner) throws InvalidProtocolBufferException { + final Account account = mock(Account.class); + final byte deviceId = 4; + final CertificateGenerator certificateGenerator = new CertificateGenerator( + SIGNING_CERTIFICATE_DATA, SIGNING_KEY, 1, embedSigner); + + when(account.getIdentityKey(IdentityType.ACI)).thenReturn(IDENTITY_KEY); + when(account.getAccountIdentifier()).thenReturn(ACI); + when(account.getNumberOptional()).thenReturn(Optional.empty()); + + final Executable generateCertificate = () -> certificateGenerator.createFor(account, deviceId, includeE164); + + if (includeE164) { + assertThrows(IllegalArgumentException.class, generateCertificate); + } else { + assertDoesNotThrow(generateCertificate); + } + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/CertificateControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/CertificateControllerTest.java index 6152ebe30..b7a674add 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/CertificateControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/CertificateControllerTest.java @@ -52,6 +52,7 @@ import org.whispersystems.textsecuregcm.entities.DeliveryCertificate; import org.whispersystems.textsecuregcm.entities.GroupCredentials; import org.whispersystems.textsecuregcm.entities.MessageProtos.SenderCertificate; import org.whispersystems.textsecuregcm.entities.MessageProtos.ServerCertificate; +import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.util.HeaderUtils; @@ -224,6 +225,25 @@ class CertificateControllerTest { assertEquals(401, response.getStatus()); } + @Test + void testValidCertificateAccountHasNoNumber() { + final Account numberlessAccount = mock(Account.class); + when(numberlessAccount.getNumberOptional()).thenReturn(Optional.empty()); + + when(ACCOUNTS_MANAGER.getByAccountIdentifier(AuthHelper.VALID_UUID)) + .thenReturn(Optional.of(numberlessAccount)); + + final Response response = resources.getJerseyTest() + .target("/v1/certificate/delivery") + .queryParam("includeUuid", "true") + .queryParam("includeE164", "true") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .get(); + + assertEquals(400, response.getStatus()); + } + @Test void testGetSingleGroupCredentialWithPniAsServiceId() { final Instant startOfDay = clock.instant().truncatedTo(ChronoUnit.DAYS); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcServiceTest.java index d2e9748d0..eee4799a6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/CredentialsGrpcServiceTest.java @@ -150,7 +150,7 @@ public class CredentialsGrpcServiceTest @BeforeEach void setUp() { when(authenticatedAccount.getAccountIdentifier()).thenReturn(AUTHENTICATED_ACI); - when(authenticatedAccount.getNumber()).thenReturn(PHONE_NUMBER); + when(authenticatedAccount.getNumberOptional()).thenReturn(Optional.of(PHONE_NUMBER)); when(authenticatedAccount.getIdentifier(IdentityType.ACI)).thenReturn(AUTHENTICATED_ACI); when(authenticatedAccount.getIdentifier(IdentityType.PNI)).thenReturn(AUTHENTICATED_PNI); when(authenticatedAccount.getIdentityKey(IdentityType.ACI)) @@ -291,6 +291,17 @@ public class CredentialsGrpcServiceTest new IdentityKey(IDENTITY_KEY_PAIR.getPublicKey()).serialize()); } + @Test + void getDeliveryCertificateAccountHasNoNumber() throws InvalidProtocolBufferException, InvalidKeyException { + when(authenticatedAccount.getNumberOptional()).thenReturn(Optional.empty()); + + final GetDeliveryCertificateResponse response = + authenticatedServiceStub().getDeliveryCertificate(GetDeliveryCertificateRequest.getDefaultInstance()); + + assertTrue(response.getCertificateWithE164().isEmpty()); + checkDeliveryCertificate(response.getCertificateWithoutE164().toByteArray(), false); + } + @ParameterizedTest @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7}) void getGroupCredentials(final int redemptionEndOffsetDays) {