Skip to content

Commit

Permalink
check if public key is a point on the curve
Browse files Browse the repository at this point in the history
  • Loading branch information
@overheadhunter
overheadhunter committed Dec 8, 2021
1 parent 6b62ddf commit f83db3d
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 10 deletions.
39 changes: 37 additions & 2 deletions src/main/java/org/cryptomator/cryptolib/common/ECKeyPair.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,16 @@
import com.google.common.base.Preconditions;

import javax.security.auth.Destroyable;
import java.math.BigInteger;
import java.security.KeyPair;
import java.security.MessageDigest;
import java.security.PublicKey;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.security.spec.ECFieldFp;
import java.security.spec.ECParameterSpec;
import java.security.spec.ECPoint;
import java.security.spec.EllipticCurve;
import java.util.Arrays;
import java.util.Objects;

Expand All @@ -15,10 +21,10 @@ public class ECKeyPair implements Destroyable {
private final KeyPair keyPair;
private boolean destroyed;

ECKeyPair(KeyPair keyPair) {
ECKeyPair(KeyPair keyPair, ECParameterSpec curveParams) {
Preconditions.checkArgument(keyPair.getPrivate() instanceof ECPrivateKey);
Preconditions.checkArgument(keyPair.getPublic() instanceof ECPublicKey);
this.keyPair = keyPair;
this.keyPair = verify(keyPair, curveParams);
}

public KeyPair keyPair() {
Expand All @@ -37,6 +43,35 @@ public ECPublicKey getPublic() {
return (ECPublicKey) keyPair.getPublic();
}

// validations taken from https://neilmadden.blog/2017/05/17/so-how-do-you-validate-nist-ecdh-public-keys/
private static KeyPair verify(KeyPair keyPair, ECParameterSpec curveParams) {
PublicKey pk = keyPair.getPublic();
Preconditions.checkArgument(pk instanceof ECPublicKey, "Not an EC key");
Preconditions.checkArgument(curveParams.getCofactor() == 1, "Verifying points on curves with cofactor not supported"); // see "Step 4" in linked post
ECPublicKey publicKey = (ECPublicKey) pk;
EllipticCurve curve = curveParams.getCurve();

// Step 1: Verify public key is not point at infinity.
Preconditions.checkArgument(!ECPoint.POINT_INFINITY.equals(publicKey.getW()), "Invalid Key");

final BigInteger x = publicKey.getW().getAffineX();
final BigInteger y = publicKey.getW().getAffineY();
final BigInteger p = ((ECFieldFp) curve.getField()).getP();

// Step 2: Verify x and y are in range [0,p-1]
Preconditions.checkArgument(x.compareTo(BigInteger.ZERO) >= 0 && x.compareTo(p) < 0, "Invalid Key");
Preconditions.checkArgument(y.compareTo(BigInteger.ZERO) >= 0 && y.compareTo(p) < 0, "Invalid Key");

// Step 3: Verify that y^2 == x^3 + ax + b (mod p)
final BigInteger a = curve.getA();
final BigInteger b = curve.getB();
final BigInteger ySquared = y.modPow(BigInteger.valueOf(2), p);
final BigInteger xCubedPlusAXPlusB = x.modPow(BigInteger.valueOf(3), p).add(a.multiply(x)).add(b).mod(p);
Preconditions.checkArgument(ySquared.equals(xCubedPlusAXPlusB), "Invalid key");

return keyPair;
}

@Override
public boolean isDestroyed() {
return destroyed;
Expand Down
17 changes: 16 additions & 1 deletion src/main/java/org/cryptomator/cryptolib/common/P384KeyPair.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.security.AlgorithmParameters;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.ECParameterSpec;
import java.security.spec.InvalidParameterSpecException;

public class P384KeyPair extends ECKeyPair {

Expand All @@ -20,7 +23,7 @@ public class P384KeyPair extends ECKeyPair {
private static final String SIGNATURE_ALG = "SHA384withECDSA";

private P384KeyPair(KeyPair keyPair) {
super(keyPair);
super(keyPair, getCurveParams());
}

public static P384KeyPair generate() {
Expand Down Expand Up @@ -97,4 +100,16 @@ private static KeyPairGenerator getKeyPairGenerator() {
}
}

private static ECParameterSpec getCurveParams() {
try {
AlgorithmParameters parameters = AlgorithmParameters.getInstance(EC_ALG);
parameters.init(new ECGenParameterSpec(EC_CURVE_NAME));
return parameters.getParameterSpec(ECParameterSpec.class);
} catch (NoSuchAlgorithmException | InvalidParameterSpecException e) {
throw new IllegalStateException(EC_CURVE_NAME + " curve not supported");
}
}



}
93 changes: 86 additions & 7 deletions src/test/java/org/cryptomator/cryptolib/common/ECKeyPairTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,36 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.mockito.Mockito;

import java.math.BigInteger;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.security.spec.ECFieldFp;
import java.security.spec.ECParameterSpec;
import java.security.spec.ECPoint;
import java.security.spec.EllipticCurve;

public class ECKeyPairTest {

@Test
public void testConstructorFailsForInvalidAlgorithm() throws NoSuchAlgorithmException {
KeyPair rsaKeyPair = KeyPairGenerator.getInstance("RSA").generateKeyPair();
ECParameterSpec curveParams = Mockito.mock(ECParameterSpec.class);
Assertions.assertThrows(IllegalArgumentException.class, () -> {
new ECKeyPair(rsaKeyPair);
new ECKeyPair(rsaKeyPair, curveParams);
});
}

private ECParameterSpec getParamsFromPublicKey(KeyPair keyPair) {
return ((ECPublicKey)keyPair.getPublic()).getParams();
}

@Nested
@DisplayName("With undestroyed key...")
public class WithUndestroyed {
Expand All @@ -32,7 +47,7 @@ public class WithUndestroyed {
public void setup() throws NoSuchAlgorithmException {
this.keyPair1 = KeyPairGenerator.getInstance("EC").generateKeyPair();
this.keyPair2 = KeyPairGenerator.getInstance("EC").generateKeyPair();
this.ecKeyPair = new ECKeyPair(keyPair1);
this.ecKeyPair = new ECKeyPair(keyPair1, getParamsFromPublicKey(keyPair1));
}

@Test
Expand All @@ -57,8 +72,8 @@ public void testDestroy() {

@Test
public void testEquals() {
ECKeyPair other1 = new ECKeyPair(keyPair1);
ECKeyPair other2 = new ECKeyPair(keyPair2);
ECKeyPair other1 = new ECKeyPair(keyPair1, getParamsFromPublicKey(keyPair1));
ECKeyPair other2 = new ECKeyPair(keyPair2, getParamsFromPublicKey(keyPair2));
Assertions.assertNotSame(ecKeyPair, other1);
Assertions.assertEquals(ecKeyPair, other1);
Assertions.assertNotSame(ecKeyPair, other2);
Expand All @@ -67,8 +82,8 @@ public void testEquals() {

@Test
public void testHashCode() {
ECKeyPair other1 = new ECKeyPair(keyPair1);
ECKeyPair other2 = new ECKeyPair(keyPair2);
ECKeyPair other1 = new ECKeyPair(keyPair1, getParamsFromPublicKey(keyPair1));
ECKeyPair other2 = new ECKeyPair(keyPair2, getParamsFromPublicKey(keyPair2));
Assertions.assertEquals(ecKeyPair.hashCode(), other1.hashCode());
Assertions.assertNotEquals(ecKeyPair.hashCode(), other2.hashCode());
}
Expand All @@ -85,7 +100,7 @@ public class WithDestroyed {
@BeforeEach
public void setup() throws NoSuchAlgorithmException {
this.keyPair = KeyPairGenerator.getInstance("EC").generateKeyPair();
this.ecKeyPair = new ECKeyPair(keyPair);
this.ecKeyPair = new ECKeyPair(keyPair, getParamsFromPublicKey(keyPair));
this.ecKeyPair.destroy();
}

Expand All @@ -111,5 +126,69 @@ public void testDestroy() {

}

@Nested
@DisplayName("With invalid public key...")
public class WithInvalidPublicKey {

private ECParameterSpec curveParams = Mockito.mock(ECParameterSpec.class);
private EllipticCurve curve = Mockito.mock(EllipticCurve.class);
private ECFieldFp field = Mockito.mock(ECFieldFp.class);
private ECPublicKey publicKey = Mockito.mock(ECPublicKey.class);
private ECPrivateKey privateKey = Mockito.mock(ECPrivateKey.class);
private KeyPair keyPair = new KeyPair(publicKey, privateKey);

@BeforeEach
public void setup() {
Mockito.doReturn(curve).when(curveParams).getCurve();
Mockito.doReturn(field).when(curve).getField();
Mockito.doReturn(BigInteger.ZERO).when(curve).getA();
Mockito.doReturn(BigInteger.ZERO).when(curve).getB();
Mockito.doReturn(1).when(curveParams).getCofactor();
Mockito.doReturn(new ECPoint(BigInteger.ONE, BigInteger.ONE)).when(publicKey).getW();
Mockito.doReturn(BigInteger.valueOf(2)).when(field).getP();
}

@Test
public void testValid() {
Assertions.assertDoesNotThrow(() -> new ECKeyPair(keyPair, curveParams));
}

@Test
public void testUnsupportedCofactor() {
Mockito.doReturn(2).when(curveParams).getCofactor();
Assertions.assertThrows(IllegalArgumentException.class, () -> new ECKeyPair(keyPair, curveParams));
}

@Test
public void testInfiniteW() {
Mockito.doReturn(ECPoint.POINT_INFINITY).when(publicKey).getW();
Assertions.assertThrows(IllegalArgumentException.class, () -> new ECKeyPair(keyPair, curveParams));
}

@ParameterizedTest
@CsvSource(value = {
"-1, 0",
"0, -1",
"2, 0",
"0, 2",
})
public void testInvalidPoint(int x, int y) {
Mockito.doReturn(new ECPoint(BigInteger.valueOf(x), BigInteger.valueOf(y))).when(publicKey).getW();
Assertions.assertThrows(IllegalArgumentException.class, () -> new ECKeyPair(keyPair, curveParams));
}

@ParameterizedTest
@CsvSource(value = {
"1, 0",
"0, 1",
})
public void testInvalidCurveCoefficients(int a, int b) {
Mockito.doReturn(BigInteger.valueOf(a)).when(curve).getA();
Mockito.doReturn(BigInteger.valueOf(b)).when(curve).getB();
Assertions.assertThrows(IllegalArgumentException.class, () -> new ECKeyPair(keyPair, curveParams));
}

}


}

0 comments on commit f83db3d

Please sign in to comment.