From d138fa45df7a221e8555eef23bd7ec791017198c Mon Sep 17 00:00:00 2001 From: erik-signal <113138376+erik-signal@users.noreply.github.com> Date: Tue, 20 Dec 2022 12:25:04 -0500 Subject: [PATCH] Handle edge cases of Math.abs on integers. --- .../auth/AuthenticationCredentials.java | 5 ++-- .../auth/BaseAccountAuthenticator.java | 11 +++++++- .../auth/TurnTokenGenerator.java | 3 ++- .../controllers/RemoteConfigController.java | 3 ++- .../textsecuregcm/util/Util.java | 25 +++++++++++++++++++ 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java index 714e447e8..88edd0c32 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/AuthenticationCredentials.java @@ -7,6 +7,7 @@ package org.whispersystems.textsecuregcm.auth; import com.google.common.annotations.VisibleForTesting; import org.apache.commons.codec.binary.Hex; import org.signal.libsignal.protocol.kdf.HKDF; +import org.whispersystems.textsecuregcm.util.Util; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; @@ -32,13 +33,13 @@ public class AuthenticationCredentials { } public AuthenticationCredentials(String authenticationToken) { - this.salt = String.valueOf(Math.abs(new SecureRandom().nextInt())); + this.salt = String.valueOf(Util.ensureNonNegativeInt(new SecureRandom().nextInt())); this.hashedAuthenticationToken = getV2HashedValue(salt, authenticationToken); } @VisibleForTesting public AuthenticationCredentials v1ForTesting(String authenticationToken) { - String salt = String.valueOf(Math.abs(new SecureRandom().nextInt())); + String salt = String.valueOf(Util.ensureNonNegativeInt(new SecureRandom().nextInt())); return new AuthenticationCredentials(getV1HashedValue(salt, authenticationToken), salt); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java index 52787f9df..e8d68842b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/BaseAccountAuthenticator.java @@ -146,9 +146,18 @@ public class BaseAccountAuthenticator { @VisibleForTesting public Account updateLastSeen(Account account, Device device) { - final long lastSeenOffsetSeconds = Math.abs(account.getUuid().getLeastSignificantBits()) % ChronoUnit.DAYS.getDuration().toSeconds(); + // compute a non-negative integer between 0 and 86400. + long n = Util.ensureNonNegativeLong(account.getUuid().getLeastSignificantBits()); + final long lastSeenOffsetSeconds = n % ChronoUnit.DAYS.getDuration().toSeconds(); + + // produce a truncated timestamp which is either today at UTC midnight + // or yesterday at UTC midnight, based on per-user randomized offset used. final long todayInMillisWithOffset = Util.todayInMillisGivenOffsetFromNow(clock, Duration.ofSeconds(lastSeenOffsetSeconds).negated()); + // only update the device's last seen time when it falls behind the truncated timestamp. + // this ensure a few things: + // (1) each account will only update last-seen at most once per day + // (2) these updates will occur throughout the day rather than all occurring at UTC midnight. if (device.getLastSeen() < todayInMillisWithOffset) { Metrics.summary(DAYS_SINCE_LAST_SEEN_DISTRIBUTION_NAME, IS_PRIMARY_DEVICE_TAG, String.valueOf(device.isMaster())) .record(Duration.ofMillis(todayInMillisWithOffset - device.getLastSeen()).toDays()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/TurnTokenGenerator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/TurnTokenGenerator.java index 5d7a3192c..df6241256 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/TurnTokenGenerator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/TurnTokenGenerator.java @@ -10,6 +10,7 @@ import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfigurati import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicTurnConfiguration; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.Pair; +import org.whispersystems.textsecuregcm.util.Util; import org.whispersystems.textsecuregcm.util.WeightedRandomSelect; import javax.crypto.Mac; @@ -36,7 +37,7 @@ public class TurnTokenGenerator { List urls = urls(e164); Mac mac = Mac.getInstance("HmacSHA1"); long validUntilSeconds = (System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1)) / 1000; - long user = Math.abs(new SecureRandom().nextInt()); + long user = Util.ensureNonNegativeInt(new SecureRandom().nextInt()); String userTime = validUntilSeconds + ":" + user; mac.init(new SecretKeySpec(key, "HmacSHA1")); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigController.java index bbfdf3219..3a36838ee 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigController.java @@ -41,6 +41,7 @@ import org.whispersystems.textsecuregcm.entities.UserRemoteConfigList; import org.whispersystems.textsecuregcm.storage.RemoteConfig; import org.whispersystems.textsecuregcm.storage.RemoteConfigsManager; import org.whispersystems.textsecuregcm.util.Conversions; +import org.whispersystems.textsecuregcm.util.Util; @Path("/v1/config") public class RemoteConfigController { @@ -133,7 +134,7 @@ public class RemoteConfigController { digest.update(bb.array()); byte[] hash = digest.digest(hashKey); - int bucket = (int)(Math.abs(Conversions.byteArrayToLong(hash)) % 100); + int bucket = (int)(Util.ensureNonNegativeLong(Conversions.byteArrayToLong(hash)) % 100); return bucket < configPercentage; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java index 26039fe9a..582ad4556 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java @@ -191,4 +191,29 @@ public class Util { public static Optional findBestLocale(List priorityList, Collection supportedLocales) { return Optional.ofNullable(Locale.lookupTag(priorityList, supportedLocales)); } + + /** + * Map ints to non-negative ints. + *
+ * Unlike Math.abs this method handles Integer.MIN_VALUE correctly. + * + * @param n any int value + * @return an int value guaranteed to be non-negative + */ + public static int ensureNonNegativeInt(int n) { + return n == Integer.MIN_VALUE ? 0 : Math.abs(n); + } + + /** + * Map longs to non-negative longs. + *
+ * Unlike Math.abs this method handles Long.MIN_VALUE correctly. + * + * @param n any long value + * @return a long value guaranteed to be non-negative + */ + public static long ensureNonNegativeLong(long n) { + return n == Long.MIN_VALUE ? 0 : Math.abs(n); + } + }