From 8e742ceb9111cd2f4a586605720648227115d4a1 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Thu, 4 May 2017 10:24:45 -0700 Subject: [PATCH] Cancel apn fallback on unregistered event // FREEBIE --- .../textsecuregcm/WhisperServerService.java | 1 + .../textsecuregcm/push/APNSender.java | 26 +++++++++--------- .../push/ApnFallbackManager.java | 2 +- .../tests/push/APNSenderTest.java | 27 +++++++++++++++++-- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index e3efc2a08..1fb800d72 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -188,6 +188,7 @@ public class WhisperServerService extends Application authorizationKey = config.getRedphoneConfiguration().getAuthorizationKey(); + apnSender.setApnFallbackManager(apnFallbackManager); environment.lifecycle().manage(apnFallbackManager); environment.lifecycle().manage(pubSubManager); environment.lifecycle().manage(pushSender); diff --git a/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java b/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java index cc1324ad4..6dce37a9c 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java +++ b/src/main/java/org/whispersystems/textsecuregcm/push/APNSender.java @@ -21,9 +21,6 @@ import com.google.common.base.Optional; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import com.relayrides.pushy.apns.ApnsClient; -import com.relayrides.pushy.apns.ApnsClientBuilder; -import org.bouncycastle.openssl.PEMReader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.configuration.ApnConfiguration; @@ -31,18 +28,13 @@ import org.whispersystems.textsecuregcm.push.RetryingApnsClient.ApnResult; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.websocket.WebsocketAddress; import javax.annotation.Nullable; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStreamReader; -import java.security.KeyPair; -import java.security.PrivateKey; -import java.security.cert.X509Certificate; import java.util.Date; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import io.dropwizard.lifecycle.Managed; @@ -51,7 +43,8 @@ public class APNSender implements Managed { private final Logger logger = LoggerFactory.getLogger(APNSender.class); - private ExecutorService executor; + private ExecutorService executor; + private ApnFallbackManager fallbackManager; private final AccountsManager accountsManager; private final String bundleId; @@ -86,8 +79,7 @@ public class APNSender implements Managed { if (message.isVoip()) { topic = topic + ".voip"; } - - + ListenableFuture future = apnsClient.send(message.getApnId(), topic, message.getMessage(), new Date(message.getExpirationTime())); @@ -100,7 +92,7 @@ public class APNSender implements Managed { } else if (result.getStatus() == ApnResult.Status.NO_SUCH_USER) { handleUnregisteredUser(message.getApnId(), message.getNumber(), message.getDeviceId()); } else if (result.getStatus() == ApnResult.Status.GENERIC_FAILURE) { - logger.warn("*** Got APN generic failure: " + result.getReason()); + logger.warn("*** Got APN generic failure: " + result.getReason() + ", " + message.getNumber()); } } @@ -125,6 +117,10 @@ public class APNSender implements Managed { this.apnsClient.disconnect(); } + public void setApnFallbackManager(ApnFallbackManager fallbackManager) { + this.fallbackManager = fallbackManager; + } + private void handleUnregisteredUser(String registrationId, String number, int deviceId) { logger.info("Got APN Unregistered: " + number + "," + deviceId); @@ -164,5 +160,9 @@ public class APNSender implements Managed { device.get().setVoipApnId(null); device.get().setFetchesMessages(false); accountsManager.update(account.get()); + + if (fallbackManager != null) { + fallbackManager.cancel(new WebsocketAddress(number, deviceId)); + } } } diff --git a/src/main/java/org/whispersystems/textsecuregcm/push/ApnFallbackManager.java b/src/main/java/org/whispersystems/textsecuregcm/push/ApnFallbackManager.java index 2029f1d92..3fde9ba30 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/push/ApnFallbackManager.java +++ b/src/main/java/org/whispersystems/textsecuregcm/push/ApnFallbackManager.java @@ -64,7 +64,7 @@ public class ApnFallbackManager implements Managed, Runnable, DispatchChannel { } } - private void cancel(WebsocketAddress address) { + public void cancel(WebsocketAddress address) { ApnFallbackTask task = taskQueue.remove(address); if (task != null) { diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/push/APNSenderTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/push/APNSenderTest.java index f548314cb..26248fe77 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/push/APNSenderTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/push/APNSenderTest.java @@ -12,6 +12,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.whispersystems.textsecuregcm.push.APNSender; +import org.whispersystems.textsecuregcm.push.ApnFallbackManager; import org.whispersystems.textsecuregcm.push.ApnMessage; import org.whispersystems.textsecuregcm.push.RetryingApnsClient; import org.whispersystems.textsecuregcm.push.RetryingApnsClient.ApnResult; @@ -19,6 +20,7 @@ import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.tests.util.SynchronousExecutorService; +import org.whispersystems.textsecuregcm.websocket.WebsocketAddress; import java.util.Date; import java.util.concurrent.TimeUnit; @@ -35,8 +37,9 @@ public class APNSenderTest { private final AccountsManager accountsManager = mock(AccountsManager.class); - private final Account destinationAccount = mock(Account.class); - private final Device destinationDevice = mock(Device.class ); + private final Account destinationAccount = mock(Account.class); + private final Device destinationDevice = mock(Device.class); + private final ApnFallbackManager fallbackManager = mock(ApnFallbackManager.class); private final DefaultEventExecutor executor = new DefaultEventExecutor(); @@ -64,6 +67,7 @@ public class APNSenderTest { ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); ListenableFuture sendFuture = apnSender.sendMessage(message); ApnResult apnResult = sendFuture.get(); @@ -80,6 +84,7 @@ public class APNSenderTest { verifyNoMoreInteractions(apnsClient); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -98,6 +103,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", false, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); ListenableFuture sendFuture = apnSender.sendMessage(message); ApnResult apnResult = sendFuture.get(); @@ -115,6 +121,7 @@ public class APNSenderTest { verifyNoMoreInteractions(apnsClient); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -134,6 +141,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); when(destinationDevice.getApnId()).thenReturn(DESTINATION_APN_ID); when(destinationDevice.getPushTimestamp()).thenReturn(System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(11)); @@ -162,8 +170,10 @@ public class APNSenderTest { verify(destinationDevice, times(1)).setVoipApnId(eq((String)null)); verify(destinationDevice, times(1)).setFetchesMessages(eq(false)); verify(accountsManager, times(1)).update(eq(destinationAccount)); + verify(fallbackManager, times(1)).cancel(eq(new WebsocketAddress(DESTINATION_NUMBER, 1))); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -183,6 +193,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); when(destinationDevice.getApnId()).thenReturn("baz"); when(destinationDevice.getVoipApnId()).thenReturn(DESTINATION_APN_ID); @@ -213,8 +224,10 @@ public class APNSenderTest { verify(destinationDevice, times(1)).setVoipApnId(eq((String)null)); verify(destinationDevice, times(1)).setFetchesMessages(eq(false)); verify(accountsManager, times(1)).update(eq(destinationAccount)); + verify(fallbackManager, times(1)).cancel(eq(new WebsocketAddress(DESTINATION_NUMBER, 1))); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -234,6 +247,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); when(destinationDevice.getApnId()).thenReturn(DESTINATION_APN_ID); when(destinationDevice.getPushTimestamp()).thenReturn(System.currentTimeMillis()); @@ -262,6 +276,7 @@ public class APNSenderTest { verifyNoMoreInteractions(destinationDevice); verifyNoMoreInteractions(destinationAccount); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -281,6 +296,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); when(destinationDevice.getApnId()).thenReturn("baz"); when(destinationDevice.getPushTimestamp()).thenReturn(System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(12)); @@ -309,6 +325,7 @@ public class APNSenderTest { verifyNoMoreInteractions(destinationDevice); verifyNoMoreInteractions(destinationAccount); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -328,6 +345,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); ListenableFuture sendFuture = apnSender.sendMessage(message); ApnResult apnResult = sendFuture.get(); @@ -344,6 +362,7 @@ public class APNSenderTest { verifyNoMoreInteractions(apnsClient); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -367,6 +386,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 10); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); ListenableFuture sendFuture = apnSender.sendMessage(message); @@ -397,6 +417,7 @@ public class APNSenderTest { verifyNoMoreInteractions(apnsClient); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } @Test @@ -415,6 +436,7 @@ public class APNSenderTest { RetryingApnsClient retryingApnsClient = new RetryingApnsClient(apnsClient, 3); ApnMessage message = new ApnMessage(DESTINATION_APN_ID, DESTINATION_NUMBER, 1, "message", true, 30); APNSender apnSender = new APNSender(new SynchronousExecutorService(), accountsManager, retryingApnsClient, "foo", false); + apnSender.setApnFallbackManager(fallbackManager); ListenableFuture sendFuture = apnSender.sendMessage(message); @@ -435,6 +457,7 @@ public class APNSenderTest { verifyNoMoreInteractions(apnsClient); verifyNoMoreInteractions(accountsManager); + verifyNoMoreInteractions(fallbackManager); } }