Check for missing version bytes in invalid identity keys

This commit is contained in:
Jon Chambers 2023-06-07 10:09:11 -04:00 committed by Jon Chambers
parent aaf43a592f
commit 1c8443210a
1 changed files with 18 additions and 20 deletions

View File

@ -10,7 +10,6 @@ import static java.util.Objects.requireNonNull;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Timer;
import java.io.IOException;
@ -31,8 +30,10 @@ import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.signal.libsignal.protocol.IdentityKey;
import org.signal.libsignal.protocol.InvalidKeyException;
import org.signal.libsignal.protocol.ecc.ECPublicKey;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.util.AttributeValues;
@ -80,8 +81,7 @@ public class Accounts extends AbstractDynamoDbStore {
private static final Timer GET_ALL_FROM_OFFSET_TIMER = Metrics.timer(name(Accounts.class, "getAllFromOffset"));
private static final Timer DELETE_TIMER = Metrics.timer(name(Accounts.class, "delete"));
private static final Counter INVALID_ACI_IDENTITY_KEY_COUNTER = Metrics.counter(name(Accounts.class, "invalidIdentityKey"), "type", "aci");
private static final Counter INVALID_PNI_IDENTITY_KEY_COUNTER = Metrics.counter(name(Accounts.class, "invalidIdentityKey"), "type", "pni");
private static final String INVALID_IDENTITY_KEY_COUNTER_NAME = name(Accounts.class, "invalidIdentityKey");
private static final String CONDITIONAL_CHECK_FAILED = "ConditionalCheckFailed";
@ -915,23 +915,8 @@ public class Accounts extends AbstractDynamoDbStore {
.map(AttributeValue::bool)
.orElse(false));
if (account.getIdentityKey() != null && account.getIdentityKey().length > 0) {
try {
new IdentityKey(account.getIdentityKey());
} catch (final InvalidKeyException e) {
log.debug("Account {} has an invalid ACI identity key", account.getUuid());
INVALID_ACI_IDENTITY_KEY_COUNTER.increment();
}
}
if (account.getPhoneNumberIdentityKey() != null && account.getPhoneNumberIdentityKey().length > 0) {
try {
new IdentityKey(account.getPhoneNumberIdentityKey());
} catch (final InvalidKeyException e) {
log.debug("Account {} has an invalid PNI identity key", account.getUuid());
INVALID_PNI_IDENTITY_KEY_COUNTER.increment();
}
}
checkIdentityKey(account.getUuid(), account.getIdentityKey(), "aci");
checkIdentityKey(account.getUuid(), account.getPhoneNumberIdentityKey(), "pni");
return account;
@ -940,6 +925,19 @@ public class Accounts extends AbstractDynamoDbStore {
}
}
private static void checkIdentityKey(final UUID accountIdentifier, @Nullable final byte[] identityKey, final String keyType) {
if (identityKey != null && identityKey.length > 0) {
try {
new IdentityKey(identityKey);
} catch (final InvalidKeyException e) {
if (identityKey.length != ECPublicKey.KEY_SIZE - 1) {
log.warn("Account {} has an invalid {} identity key; length = {}", accountIdentifier, keyType, identityKey.length);
Metrics.counter(INVALID_IDENTITY_KEY_COUNTER_NAME, "type", keyType).increment();
}
}
}
}
private static boolean conditionalCheckFailed(final CancellationReason reason) {
return CONDITIONAL_CHECK_FAILED.equals(reason.code());
}