Add tests and metrics for parsing invalid keys

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<K> extends JsonDeserializer<K> {
 
+  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<K> extends JsonDeserializer<K> {
     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<K> extends JsonDeserializer<K> {
     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);
     }
   }

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<Arguments> deserialize() {
-    final String template = """
-        {
-          "publicKey": %s
-        }
-        """;
-
     return Stream.of(
-        Arguments.of(String.format(template, "null"), null),
+        Arguments.of(String.format(JSON_TEMPLATE, "null"), null),
-        Arguments.of(String.format(template, "\"\""), null),
+        Arguments.of(String.format(JSON_TEMPLATE, "\"\""), null),
-        Arguments.of(String.format(template, "\"" + Base64.getEncoder().encodeToString(EC_PUBLIC_KEY.serialize()) + "\""), EC_PUBLIC_KEY)
+        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<String> 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\"")
     );
   }
 }

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<Arguments> deserialize() {
-    final String template = """
-        {
-          "publicKey": %s
-        }
-        """;
-
     return Stream.of(
-        Arguments.of(String.format(template, "null"), null),
+        Arguments.of(String.format(JSON_TEMPLATE, "null"), null),
-        Arguments.of(String.format(template, "\"\""), null),
+        Arguments.of(String.format(JSON_TEMPLATE, "\"\""), null),
-        Arguments.of(String.format(template, "\"" + Base64.getEncoder().encodeToString(KEM_PUBLIC_KEY.serialize()) + "\""),
+        Arguments.of(String.format(JSON_TEMPLATE,
-            KEM_PUBLIC_KEY)
+            "\"" + 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<String> 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\"")
     );
   }
 }