From 781cd0ca3f15fa57e21dcb04a5a5c278ca9e7f4c Mon Sep 17 00:00:00 2001 From: gram-signal <84339875+gram-signal@users.noreply.github.com> Date: Thu, 30 Mar 2023 17:19:18 -0600 Subject: [PATCH] Truncate SVR2 IDs to 16 bytes rather than 10. --- .../ExternalServiceCredentialsGenerator.java | 23 +++++++++++++------ .../SecureValueRecovery2Controller.java | 1 + ...ternalServiceCredentialsGeneratorTest.java | 11 +++++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/ExternalServiceCredentialsGenerator.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/ExternalServiceCredentialsGenerator.java index 11d894b75..4edc42dd1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/ExternalServiceCredentialsGenerator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/ExternalServiceCredentialsGenerator.java @@ -18,8 +18,6 @@ import org.apache.commons.lang3.Validate; public class ExternalServiceCredentialsGenerator { - private static final int TRUNCATE_LENGTH = 10; - private static final String DELIMITER = ":"; private final byte[] key; @@ -32,6 +30,7 @@ public class ExternalServiceCredentialsGenerator { private final Clock clock; + private final int truncateLength; public static ExternalServiceCredentialsGenerator.Builder builder(final byte[] key) { return new Builder(key); @@ -42,12 +41,14 @@ public class ExternalServiceCredentialsGenerator { final byte[] userDerivationKey, final boolean prependUsername, final boolean truncateSignature, - final Clock clock) { + final Clock clock, + final int truncateLength) { this.key = requireNonNull(key); this.userDerivationKey = requireNonNull(userDerivationKey); this.prependUsername = prependUsername; this.truncateSignature = truncateSignature; this.clock = requireNonNull(clock); + this.truncateLength = truncateLength; } /** @@ -66,7 +67,7 @@ public class ExternalServiceCredentialsGenerator { */ public ExternalServiceCredentials generateFor(final String identity) { final String username = shouldDeriveUsername() - ? hmac256TruncatedToHexString(userDerivationKey, identity, TRUNCATE_LENGTH) + ? hmac256TruncatedToHexString(userDerivationKey, identity, truncateLength) : identity; final long currentTimeSeconds = currentTimeSeconds(); @@ -74,7 +75,7 @@ public class ExternalServiceCredentialsGenerator { final String dataToSign = username + DELIMITER + currentTimeSeconds; final String signature = truncateSignature - ? hmac256TruncatedToHexString(key, dataToSign, TRUNCATE_LENGTH) + ? hmac256TruncatedToHexString(key, dataToSign, truncateLength) : hmac256ToHexString(key, dataToSign); final String token = (prependUsername ? dataToSign : currentTimeSeconds) + DELIMITER + signature; @@ -131,7 +132,7 @@ public class ExternalServiceCredentialsGenerator { final String signedData = credentials.username() + DELIMITER + timestampSeconds; final String expectedSignature = truncateSignature - ? hmac256TruncatedToHexString(key, signedData, TRUNCATE_LENGTH) + ? hmac256TruncatedToHexString(key, signedData, truncateLength) : hmac256ToHexString(key, signedData); // if the signature is valid it's safe to parse the `timestampSeconds` string into Long @@ -171,6 +172,8 @@ public class ExternalServiceCredentialsGenerator { private boolean truncateSignature = true; + private int truncateLength = 10; + private Clock clock = Clock.systemUTC(); @@ -189,6 +192,12 @@ public class ExternalServiceCredentialsGenerator { return this; } + public Builder withTruncateLength(int truncateLength) { + Validate.inclusiveBetween(10, 32, truncateLength); + this.truncateLength = truncateLength; + return this; + } + public Builder prependUsername(final boolean prependUsername) { this.prependUsername = prependUsername; return this; @@ -201,7 +210,7 @@ public class ExternalServiceCredentialsGenerator { public ExternalServiceCredentialsGenerator build() { return new ExternalServiceCredentialsGenerator( - key, userDerivationKey, prependUsername, truncateSignature, clock); + key, userDerivationKey, prependUsername, truncateSignature, clock, truncateLength); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SecureValueRecovery2Controller.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SecureValueRecovery2Controller.java index 5ac1e977a..85f581cd3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SecureValueRecovery2Controller.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SecureValueRecovery2Controller.java @@ -29,6 +29,7 @@ public class SecureValueRecovery2Controller { .builder(cfg.userAuthenticationTokenSharedSecret()) .withUserDerivationKey(cfg.userIdTokenSharedSecret()) .prependUsername(false) + .withTruncateLength(16) .build(); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/ExternalServiceCredentialsGeneratorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/ExternalServiceCredentialsGeneratorTest.java index f492a5c1c..828f53a0a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/ExternalServiceCredentialsGeneratorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/auth/ExternalServiceCredentialsGeneratorTest.java @@ -112,4 +112,15 @@ class ExternalServiceCredentialsGeneratorTest { assertEquals(generator.validateAndGetTimestamp(credentials, elapsedSeconds + 1).orElseThrow(), TIME_SECONDS); assertTrue(generator.validateAndGetTimestamp(credentials, elapsedSeconds - 1).isEmpty()); } + + @Test + public void testTruncateLength() throws Exception { + final ExternalServiceCredentialsGenerator generator = ExternalServiceCredentialsGenerator.builder(new byte[32]) + .withUserDerivationKey(new byte[32]) + .withTruncateLength(14) + .build(); + final ExternalServiceCredentials creds = generator.generateFor(E164); + assertEquals(14*2 /* 2 chars per byte, because hex */, creds.username().length()); + assertEquals("805b84df7eff1e8fe1baf0c6e838", creds.username()); + } }