Implements curve and subgroup checks

src/main/java/org/cryptimeleon/math/structures/groups/elliptic/AbstractEllipticCurvePoint.java

@@ -17,10 +17,24 @@ public AbstractEllipticCurvePoint(WeierstrassCurve curve, FieldElement x, FieldE
         this.z = z;
     }
 
+    /**
+     * Deserializes a curve element.
+     *
+     * @throws IllegalArgumentException if the point is not on the curve.
+     */
     public AbstractEllipticCurvePoint(WeierstrassCurve curve, Representation repr) {
-        this(curve, curve.getFieldOfDefinition().restoreElement(repr.obj().get("x")),
-                curve.getFieldOfDefinition().restoreElement(repr.obj().get("y")),
-                curve.getFieldOfDefinition().restoreElement(repr.obj().get("z")));
+        // first we have to instantiate the point so we can normalize
+        FieldElement x = curve.getFieldOfDefinition().restoreElement(repr.obj().get("x"));
+        FieldElement y = curve.getFieldOfDefinition().restoreElement(repr.obj().get("y"));
+        FieldElement z = curve.getFieldOfDefinition().restoreElement(repr.obj().get("z"));
+        // check whether z is zero (neutral element) first so we can potentially save curve check
+        if (!z.isZero() && !curve.isOnCurve(x, y)) {
+            throw new IllegalArgumentException("Point cannot be deserialized as it is not on the given curve");
+        }
+        this.structure = curve;
+        this.x = x;
+        this.y = y;
+        this.z = z;
     }
 
     public Field getFieldOfDefinition() {

src/main/java/org/cryptimeleon/math/structures/groups/elliptic/PairingSourceGroupImpl.java

@@ -130,53 +130,36 @@ public Representation getRepresentation() {
         return or;
     }
 
-    /**
-     * Tests if (x,y) is on curve that defines this group. Does not check subgroup membership.
-     *
-     * @param x - x-coordinate of point that shall be checked
-     * @param y - y-coordinate of point that shall be checked
-     * @return true if p fulfills equation of this group
-     */
-    public boolean isOnCurve(FieldElement x, FieldElement y) {
-        // FieldElement x,y;
-        //
-        // x = p.getX();
-        // y = p.getY();
-
-        /*
-         * check y^2+a_1 xy + a_3 y = x^3+a_2 x^2 + a_4 x + a_6
-         *
-         * rewritten as
-         *
-         * ((a_1 x + a_3)y + y)y = x ( x ( x+a_2 )+a_4)+a_6
-         */
-        return x.mul(getA1()).add(getA3()).mul(y).add(y).mul(y).equals(x.add(getA2()).mul(x).add(getA4()).mul(x).add(getA6()));
-    }
-
     /**
      * Tests if (x,y) is a member of this (sub)group.
      * <p>
-     * This function first checks of (x,y) defines a point on the curve that defines this group. Then a subgroup membership test is performed by multiplication either with the group order or with the cofactor. If both are large, this is an expensive
-     * operation.
+     * Does NOT check whether point is on the curve. This needs to be done separately before.
      * <p>
-     * For cryptographic protocols where x and y are inputs to the algorithm, a subgroup membership test is mandatory to avoid small subgroup attacks, twist attacks,...
+     * For cryptographic protocols where x and y are inputs to the algorithm,
+     * a subgroup membership test is mandatory to avoid small subgroup attacks, twist attacks, etc.
      *
-     * @param x - x-coordinate of point to be checked
-     * @param y - y-coordinate of point to be checked
-     * @return true if (x,y) is on curve
+     * @param x x-coordinate of point to be checked
+     * @param y y-coordinate of point to be checked
+     * @return true if (x,y) is member of this (sub)group, false otherwise
      */
     public boolean isMember(FieldElement x, FieldElement y) {
         //Ensure there is only one subgroup of size this.size()
         if (!this.size().gcd(this.getCofactor()).equals(BigInteger.ONE)) {
             throw new IllegalArgumentException("Require cofactor coprime to order of subgroup.");
         }
 
-        //Check if point is on curve
-        if (!isOnCurve(x, y))
-            return false;
-
-        //Check subgroup membership
-        return this.getElement(x, y).pow(this.size()).isNeutralElement();
+        // Check subgroup membership by exponentiating with subgroup order
+        // Need custom exponentiation since pow() could have special handling for size() parameter
+        //   e.g. hardcoded 1 (since it may assume membership in group already)
+        PairingSourceGroupElement elem = getElement(x, y);
+        BigInteger size = this.size();
+        GroupElementImpl result = getNeutralElement();
+        for (int i = size.bitLength() - 1; i >= 0; i--) {
+            result = result.op(result);
+            if (size.testBit(i))
+                result = result.op(elem);
+        }
+        return result.isNeutralElement();
     }
 
     public Field getFieldOfDefinition() {
@@ -210,6 +193,15 @@ public PairingSourceGroupElement getUniformlyRandomElement() throws UnsupportedO
         return (PairingSourceGroupElement) this.getGenerator().pow(zp.getUniformlyRandomElement().asInteger());
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * Checks that the deserialized point is on the curve and in this (sub)group.
+     * If not, a {@link IllegalArgumentException} is thrown.
+     *
+     * @throws IllegalArgumentException if the deserialized point is either not on the curve or not member of this
+     *                                  (sub)group.
+     */
     @Override
     public PairingSourceGroupElement restoreElement(Representation repr) {
         ObjectRepresentation or = (ObjectRepresentation) repr;
@@ -218,7 +210,14 @@ public PairingSourceGroupElement restoreElement(Representation repr) {
         FieldElement z = getFieldOfDefinition().restoreElement(or.get("z"));
         if (z.isZero())
             return (PairingSourceGroupElement) getNeutralElement();
-
+        // Check that point is on this curve
+        if (!isOnCurve(x, y)) {
+            throw new IllegalArgumentException("Point is not on the curve underlying this group");
+        }
+        // Check that point is in this group
+        if (!isMember(x, y)) {
+            throw new IllegalArgumentException("Element specified by given representation is not member of this group");
+        }
         return getElement(x, y);
     }
 

src/main/java/org/cryptimeleon/math/structures/groups/elliptic/WeierstrassCurve.java

@@ -56,6 +56,30 @@ public interface WeierstrassCurve extends EllipticCurve {
      */
     EllipticCurvePoint getElement(FieldElement x, FieldElement y);
 
+    /**
+     * Checks whether the given point is on this curve.
+     * 
+     * @param x the x-coordinate of the point to check
+     * @param y the y-coordinate of the point to check
+     * @return true if the point is on this curve, false otherwise
+     */
+    default boolean isOnCurve(FieldElement x, FieldElement y) {
+        // FieldElement x,y;
+        //
+        // x = p.getX();
+        // y = p.getY();
+
+        /*
+         * check y^2+a_1 xy + a_3 y = x^3+a_2 x^2 + a_4 x + a_6
+         *
+         * rewritten as
+         *
+         * ((a_1 x + a_3)y + y)y = x ( x ( x+a_2 )+a_4)+a_6
+         */
+        return x.mul(getA1()).add(getA3()).mul(y).add(y).mul(y)
+                .equals(x.add(getA2()).mul(x).add(getA4()).mul(x).add(getA6()));
+    }
+
     default boolean isShortForm() {
         return getA3().isZero() && getA2().isZero() && getA1().isZero();
     }

src/test/java/org/cryptimeleon/math/structures/CurveTest.java

@@ -0,0 +1,37 @@
+package org.cryptimeleon.math.structures;
+
+import org.cryptimeleon.math.serialization.BigIntegerRepresentation;
+import org.cryptimeleon.math.serialization.Representation;
+import org.cryptimeleon.math.structures.groups.Group;
+import org.cryptimeleon.math.structures.groups.GroupImpl;
+import org.cryptimeleon.math.structures.groups.elliptic.AbstractEllipticCurvePoint;
+import org.cryptimeleon.math.structures.groups.elliptic.AffineEllipticCurvePoint;
+import org.cryptimeleon.math.structures.groups.elliptic.PairingSourceGroupImpl;
+import org.cryptimeleon.math.structures.groups.elliptic.WeierstrassCurve;
+import org.cryptimeleon.math.structures.groups.elliptic.nopairing.Secp256k1;
+import org.cryptimeleon.math.structures.groups.elliptic.type3.bn.BarretoNaehrigBilinearGroup;
+import org.cryptimeleon.math.structures.groups.elliptic.type3.bn.BarretoNaehrigParameterSpec;
+import org.cryptimeleon.math.structures.rings.FieldElement;
+import org.junit.Test;
+
+import java.math.BigInteger;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class CurveTest {
+
+    @Test
+    public void testIsOnCurve() {
+        Secp256k1 curve = new Secp256k1();
+        WeierstrassCurve curveImpl = ((WeierstrassCurve) curve.getImpl());
+        FieldElement x = curveImpl.getFieldOfDefinition().restoreElement(new BigIntegerRepresentation(
+                new BigInteger("67666341147119015517745455968511312184352216481177330889575508950261290521184")
+        ));
+        FieldElement y = curveImpl.getFieldOfDefinition().restoreElement(new BigIntegerRepresentation(
+                new BigInteger("10255259455662915565452952018722490464546702313657804382412957617802230297684")
+        ));
+        assertTrue(curveImpl.isOnCurve(x, y));
+        // now check invalid point
+        assertFalse(curveImpl.isOnCurve(x, y.add(curveImpl.getFieldOfDefinition().getOneElement())));
+    }
+}