Check structural validity of prekeys at upload time

This commit is contained in:
Jonathan Klabunde Tomer 2023-05-31 14:29:39 -07:00 committed by GitHub
parent 0ab66f2f14
commit ecd207f0a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 191 additions and 29 deletions

View File

@ -18,6 +18,8 @@ import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import org.whispersystems.textsecuregcm.util.ByteArrayAdapter;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
public record ChangeNumberRequest(
@Schema(description="""
@ -49,7 +51,7 @@ public record ChangeNumberRequest(
@Schema(description="""
A new signed elliptic-curve prekey for each enabled device on the account, including this one.
Each must be accompanied by a valid signature from the new identity key in this request.""")
@NotNull @Valid Map<Long, @NotNull @Valid SignedPreKey> devicePniSignedPrekeys,
@NotNull @Valid Map<Long, @NotNull @Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> devicePniSignedPrekeys,
@Schema(description="""
A new signed post-quantum last-resort prekey for each enabled device on the account, including this one.
@ -57,7 +59,7 @@ public record ChangeNumberRequest(
If present, must contain one prekey per enabled device including this one.
Prekeys for devices that did not previously have any post-quantum prekeys stored will be silently dropped.
Each must be accompanied by a valid signature from the new identity key in this request.""")
@Valid Map<Long, @NotNull @Valid SignedPreKey> devicePniPqLastResortPrekeys,
@Valid Map<Long, @NotNull @Valid @ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> devicePniPqLastResortPrekeys,
@Schema(description="the new phone-number-identity registration ID for each enabled device on the account, including this one")
@NotNull Map<Long, Integer> pniRegistrationIds) implements PhoneVerificationRequest {

View File

@ -18,6 +18,8 @@ import javax.annotation.Nullable;
import javax.validation.Valid;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotNull;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
public record ChangePhoneNumberRequest(
@Schema(description="the new phone number for this account")
@ -42,7 +44,7 @@ public record ChangePhoneNumberRequest(
@Schema(description="""
A new signed elliptic-curve prekey for each enabled device on the account, including this one.
Each must be accompanied by a valid signature from the new identity key in this request.""")
@Nullable Map<Long, SignedPreKey> devicePniSignedPrekeys,
@Nullable Map<Long, @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> devicePniSignedPrekeys,
@Schema(description="""
A new signed post-quantum last-resort prekey for each enabled device on the account, including this one.
@ -50,7 +52,7 @@ public record ChangePhoneNumberRequest(
If present, must contain one prekey per enabled device including this one.
Prekeys for devices that did not previously have any post-quantum prekeys stored will be silently dropped.
Each must be accompanied by a valid signature from the new identity key in this request.""")
@Nullable @Valid Map<Long, @NotNull @Valid SignedPreKey> devicePniPqLastResortPrekeys,
@Nullable @Valid Map<Long, @NotNull @Valid @ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> devicePniPqLastResortPrekeys,
@Schema(description="the new phone-number-identity registration ID for each enabled device on the account, including this one")
@Nullable Map<Long, Integer> pniRegistrationIds) {

View File

@ -3,6 +3,10 @@ package org.whispersystems.textsecuregcm.entities;
import io.swagger.v3.oas.annotations.media.Schema;
import javax.validation.Valid;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
import java.util.Optional;
public record DeviceActivationRequest(@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, description = """
@ -10,28 +14,28 @@ public record DeviceActivationRequest(@Schema(requiredMode = Schema.RequiredMode
will be created "atomically," and all other properties needed for atomic account
creation must also be present.
""")
Optional<@Valid SignedPreKey> aciSignedPreKey,
Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> aciSignedPreKey,
@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, description = """
A signed EC pre-key to be associated with this account's PNI. If provided, an account
will be created "atomically," and all other properties needed for atomic account
creation must also be present.
""")
Optional<@Valid SignedPreKey> pniSignedPreKey,
Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> pniSignedPreKey,
@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, description = """
A signed Kyber-1024 "last resort" pre-key to be associated with this account's ACI. If
provided, an account will be created "atomically," and all other properties needed for
atomic account creation must also be present.
""")
Optional<@Valid SignedPreKey> aciPqLastResortPreKey,
Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> aciPqLastResortPreKey,
@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, description = """
A signed Kyber-1024 "last resort" pre-key to be associated with this account's PNI. If
provided, an account will be created "atomically," and all other properties needed for
atomic account creation must also be present.
""")
Optional<@Valid SignedPreKey> pniPqLastResortPreKey,
Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> pniPqLastResortPreKey,
@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, description = """
An APNs token set for the account's primary device. If provided, the account's primary

View File

@ -5,7 +5,6 @@
package org.whispersystems.textsecuregcm.entities;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.ArrayList;
import java.util.List;
@ -15,7 +14,12 @@ import javax.validation.constraints.AssertTrue;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import org.whispersystems.textsecuregcm.util.ByteArrayAdapter;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
public record PhoneNumberIdentityKeyDistributionRequest(
@NotEmpty
@ -36,7 +40,7 @@ public record PhoneNumberIdentityKeyDistributionRequest(
@Schema(description="""
A new signed elliptic-curve prekey for each enabled device on the account, including this one.
Each must be accompanied by a valid signature from the new identity key in this request.""")
Map<Long, @NotNull @Valid SignedPreKey> devicePniSignedPrekeys,
Map<Long, @NotNull @Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> devicePniSignedPrekeys,
@Schema(description="""
A new signed post-quantum last-resort prekey for each enabled device on the account, including this one.
@ -44,7 +48,7 @@ public record PhoneNumberIdentityKeyDistributionRequest(
If present, must contain one prekey per enabled device including this one.
Prekeys for devices that did not previously have any post-quantum prekeys stored will be silently dropped.
Each must be accompanied by a valid signature from the new identity key in this request.""")
@Valid Map<Long, @NotNull @Valid SignedPreKey> devicePniPqLastResortPrekeys,
@Valid Map<Long, @NotNull @Valid @ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> devicePniPqLastResortPrekeys,
@NotNull
@Valid

View File

@ -16,6 +16,8 @@ import javax.validation.Valid;
import javax.validation.constraints.AssertTrue;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
public class PreKeyState {
@ -24,10 +26,11 @@ public class PreKeyState {
@Schema(description="A list of unsigned elliptic-curve prekeys to use for this device. " +
"If present and not empty, replaces all stored unsigned EC prekeys for the device; " +
"if absent or empty, any stored unsigned EC prekeys for the device are not deleted.")
private List<PreKey> preKeys;
private List<@ValidPreKey(type=PreKeyType.ECC) PreKey> preKeys;
@JsonProperty
@Valid
@ValidPreKey(type=PreKeyType.ECC)
@Schema(description="An optional signed elliptic-curve prekey to use for this device. " +
"If present, replaces the stored signed elliptic-curve prekey for the device; " +
"if absent, the stored signed prekey is not deleted. " +
@ -40,10 +43,11 @@ public class PreKeyState {
"Each key must have a valid signature from the identity key in this request. " +
"If present and not empty, replaces all stored unsigned PQ prekeys for the device; " +
"if absent or empty, any stored unsigned PQ prekeys for the device are not deleted.")
private List<SignedPreKey> pqPreKeys;
private List<@ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> pqPreKeys;
@JsonProperty
@Valid
@ValidPreKey(type=PreKeyType.KYBER)
@Schema(description="An optional signed last-resort post-quantum prekey to use for this device. " +
"If present, replaces the stored signed post-quantum last-resort prekey for the device; " +
"if absent, a stored last-resort prekey will *not* be deleted. " +
@ -110,4 +114,5 @@ public class PreKeyState {
}
return spks.isEmpty() || PreKeySignatureValidator.validatePreKeySignatures(identityKey, spks);
}
}

View File

@ -14,6 +14,8 @@ import com.google.common.annotations.VisibleForTesting;
import io.swagger.v3.oas.annotations.media.Schema;
import org.whispersystems.textsecuregcm.util.ByteArrayAdapter;
import org.whispersystems.textsecuregcm.util.OptionalBase64ByteArrayDeserializer;
import org.whispersystems.textsecuregcm.util.ValidPreKey;
import org.whispersystems.textsecuregcm.util.ValidPreKey.PreKeyType;
import javax.validation.Valid;
import javax.validation.constraints.AssertTrue;
@ -73,17 +75,17 @@ public record RegistrationRequest(@Schema(requiredMode = Schema.RequiredMode.NOT
@JsonCreator
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public RegistrationRequest(@JsonProperty("sessionId") String sessionId,
@JsonProperty("recoveryPassword") byte[] recoveryPassword,
@JsonProperty("accountAttributes") AccountAttributes accountAttributes,
@JsonProperty("skipDeviceTransfer") boolean skipDeviceTransfer,
@JsonProperty("aciIdentityKey") Optional<byte[]> aciIdentityKey,
@JsonProperty("pniIdentityKey") Optional<byte[]> pniIdentityKey,
@JsonProperty("aciSignedPreKey") Optional<@Valid SignedPreKey> aciSignedPreKey,
@JsonProperty("pniSignedPreKey") Optional<@Valid SignedPreKey> pniSignedPreKey,
@JsonProperty("aciPqLastResortPreKey") Optional<@Valid SignedPreKey> aciPqLastResortPreKey,
@JsonProperty("pniPqLastResortPreKey") Optional<@Valid SignedPreKey> pniPqLastResortPreKey,
@JsonProperty("apnToken") Optional<@Valid ApnRegistrationId> apnToken,
@JsonProperty("gcmToken") Optional<@Valid GcmRegistrationId> gcmToken) {
@JsonProperty("recoveryPassword") byte[] recoveryPassword,
@JsonProperty("accountAttributes") AccountAttributes accountAttributes,
@JsonProperty("skipDeviceTransfer") boolean skipDeviceTransfer,
@JsonProperty("aciIdentityKey") Optional<byte[]> aciIdentityKey,
@JsonProperty("pniIdentityKey") Optional<byte[]> pniIdentityKey,
@JsonProperty("aciSignedPreKey") Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> aciSignedPreKey,
@JsonProperty("pniSignedPreKey") Optional<@Valid @ValidPreKey(type=PreKeyType.ECC) SignedPreKey> pniSignedPreKey,
@JsonProperty("aciPqLastResortPreKey") Optional<@Valid @ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> aciPqLastResortPreKey,
@JsonProperty("pniPqLastResortPreKey") Optional<@Valid @ValidPreKey(type=PreKeyType.KYBER) SignedPreKey> pniPqLastResortPreKey,
@JsonProperty("apnToken") Optional<@Valid ApnRegistrationId> apnToken,
@JsonProperty("gcmToken") Optional<@Valid GcmRegistrationId> gcmToken) {
// This may seem a little verbose, but at the time of writing, Jackson struggles with `@JsonUnwrapped` members in
// records, and this is a workaround. Please see

View File

@ -0,0 +1,37 @@
/*
* Copyright 2023 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.util;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE_USE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import javax.validation.Constraint;
import javax.validation.Payload;
@Target({FIELD, PARAMETER, TYPE_USE})
@Retention(RUNTIME)
@Constraint(validatedBy = {ValidPreKeyValidator.class})
@Documented
public @interface ValidPreKey {
public enum PreKeyType {
ECC,
KYBER
}
PreKeyType type();
String message() default "{org.whispersystems.textsecuregcm.util.ValidPreKey.message}";
Class<?>[] groups() default { };
Class<? extends Payload>[] payload() default { };
}

View File

@ -0,0 +1,38 @@
/*
* Copyright 2013-2020 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.util;
import javax.validation.ConstraintValidator;
import javax.validation.ConstraintValidatorContext;
import org.signal.libsignal.protocol.InvalidKeyException;
import org.signal.libsignal.protocol.ecc.Curve;
import org.signal.libsignal.protocol.kem.KEMPublicKey;
import org.whispersystems.textsecuregcm.entities.PreKey;
public class ValidPreKeyValidator implements ConstraintValidator<ValidPreKey, PreKey> {
private ValidPreKey.PreKeyType type;
@Override
public void initialize(ValidPreKey annotation) {
type = annotation.type();
}
@Override
public boolean isValid(PreKey value, ConstraintValidatorContext context) {
if (value == null) {
return true;
}
try {
switch (type) {
case ECC -> Curve.decodePoint(value.getPublicKey(), 0);
case KYBER -> new KEMPublicKey(value.getPublicKey());
}
} catch (IllegalArgumentException | InvalidKeyException e) {
return false;
}
return true;
}
}

View File

@ -686,8 +686,8 @@ class KeysControllerTest {
final PreKey preKey = KeysHelper.ecPreKey(31337);
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final SignedPreKey signedPreKey = KeysHelper.signedECPreKey(31338, identityKeyPair);
final SignedPreKey pqPreKey = KeysHelper.signedECPreKey(31339, identityKeyPair);
final SignedPreKey pqLastResortPreKey = KeysHelper.signedECPreKey(31340, identityKeyPair);
final SignedPreKey pqPreKey = KeysHelper.signedKEMPreKey(31339, identityKeyPair);
final SignedPreKey pqLastResortPreKey = KeysHelper.signedKEMPreKey(31340, identityKeyPair);
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
PreKeyState preKeyState = new PreKeyState(identityKey, signedPreKey, List.of(preKey), List.of(pqPreKey), pqLastResortPreKey);
@ -713,6 +713,74 @@ class KeysControllerTest {
verify(accounts).update(eq(AuthHelper.VALID_ACCOUNT), any());
}
@Test
void putKeysStructurallyInvalidSignedECKey() {
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
final SignedPreKey wrongPreKey = KeysHelper.signedKEMPreKey(1, identityKeyPair);
final PreKeyState preKeyState = new PreKeyState(identityKey, wrongPreKey, null, null, null);
Response response =
resources.getJerseyTest()
.target("/v2/keys")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.entity(preKeyState, MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void putKeysStructurallyInvalidUnsignedECKey() {
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
final PreKey wrongPreKey = new PreKey(1, "cluck cluck i'm a parrot".getBytes());
final PreKeyState preKeyState = new PreKeyState(identityKey, null, List.of(wrongPreKey), null, null);
Response response =
resources.getJerseyTest()
.target("/v2/keys")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.entity(preKeyState, MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void putKeysStructurallyInvalidPQOneTimeKey() {
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
final SignedPreKey wrongPreKey = KeysHelper.signedECPreKey(1, identityKeyPair);
final PreKeyState preKeyState = new PreKeyState(identityKey, null, null, List.of(wrongPreKey), null);
Response response =
resources.getJerseyTest()
.target("/v2/keys")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.entity(preKeyState, MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void putKeysStructurallyInvalidPQLastResortKey() {
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
final SignedPreKey wrongPreKey = KeysHelper.signedECPreKey(1, identityKeyPair);
final PreKeyState preKeyState = new PreKeyState(identityKey, null, null, null, wrongPreKey);
Response response =
resources.getJerseyTest()
.target("/v2/keys")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.put(Entity.entity(preKeyState, MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void putKeysByPhoneNumberIdentifierTestV2() {
final PreKey preKey = KeysHelper.ecPreKey(31337);
@ -747,8 +815,8 @@ class KeysControllerTest {
final PreKey preKey = KeysHelper.ecPreKey(31337);
final ECKeyPair identityKeyPair = Curve.generateKeyPair();
final SignedPreKey signedPreKey = KeysHelper.signedECPreKey(31338, identityKeyPair);
final SignedPreKey pqPreKey = KeysHelper.signedECPreKey(31339, identityKeyPair);
final SignedPreKey pqLastResortPreKey = KeysHelper.signedECPreKey(31340, identityKeyPair);
final SignedPreKey pqPreKey = KeysHelper.signedKEMPreKey(31339, identityKeyPair);
final SignedPreKey pqLastResortPreKey = KeysHelper.signedKEMPreKey(31340, identityKeyPair);
final byte[] identityKey = identityKeyPair.getPublicKey().serialize();
PreKeyState preKeyState = new PreKeyState(identityKey, signedPreKey, List.of(preKey), List.of(pqPreKey), pqLastResortPreKey);
@ -823,7 +891,7 @@ class KeysControllerTest {
@Test
void putIdentityKeyNonPrimary() {
final PreKey preKey = KeysHelper.ecPreKey(31337);
final PreKey preKey = KeysHelper.ecPreKey(31337);
final SignedPreKey signedPreKey = KeysHelper.signedECPreKey(31338, IDENTITY_KEY_PAIR);
List<PreKey> preKeys = List.of(preKey);