Validate registration IDs

This commit is contained in:
Katherine
2023-11-28 15:43:35 -08:00
committed by GitHub
parent 8b95bb0c03
commit f46842c6c9
10 changed files with 123 additions and 31 deletions

View File

@@ -8,7 +8,6 @@ import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
import io.dropwizard.auth.Auth;
import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Tags;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
@@ -228,10 +227,6 @@ public class AccountController {
final Account account = disabledPermittedAuth.getAccount();
final byte deviceId = disabledPermittedAuth.getAuthenticatedDevice().getId();
if (!AccountsManager.validNewAccountAttributes(attributes)) {
Metrics.counter(INVALID_REGISTRATION_ID, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).increment();
}
final Account updatedAccount = accounts.update(account, a -> {
a.getDevice(deviceId).ifPresent(d -> {
d.setFetchesMessages(attributes.getFetchesMessages());

View File

@@ -107,10 +107,6 @@ public class RegistrationController {
final String password = authorizationHeader.getPassword();
RateLimiter.adaptLegacyException(() -> rateLimiters.getRegistrationLimiter().validate(number));
if (!AccountsManager.validNewAccountAttributes(registrationRequest.accountAttributes())) {
Metrics.counter(INVALID_ACCOUNT_ATTRS_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent))).increment();
throw new WebApplicationException(Response.status(422, "account attributes invalid").build());
}
final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(number,
registrationRequest);

View File

@@ -4,12 +4,15 @@
*/
package org.whispersystems.textsecuregcm.entities;
import static org.whispersystems.textsecuregcm.util.RegistrationIdValidator.validRegistrationId;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.common.annotations.VisibleForTesting;
import java.util.Optional;
import java.util.OptionalInt;
import javax.annotation.Nullable;
import javax.validation.constraints.AssertTrue;
import javax.validation.constraints.Size;
import org.whispersystems.textsecuregcm.auth.UnidentifiedAccessUtil;
import org.whispersystems.textsecuregcm.storage.Device.DeviceCapabilities;
@@ -127,4 +130,10 @@ public class AccountAttributes {
public void setPhoneNumberIdentityRegistrationId(final Integer phoneNumberIdentityRegistrationId) {
this.phoneNumberIdentityRegistrationId = phoneNumberIdentityRegistrationId;
}
@AssertTrue
public boolean isEachRegistrationIdValid() {
return validRegistrationId(registrationId) &&
(phoneNumberIdentityRegistrationId == null || validRegistrationId(phoneNumberIdentityRegistrationId));
}
}

View File

@@ -21,6 +21,7 @@ import javax.validation.constraints.NotNull;
import org.signal.libsignal.protocol.IdentityKey;
import org.whispersystems.textsecuregcm.util.ByteArrayAdapter;
import org.whispersystems.textsecuregcm.util.IdentityKeyAdapter;
import org.whispersystems.textsecuregcm.util.RegistrationIdValidator;
public record ChangeNumberRequest(
@Schema(description="""
@@ -78,4 +79,9 @@ public record ChangeNumberRequest(
}
return spks.isEmpty() || PreKeySignatureValidator.validatePreKeySignatures(pniIdentityKey, spks);
}
@AssertTrue
public boolean isEachPniRegistrationIdValid() {
return pniRegistrationIds == null || pniRegistrationIds.values().stream().allMatch(RegistrationIdValidator::validRegistrationId);
}
}

View File

@@ -16,6 +16,7 @@ public record LinkDeviceRequest(@Schema(requiredMode = Schema.RequiredMode.REQUI
""")
String verificationCode,
@Valid
AccountAttributes accountAttributes,
@NotNull

View File

@@ -33,7 +33,6 @@ import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.Queue;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
@@ -386,21 +385,6 @@ public class AccountsManager {
return updatedAccount.get();
}
public static boolean validNewAccountAttributes(final AccountAttributes accountAttributes) {
if (!validRegistrationId(accountAttributes.getRegistrationId())) {
return false;
}
final OptionalInt pniRegistrationId = accountAttributes.getPhoneNumberIdentityRegistrationId();
if (pniRegistrationId.isPresent() && !validRegistrationId(pniRegistrationId.getAsInt())) {
return false;
}
return true;
}
private static boolean validRegistrationId(int registrationId) {
return registrationId > 0 && registrationId <= Device.MAX_REGISTRATION_ID;
}
public Account updatePniKeys(final Account account,
final IdentityKey pniIdentityKey,
final Map<Byte, ECSignedPreKey> pniSignedPreKeys,

View File

@@ -56,7 +56,6 @@ public class ChangeNumberManager {
throw new IllegalArgumentException("PNI identity key, signed pre-keys, device messages, and registration IDs must be all null or all non-null");
}
if (number.equals(account.getNumber())) {
// The client has gotten confused/desynchronized with us about their own phone number, most likely due to losing
// our OK response to an immediately preceding change-number request, and are sending a change they don't realize

View File

@@ -0,0 +1,15 @@
/*
* Copyright 2023 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*
*/
package org.whispersystems.textsecuregcm.util;
import org.whispersystems.textsecuregcm.storage.Device;
public class RegistrationIdValidator {
public static boolean validRegistrationId(int registrationId) {
return registrationId > 0 && registrationId <= Device.MAX_REGISTRATION_ID;
}
}