- * 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. *
- * 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() @@ -171,12 +148,18 @@ public boolean isMember(FieldElement x, FieldElement y) { 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} + *
+ * 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); } diff --git a/src/main/java/org/cryptimeleon/math/structures/groups/elliptic/WeierstrassCurve.java b/src/main/java/org/cryptimeleon/math/structures/groups/elliptic/WeierstrassCurve.java index 9b1bc82e..50b3203a 100644 --- a/src/main/java/org/cryptimeleon/math/structures/groups/elliptic/WeierstrassCurve.java +++ b/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(); } From 3c886faa32c445c65bed7c631c13276c2b6aa61d Mon Sep 17 00:00:00 2001 From: Raphael Heitjohann <5891816+rheitjoh@users.noreply.github.com> Date: Wed, 18 Aug 2021 09:33:30 +0200 Subject: [PATCH 2/2] Add isOnCurve test --- .../math/structures/CurveTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/test/java/org/cryptimeleon/math/structures/CurveTest.java diff --git a/src/test/java/org/cryptimeleon/math/structures/CurveTest.java b/src/test/java/org/cryptimeleon/math/structures/CurveTest.java new file mode 100644 index 00000000..dcae3f98 --- /dev/null +++ b/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()))); + } +}