Check that G1 and G2 points lie in the correct subgroups when created (…

…#741)
Consensys · Jun 24, 2019 · a721bda · a721bda
1 parent b1c9aa6
commit a721bda
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 22 deletions.
diff --git a/util/src/main/java/tech/pegasys/artemis/util/mikuli/G1Point.java b/util/src/main/java/tech/pegasys/artemis/util/mikuli/G1Point.java
@@ -47,7 +47,8 @@ public static G1Point random() {
       point = new ECP(BIG.fromBytes(xBytes));
     } while (point.is_infinity());
 
-    return new G1Point(point);
+    // Multiply by the cofactor to ensure that we end up on G1
+    return new G1Point(scaleWithCofactor(point));
   }
 
   /**
@@ -104,8 +105,12 @@ public static G1Point fromBytesCompressed(Bytes bytes) {
       throw new IllegalArgumentException("X coordinate is not on the curve.");
     }
 
+    if (!isInGroup(point)) {
+      throw new IllegalArgumentException("The deserialised point is not in the G1 subgroup.");
+    }
+
     // Did we get the right branch of the sqrt?
-    if (!point.is_infinity() && aIn != calculateYFlag(point.getY())) {
+    if (aIn != calculateYFlag(point.getY())) {
       // We didn't: so choose the other branch of the sqrt.
       FP x = new FP(point.getX());
       FP yneg = new FP(point.getY());
@@ -116,6 +121,23 @@ public static G1Point fromBytesCompressed(Bytes bytes) {
     return new G1Point(point);
   }
 
+  /**
+   * Multiply the point by the group cofactor.
+   *
+   * @param point the point to be scaled
+   * @return a scaled point
+   */
+  static ECP scaleWithCofactor(ECP point) {
+
+    // The G1 cofactor
+    String cofactorHex =
+        "0x0000000000000000000000000000000000000000000000000000000000000000396c8c005555e1568c00aaab0000aaab";
+
+    BIG cofactor = BIG.fromBytes(Bytes.fromHexString(cofactorHex).toArray());
+
+    return (point.mul(cofactor));
+  }
+
   private final ECP point;
 
   private static final int fpPointSize = BIG.MODBYTES;
@@ -184,6 +206,12 @@ public Bytes toBytesCompressed() {
     return Bytes.wrap(xBytes);
   }
 
+  // Verify that the given point is in the correct subgroup for G2 by multiplying by the group order
+  static boolean isInGroup(ECP point) {
+    ECP orderCheck = point.mul(new BIG(ROM.CURVE_Order));
+    return orderCheck.is_infinity();
+  }
+
   ECP ecpPoint() {
     return point;
   }

diff --git a/util/src/main/java/tech/pegasys/artemis/util/mikuli/G2Point.java b/util/src/main/java/tech/pegasys/artemis/util/mikuli/G2Point.java
@@ -53,7 +53,8 @@ public static G2Point random() {
       point = new ECP2(new FP2(BIG.fromBytes(xReBytes), BIG.fromBytes(xImBytes)));
     } while (point.is_infinity());
 
-    return new G2Point(point);
+    // Multiply by the cofactor to ensure that we end up on G2 (see hashToG2)
+    return new G2Point(scaleWithCofactor(point));
   }
 
   /**
@@ -107,7 +108,6 @@ public static G2Point hashToG2(Bytes message, long domain) {
    * @param point the point whose Y coordinate is to be normalised.
    * @return a new point with the correct Y coordinate, which may the original.
    */
