diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/AbstractPublicKeyDeserializer.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/AbstractPublicKeyDeserializer.java index 78dcce46b..6b3163fdc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/AbstractPublicKeyDeserializer.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/AbstractPublicKeyDeserializer.java @@ -6,10 +6,16 @@ import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; import java.io.IOException; import java.util.Base64; +import io.micrometer.core.instrument.Metrics; import org.signal.libsignal.protocol.InvalidKeyException; +import org.whispersystems.textsecuregcm.metrics.MetricsUtil; abstract class AbstractPublicKeyDeserializer extends JsonDeserializer { + private final String invalidKeyCounterName = MetricsUtil.name(getClass(), "invalidKey"); + + private static final String REASON_TAG_NAME = "reason"; + @Override public K deserialize(final JsonParser parser, final DeserializationContext context) throws IOException { final byte[] publicKeyBytes; @@ -17,6 +23,7 @@ abstract class AbstractPublicKeyDeserializer extends JsonDeserializer { try { publicKeyBytes = Base64.getDecoder().decode(parser.getValueAsString()); } catch (final IllegalArgumentException e) { + Metrics.counter(invalidKeyCounterName, REASON_TAG_NAME, "illegal-base64").increment(); throw new JsonParseException(parser, "Could not parse public key as a base64-encoded value", e); } @@ -27,6 +34,7 @@ abstract class AbstractPublicKeyDeserializer extends JsonDeserializer { try { return deserializePublicKey(publicKeyBytes); } catch (final InvalidKeyException e) { + Metrics.counter(invalidKeyCounterName, REASON_TAG_NAME, "invalid-key").increment(); throw new JsonParseException(parser, "Could not interpret key bytes as a public key", e); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/ECPublicKeyAdapterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/ECPublicKeyAdapterTest.java index 868971f65..91ae48be0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/ECPublicKeyAdapterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/ECPublicKeyAdapterTest.java @@ -5,23 +5,30 @@ package org.whispersystems.textsecuregcm.util; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import java.util.Base64; +import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.signal.libsignal.protocol.ecc.Curve; import org.signal.libsignal.protocol.ecc.ECPublicKey; -import javax.annotation.Nullable; -import java.util.Base64; -import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.assertEquals; - class ECPublicKeyAdapterTest { + private static final String JSON_TEMPLATE = """ + { + "publicKey": %s + } + """; + private static final ECPublicKey EC_PUBLIC_KEY = Curve.generateKeyPair().getPublicKey(); private record ECPublicKeyCarrier(@JsonSerialize(using = ECPublicKeyAdapter.Serializer.class) @@ -38,16 +45,24 @@ class ECPublicKeyAdapterTest { } private static Stream deserialize() { - final String template = """ - { - "publicKey": %s - } - """; - return Stream.of( - Arguments.of(String.format(template, "null"), null), - Arguments.of(String.format(template, "\"\""), null), - Arguments.of(String.format(template, "\"" + Base64.getEncoder().encodeToString(EC_PUBLIC_KEY.serialize()) + "\""), EC_PUBLIC_KEY) + Arguments.of(String.format(JSON_TEMPLATE, "null"), null), + Arguments.of(String.format(JSON_TEMPLATE, "\"\""), null), + Arguments.of(String.format(JSON_TEMPLATE, "\"" + Base64.getEncoder().encodeToString(EC_PUBLIC_KEY.serialize()) + "\""), EC_PUBLIC_KEY) + ); + } + + @ParameterizedTest + @MethodSource + void deserializeInvalidKey(final String json) { + assertThrows(JsonMappingException.class, + () -> SystemMapper.jsonMapper().readValue(json, ECPublicKeyCarrier.class)); + } + + private static Stream deserializeInvalidKey() { + return Stream.of( + String.format(JSON_TEMPLATE, "\"" + Base64.getEncoder().encodeToString(new byte[12]) + "\""), + String.format(JSON_TEMPLATE, "\"This is not a legal base64-encoded string\"") ); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/KEMPublicKeyAdapterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/KEMPublicKeyAdapterTest.java index 5a729ed2e..abf02ce0a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/KEMPublicKeyAdapterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/util/KEMPublicKeyAdapterTest.java @@ -5,23 +5,31 @@ package org.whispersystems.textsecuregcm.util; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import java.util.Base64; +import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.signal.libsignal.protocol.kem.KEMKeyPair; import org.signal.libsignal.protocol.kem.KEMKeyType; import org.signal.libsignal.protocol.kem.KEMPublicKey; -import javax.annotation.Nullable; -import java.util.Base64; -import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.*; class KEMPublicKeyAdapterTest { + private static final String JSON_TEMPLATE = """ + { + "publicKey": %s + } + """; + private static final KEMPublicKey KEM_PUBLIC_KEY = KEMKeyPair.generate(KEMKeyType.KYBER_1024).getPublicKey(); private record KEMPublicKeyCarrier(@JsonSerialize(using = KEMPublicKeyAdapter.Serializer.class) @@ -38,17 +46,25 @@ class KEMPublicKeyAdapterTest { } private static Stream deserialize() { - final String template = """ - { - "publicKey": %s - } - """; - return Stream.of( - Arguments.of(String.format(template, "null"), null), - Arguments.of(String.format(template, "\"\""), null), - Arguments.of(String.format(template, "\"" + Base64.getEncoder().encodeToString(KEM_PUBLIC_KEY.serialize()) + "\""), - KEM_PUBLIC_KEY) + Arguments.of(String.format(JSON_TEMPLATE, "null"), null), + Arguments.of(String.format(JSON_TEMPLATE, "\"\""), null), + Arguments.of(String.format(JSON_TEMPLATE, + "\"" + Base64.getEncoder().encodeToString(KEM_PUBLIC_KEY.serialize()) + "\""), KEM_PUBLIC_KEY) + ); + } + + @ParameterizedTest + @MethodSource + void deserializeInvalidKey(final String json) { + assertThrows(JsonMappingException.class, + () -> SystemMapper.jsonMapper().readValue(json, KEMPublicKeyAdapterTest.KEMPublicKeyCarrier.class)); + } + + private static Stream deserializeInvalidKey() { + return Stream.of( + String.format(JSON_TEMPLATE, "\"" + Base64.getEncoder().encodeToString(new byte[12]) + "\""), + String.format(JSON_TEMPLATE, "\"This is not a legal base64-encoded string\"") ); } }