From e88ec23a3ca2ccebc156c8af5112cae372a56c6e Mon Sep 17 00:00:00 2001
From: Seth Moore <sethmo@google.com>
Date: Fri, 23 Feb 2024 15:53:37 -0800
Subject: [PATCH] Validate coordinates when creating EC2 keys

Previously, we would simply "trust" the caller was providing legitimate
points on the curve. Now, use named curves from bouncy castle to verify
that the coordinates are valid.
---
 src/com/google/cose/utils/CoseUtils.java    | 10 ++++++-
 test/com/google/cose/Ec2SigningKeyTest.java | 33 ++++++++++++++-------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/com/google/cose/utils/CoseUtils.java b/src/com/google/cose/utils/CoseUtils.java
index 578b38e..ff00dc2 100644
--- a/src/com/google/cose/utils/CoseUtils.java
+++ b/src/com/google/cose/utils/CoseUtils.java
@@ -66,6 +66,7 @@
 import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.DERSequenceGenerator;
 import org.bouncycastle.jce.ECNamedCurveTable;
+import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec;
 
 public class CoseUtils {
   private static final String EC_PARAMETER_SPEC = "EC";
@@ -148,8 +149,15 @@ public static ECPrivateKey getEc2PrivateKeyFromInteger(int curve, BigInteger s)
   public static PublicKey getEc2PublicKeyFromCoordinates(int curve, BigInteger x, BigInteger y)
       throws CoseException {
     try {
+      final String curveName = getEc2CoseCurveName(curve);
+      final ECNamedCurveParameterSpec spec = ECNamedCurveTable.getParameterSpec(curveName);
+      if (spec == null) {
+        throw new IllegalStateException("Unsupported curve: " + curveName);
+      }
+      spec.getCurve().validatePoint(x, y);
+
       final AlgorithmParameters params = AlgorithmParameters.getInstance(EC_PARAMETER_SPEC);
-      params.init(new ECGenParameterSpec(getEc2CoseCurveName(curve)));
+      params.init(new ECGenParameterSpec(curveName));
       final ECParameterSpec ecParameters = params.getParameterSpec(ECParameterSpec.class);
 
       final ECPoint ecPoint = new ECPoint(x, y);
diff --git a/test/com/google/cose/Ec2SigningKeyTest.java b/test/com/google/cose/Ec2SigningKeyTest.java
index 52a36eb..ab9d0bd 100644
--- a/test/com/google/cose/Ec2SigningKeyTest.java
+++ b/test/com/google/cose/Ec2SigningKeyTest.java
@@ -80,17 +80,28 @@ public void testRoundTrip() throws CborException, CoseException {
   }
 
   @Test
-  public void testConversion() throws CborException, CoseException {
-    final String cborString = "A4010220012158205A88D182BCE5F42EFA59943F33359D2E8A968FF289D93E5FA44"
-        + "4B624343167FE225820B16E8CF858DDC7690407BA61D4C338237A8CFCF3DE6AA672FC60A557AA32FC67";
-    final ByteString x = new ByteString(X_BYTES);
-    final ByteString y = new ByteString(Y_BYTES);
-    final Ec2SigningKey sKey = Ec2SigningKey.parse(TestUtilities.hexStringToByteArray(cborString));
-    Assert.assertEquals(Headers.KEY_TYPE_EC2, sKey.getKeyType());
-    Assert.assertEquals(new UnsignedInteger(Headers.CURVE_EC2_P256),
-        sKey.getLabels().get(Headers.KEY_PARAMETER_CURVE));
-    Assert.assertEquals(x, sKey.getLabels().get(Headers.KEY_PARAMETER_X));
-    Assert.assertEquals(y, sKey.getLabels().get(Headers.KEY_PARAMETER_Y));
+  public void testCoordinatesNotOnCurve() {
+    final int[] allCurves = new int[]{
+        Headers.CURVE_EC2_P256,
+        Headers.CURVE_EC2_P384,
+        Headers.CURVE_EC2_P521
+    };
+
+    for (int curve: allCurves) {
+      final Map map = new Map();
+      map.put(new UnsignedInteger(Headers.KEY_PARAMETER_KEY_TYPE),
+              new UnsignedInteger(Headers.KEY_TYPE_EC2));
+      map.put(new NegativeInteger(Headers.KEY_PARAMETER_CURVE), new UnsignedInteger(curve));
+      map.put(new NegativeInteger(Headers.KEY_PARAMETER_X),
+              new ByteString(
+                  TestUtilities.hexStringToByteArray(
+                      "59cc8d401bbd23517c7b5e948d269190d2a897e9061d1af3bc0398d77edd1400")));
+      map.put(new NegativeInteger(Headers.KEY_PARAMETER_Y),
+              new ByteString(
+                  TestUtilities.hexStringToByteArray(
+                      "77550f5dbc8b3fbbb35c099961d51a956c7228c6e36aeccb4c95ac69cf27f7e6")));
+      assertThrows(IllegalArgumentException.class, () -> new Ec2SigningKey(map));
+    }
   }
 
   @Test