-  @VisibleForTesting
   static ECP2 normaliseY(ECP2 point) {
     FP2 y = point.getY();
     FP2 yNeg = new FP2(y);
@@ -128,7 +128,6 @@ static ECP2 normaliseY(ECP2 point) {
    * @param point the point to be scaled
    * @return a scaled point
    */
-  @VisibleForTesting
   static ECP2 scaleWithCofactor(ECP2 point) {
 
     // These are a representation of the G2 cofactor (a 512 bit number)
@@ -285,8 +284,12 @@ static G2Point fromBytesCompressed(Bytes bytes) {
       throw new IllegalArgumentException("X coordinate is not on the curve.");
     }
 
+    if (!isInGroup(point)) {
+      throw new IllegalArgumentException("The deserialised point is not in the G2 subgroup.");
+    }
+
     // Did we get the right branch of the sqrt?
-    if (!point.is_infinity() && aIn != calculateYFlag(point.getY().getB())) {
+    if (aIn != calculateYFlag(point.getY().getB())) {
       // We didn't: so choose the other branch of the sqrt.
       FP2 x = point.getX();
       FP2 yneg = point.getY();
@@ -297,6 +300,12 @@ static G2Point fromBytesCompressed(Bytes bytes) {
     return new G2Point(point);
   }
 
+  // Verify that the given point is in the correct subgroup for G2 by multiplying by the group order
+  static boolean isInGroup(ECP2 point) {
+    ECP2 orderCheck = point.mul(new BIG(ROM.CURVE_Order));
+    return orderCheck.is_infinity();
+  }
+
   ECP2 ecp2Point() {
     return point;
   }

diff --git a/util/src/test/java/tech/pegasys/artemis/util/mikuli/G1PointTest.java b/util/src/test/java/tech/pegasys/artemis/util/mikuli/G1PointTest.java
@@ -15,16 +15,24 @@
 
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static tech.pegasys.artemis.util.mikuli.G1Point.isInGroup;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.jupiter.api.Test;
 
 class G1PointTest {
 
+  @Test
+  void succeedsWhenRandomPointsAreInTheG1Subgroup() {
+    for (int i = 0; i < 20; i++) {
+      G1Point point = G1Point.random();
+      assertTrue(isInGroup(point.ecpPoint()));
+    }
+  }
+
   @Test
   void succeedsWhenEqualsReturnsTrueForTheSamePoint() {
     G1Point point = G1Point.random();
@@ -73,10 +81,19 @@ void succeedsWhenSerialiseDeserialiseCompressedRoundTripWorks() {
   @Test
   void succeedsWhenDeserialisingACorrectPointDoesNotThrow() {
     String xInput =
-        "0x8123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+        "0xa491d1b0ecd9bb917989f0e74f0dea0422eac4a873e5e2644f368dffb9a6e20fd6e10c1b77654d067c0618f6e5a7f79a";
     assertAll(() -> G1Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
   }
 
+  @Test
+  void succeedsWhenDeserialisingAPointOnCurveButNotInG1ThrowsIllegalArgumentException() {
+    String xInput =
+        "0x8123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> G1Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
+  }
+
   @Test
   void succeedsWhenDeserialisingAnIncorrectPointThrowsIllegalArgumentException() {
     String xInput =
@@ -106,14 +123,6 @@ void succeedsWhenAttemptToDeserialiseXGreaterThanModulusThrowsIllegalArgumentExc
         () -> G1Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
   }
 
-  @Test
-  void succeedsWhenAttemptToDeserialiseXLessThanModulusDoesNotThrowIllegalArgumentException() {
-    // There's a valid X three less than the modulus. We prepend the c flag.
-    String xInput =
-        "0x9a0111ea397fe69a4b1ba7b6434bacd764774b84f38512bf6730d2a0f6b0f6241eabfffeb153ffffb9feffffffffaaa8";
-    assertAll(() -> G1Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
-  }
-
   @Test
   void succeedsWhenProvidingTooFewBytesToFromBytesCompressedThrowsIllegalArgumentException() {
     String xInput =
@@ -135,7 +144,7 @@ void succeedsWhenProvidingTooManyBytesToFromBytesCompressedThrowsIllegalArgument
   @Test
   void succeedsWhenRoundTripDeserialiseSerialiseCompressedReturnsTheOriginalInput() {
     String xInput =
-        "0x8123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+        "0xb301803f8b5ac4a1133581fc676dfedc60d891dd5fa99028805e5ea5b08d3491af75d0707adab3b70c6a6a580217bf81";
     String xOutput =
         G1Point.fromBytesCompressed(Bytes.fromHexString(xInput))
             .toBytesCompressed()
@@ -156,8 +165,9 @@ void succeedsWhenDeserialiseCompressedInfinityWithTrueBFlagCreatesPointAtInfinit
   void succeedsWhenDeserialiseCompressedInfinityWithFalseBFlagDoesNotCreatePointAtInfinity() {
     String xInput =
         "0x800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000";
-    G1Point point = G1Point.fromBytesCompressed(Bytes.fromHexString(xInput));
-    assertFalse(point.ecpPoint().is_infinity());
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> G1Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
   }
 
   @Test

diff --git a/util/src/test/java/tech/pegasys/artemis/util/mikuli/G2PointTest.java b/util/src/test/java/tech/pegasys/artemis/util/mikuli/G2PointTest.java
@@ -18,6 +18,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static tech.pegasys.artemis.util.mikuli.G2Point.isInGroup;
 import static tech.pegasys.artemis.util.mikuli.G2Point.normaliseY;
 import static tech.pegasys.artemis.util.mikuli.G2Point.scaleWithCofactor;
 
@@ -29,6 +30,14 @@
 
 class G2PointTest {
 
+  @Test
+  void succeedsWhenRandomPointsAreInTheG2Subgroup() {
+    for (int i = 0; i < 20; i++) {
+      G2Point point = G2Point.random();
+      assertTrue(isInGroup(point.ecp2Point()));
+    }
+  }
+
   @Test
   void succeedsWhenEqualsReturnsTrueForTheSamePoint() {
     G2Point point = G2Point.random();
@@ -200,11 +209,22 @@ void succeedsWhenAttemptToDeserialiseXImGreaterThanModulusThrowsIllegalArgumentE
 
   @Test
   void succeedsWhenDeserialisingACorrectPointDoesNotThrow() {
+    String xInput =
+        "0x"
+            + "b2cc74bc9f089ed9764bbceac5edba416bef5e73701288977b9cac1ccb6964269d4ebf78b4e8aa7792ba09d3e49c8e6a"
+            + "1351bdf582971f796bbaf6320e81251c9d28f674d720cca07ed14596b96697cf18238e0e03ebd7fc1353d885a39407e0";
+    assertAll(() -> G2Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
+  }
+
+  @Test
+  void succeedsWhenDeserialisingAPointOnCurveButNotInG2ThrowsIllegalArgumentException() {
     String xInput =
         "0x"
             + "8123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
             + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
-    assertAll(() -> G2Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> G2Point.fromBytesCompressed(Bytes.fromHexString(xInput)));
   }
 
   @Test
@@ -244,8 +264,8 @@ void succeedsWhenProvidingTooManyBytesToFromBytesCompressedThrowsIllegalArgument
   void succeedsWhenRoundTripDeserialiseSerialiseCompressedReturnsTheOriginalInput() {
     String xInput =
         "0x"
-            + "8123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
-            + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+            + "b2cc74bc9f089ed9764bbceac5edba416bef5e73701288977b9cac1ccb6964269d4ebf78b4e8aa7792ba09d3e49c8e6a"
+            + "1351bdf582971f796bbaf6320e81251c9d28f674d720cca07ed14596b96697cf18238e0e03ebd7fc1353d885a39407e0";
     String xOutput =
         G2Point.fromBytesCompressed(Bytes.fromHexString(xInput))
             .toBytesCompressed()