From 387f18be982c0ce57b4fad6fa89d9b2e8fa2baf8 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 11 May 2023 16:54:57 -0400 Subject: [PATCH] Avoid some 401 errors during story sends. --- .../crypto/UnidentifiedAccessUtil.java | 44 ++++++++++++------- .../api/SignalServiceMessageSender.java | 6 +++ .../api/crypto/UnidentifiedAccess.java | 13 +++++- .../java/org/signal/util/SignalClient.kt | 2 +- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/crypto/UnidentifiedAccessUtil.java b/app/src/main/java/org/thoughtcrime/securesms/crypto/UnidentifiedAccessUtil.java index 24c4236a36..797e4fa04b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/crypto/UnidentifiedAccessUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/crypto/UnidentifiedAccessUtil.java @@ -96,25 +96,29 @@ public class UnidentifiedAccessUtil { Map typeCounts = new HashMap<>(); for (Recipient recipient : recipients) { - byte[] theirUnidentifiedAccessKey = getTargetUnidentifiedAccessKey(recipient, isForStory); - CertificateType certificateType = getUnidentifiedAccessCertificateType(recipient); - byte[] ourUnidentifiedAccessCertificate = SignalStore.certificateValues().getUnidentifiedAccessCertificate(certificateType); + CertificateType certificateType = getUnidentifiedAccessCertificateType(recipient); + byte[] ourUnidentifiedAccessCertificate = SignalStore.certificateValues().getUnidentifiedAccessCertificate(certificateType); int typeCount = Util.getOrDefault(typeCounts, certificateType, 0); typeCount++; typeCounts.put(certificateType, typeCount); - if (theirUnidentifiedAccessKey != null && ourUnidentifiedAccessCertificate != null) { + if (ourUnidentifiedAccessCertificate != null) { try { - access.add(Optional.of(new UnidentifiedAccessPair(new UnidentifiedAccess(theirUnidentifiedAccessKey, - ourUnidentifiedAccessCertificate), - new UnidentifiedAccess(ourUnidentifiedAccessKey, - ourUnidentifiedAccessCertificate)))); + UnidentifiedAccess theirAccess = getTargetUnidentifiedAccess(recipient, ourUnidentifiedAccessCertificate, isForStory); + UnidentifiedAccess ourAccess = new UnidentifiedAccess(ourUnidentifiedAccessKey, ourUnidentifiedAccessCertificate, false); + + if (theirAccess != null) { + access.add(Optional.of(new UnidentifiedAccessPair(theirAccess, ourAccess))); + } else { + access.add(Optional.empty()); + } } catch (InvalidCertificateException e) { - Log.w(TAG, e); + Log.w(TAG, "Invalid unidentified access certificate!", e); access.add(Optional.empty()); } } else { + Log.w(TAG, "Missing unidentified access certificate!"); access.add(Optional.empty()); } } @@ -140,9 +144,11 @@ public class UnidentifiedAccessUtil { if (ourUnidentifiedAccessCertificate != null) { return Optional.of(new UnidentifiedAccessPair(new UnidentifiedAccess(ourUnidentifiedAccessKey, - ourUnidentifiedAccessCertificate), + ourUnidentifiedAccessCertificate, + false), new UnidentifiedAccess(ourUnidentifiedAccessKey, - ourUnidentifiedAccessCertificate))); + ourUnidentifiedAccessCertificate, + false))); } return Optional.empty(); @@ -168,7 +174,7 @@ public class UnidentifiedAccessUtil { .getUnidentifiedAccessCertificate(getUnidentifiedAccessCertificateType(recipient)); } - private static @Nullable byte[] getTargetUnidentifiedAccessKey(@NonNull Recipient recipient, boolean isForStory) { + private static @Nullable UnidentifiedAccess getTargetUnidentifiedAccess(@NonNull Recipient recipient, @NonNull byte[] certificate, boolean isForStory) throws InvalidCertificateException { ProfileKey theirProfileKey = ProfileKeyUtil.profileKeyOrNull(recipient.resolve().getProfileKey()); byte[] accessKey; @@ -176,7 +182,11 @@ public class UnidentifiedAccessUtil { switch (recipient.resolve().getUnidentifiedAccessMode()) { case UNKNOWN: if (theirProfileKey == null) { - accessKey = UNRESTRICTED_KEY; + if (isForStory) { + accessKey = null; + } else { + accessKey = UNRESTRICTED_KEY; + } } else { accessKey = UnidentifiedAccess.deriveAccessKeyFrom(theirProfileKey); } @@ -199,10 +209,12 @@ public class UnidentifiedAccessUtil { } if (accessKey == null && isForStory) { - accessKey = UNRESTRICTED_KEY; + return new UnidentifiedAccess(UNRESTRICTED_KEY, certificate, true); + } else if (accessKey != null) { + return new UnidentifiedAccess(accessKey, certificate, false); + } else { + return null; } - - return accessKey; } private enum CertificateValidatorHolder { diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java index afaa671a26..4207ff2081 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java @@ -2378,9 +2378,15 @@ public class SignalServiceMessageSender { private List getPreKeys(SignalServiceAddress recipient, Optional unidentifiedAccess, int deviceId, boolean story) throws IOException { try { + // If it's only unrestricted because it's a story send, then we know it'll fail + if (story && unidentifiedAccess.isPresent() && unidentifiedAccess.get().isUnrestrictedForStory()) { + unidentifiedAccess = Optional.empty(); + } + return socket.getPreKeys(recipient, unidentifiedAccess, deviceId); } catch (NonSuccessfulResponseCodeException e) { if (e.getCode() == 401 && story) { + Log.d(TAG, "Got 401 when fetching prekey for story. Trying without UD."); return socket.getPreKeys(recipient, Optional.empty(), deviceId); } else { throw e; diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/UnidentifiedAccess.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/UnidentifiedAccess.java index f94e1ed4ed..a69ca11ec2 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/UnidentifiedAccess.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/UnidentifiedAccess.java @@ -21,12 +21,19 @@ public class UnidentifiedAccess { private final byte[] unidentifiedAccessKey; private final SenderCertificate unidentifiedCertificate; + private final boolean isUnrestrictedForStory; - public UnidentifiedAccess(byte[] unidentifiedAccessKey, byte[] unidentifiedCertificate) + /** + * @param isUnrestrictedForStory When sending to a story, we always want to use sealed sender. Receivers will accept it for story messages. However, there are + * some situations where we need to know if this access key will be correct for non-story purposes. Set this flag to true if + * the access key is a synthetic one that would only be valid for story messages. + */ + public UnidentifiedAccess(byte[] unidentifiedAccessKey, byte[] unidentifiedCertificate, boolean isUnrestrictedForStory) throws InvalidCertificateException { this.unidentifiedAccessKey = unidentifiedAccessKey; this.unidentifiedCertificate = new SenderCertificate(unidentifiedCertificate); + this.isUnrestrictedForStory = isUnrestrictedForStory; } public byte[] getUnidentifiedAccessKey() { @@ -37,6 +44,10 @@ public class UnidentifiedAccess { return unidentifiedCertificate; } + public boolean isUnrestrictedForStory() { + return isUnrestrictedForStory; + } + public static byte[] deriveAccessKeyFrom(ProfileKey profileKey) { try { byte[] nonce = new byte[12]; diff --git a/microbenchmark/src/androidTest/java/org/signal/util/SignalClient.kt b/microbenchmark/src/androidTest/java/org/signal/util/SignalClient.kt index cf13a291ff..2560836887 100644 --- a/microbenchmark/src/androidTest/java/org/signal/util/SignalClient.kt +++ b/microbenchmark/src/androidTest/java/org/signal/util/SignalClient.kt @@ -125,7 +125,7 @@ class SignalClient { val outgoingPushMessage: OutgoingPushMessage = cipher.encrypt( SignalProtocolAddress(to.serviceId.toString(), 1), - Optional.of(UnidentifiedAccess(to.unidentifiedAccessKey, senderCertificate.serialized)), + Optional.of(UnidentifiedAccess(to.unidentifiedAccessKey, senderCertificate.serialized, false)), EnvelopeContent.encrypted(content, ContentHint.RESENDABLE, Optional.empty()) )