[8u] RFR Backport: 8208698: Improved ECC Implementation
Alvarez, David
alvdavi at amazon.com
Fri Jun 14 22:33:23 UTC 2019
Hi,
Please review this backport of JDK-8208698: Improved ECC Implementation
Bug: https://bugs.openjdk.java.net/browse/JDK-8208698
Original: http://hg.openjdk.java.net/jdk/jdk/rev/752e57845ad2
Webrev: http://cr.openjdk.java.net/~phh/8208698/webrev.8u.00/
JDK-8208698 is marked as jdk8u-critical-yes
This is the last of the three patches I was sending today, JDK-8181594, JDK-8208648 and JDK-8208698
This backport is effectively also fixing JDK-8205476. However, JDK-8205476 includes some Javadoc changes and a test this patch is not bringing. If needed, I could backport JDK-8205476 independently and do the webrev, or modify the existing backport to ensure we do not wipe the secret.
There are other reasons why this patch did not apply cleanly
ECDHKeyAgreement.java: These are mostly caused by the missing JDK-8205476
ECDSASignature.java: jdk supports SHA224inP1363Format which we don't. I adapted the patch to ignore P1363 references (P1363 format here means unencoded values)
ECKeyPairGeneration.java: jdk8u is missing JDK-8182999, so the patching of the initialize method had to be written manually.
Additionally, there were some other compilation changes, mostly to accommodate for newer API methods not present in jdk8u like Optional.isEmpty or Map.of
The changes I did to fix rejects are listed below
Thanks,
David
----
diff --git a/src/jdk/src/share/classes/sun/security/ec/ECDHKeyAgreement.java b/src/jdk/src/share/classes/sun/security/ec/ECDHKeyAgreement.java
index e17f8393..5425179f 100644
--- a/src/jdk/src/share/classes/sun/security/ec/ECDHKeyAgreement.java
+++ b/src/jdk/src/share/classes/sun/security/ec/ECDHKeyAgreement.java
@@ -104,40 +104,74 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi {
("Key must be a PublicKey with algorithm EC");
}
- ECPublicKey ecKey = (ECPublicKey)key;
- ECParameterSpec params = ecKey.getParams();
+ this.publicKey = (ECPublicKey) key;
- if (ecKey instanceof ECPublicKeyImpl) {
- publicValue = ((ECPublicKeyImpl)ecKey).getEncodedPublicValue();
- } else { // instanceof ECPublicKey
- publicValue =
- ECUtil.encodePoint(ecKey.getW(), params.getCurve());
- }
+ ECParameterSpec params = publicKey.getParams();
int keyLenBits = params.getCurve().getField().getFieldSize();
secretLen = (keyLenBits + 7) >> 3;
return null;
}
- // see JCE spec
- @Override
- protected byte[] engineGenerateSecret() throws IllegalStateException {
- if ((privateKey == null) || (publicValue == null)) {
- throw new IllegalStateException("Not initialized correctly");
+ private static void validateCoordinate(BigInteger c, BigInteger mod) {
+ if (c.compareTo(BigInteger.ZERO) < 0) {
+ throw new ProviderException("invalid coordinate");
}
- byte[] s = privateKey.getS().toByteArray();
- byte[] encodedParams = // DER OID
- ECUtil.encodeECParameterSpec(null, privateKey.getParams());
+ if (c.compareTo(mod) >= 0) {
+ throw new ProviderException("invalid coordinate");
+ }
+ }
- try {
+ /*
+ * Check whether a public key is valid. Throw ProviderException
+ * if it is not valid or could not be validated.
+ */
+ private static void validate(ECOperations ops, ECPublicKey key) {
+
+ // ensure that integers are in proper range
+ BigInteger x = key.getW().getAffineX();
+ BigInteger y = key.getW().getAffineY();
+
+ BigInteger p = ops.getField().getSize();
+ validateCoordinate(x, p);
+ validateCoordinate(y, p);
+
+ // ensure the point is on the curve
+ EllipticCurve curve = key.getParams().getCurve();
+ BigInteger rhs = x.modPow(BigInteger.valueOf(3), p).add(curve.getA()
+ .multiply(x)).add(curve.getB()).mod(p);
+ BigInteger lhs = y.modPow(BigInteger.valueOf(2), p).mod(p);
+ if (!rhs.equals(lhs)) {
+ throw new ProviderException("point is not on curve");
+ }
- return deriveKey(s, publicValue, encodedParams);
+ // check the order of the point
+ ImmutableIntegerModuloP xElem = ops.getField().getElement(x);
+ ImmutableIntegerModuloP yElem = ops.getField().getElement(y);
+ AffinePoint affP = new AffinePoint(xElem, yElem);
+ byte[] order = key.getParams().getOrder().toByteArray();
+ ArrayUtil.reverse(order);
+ Point product = ops.multiply(affP, order);
+ if (!ops.isNeutral(product)) {
+ throw new ProviderException("point has incorrect order");
+ }
- } catch (GeneralSecurityException e) {
- throw new ProviderException("Could not derive key", e);
+ }
+
+ // see JCE spec
+ @Override
+ protected byte[] engineGenerateSecret() throws IllegalStateException {
+ if ((privateKey == null) || (publicKey == null)) {
+ throw new IllegalStateException("Not initialized correctly");
}
+ Optional<byte[]> resultOpt = deriveKeyImpl(privateKey, publicKey);
+ byte[] result = resultOpt.orElseGet(
+ () -> deriveKeyNative(privateKey, publicKey)
+ );
+ publicKey = null;
+ return result;
}
// see JCE spec
@@ -175,7 +209,7 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi {
ECParameterSpec ecSpec = priv.getParams();
EllipticCurve curve = ecSpec.getCurve();
Optional<ECOperations> opsOpt = ECOperations.forParameters(ecSpec);
- if (opsOpt.isEmpty()) {
+ if (!opsOpt.isPresent()) {
return Optional.empty();
}
ECOperations ops = opsOpt.get();
diff --git a/src/jdk/src/share/classes/sun/security/ec/ECDSASignature.java b/src/jdk/src/share/classes/sun/security/ec/ECDSASignature.java
index de98f3b4..89f527e5 100644
--- a/src/jdk/src/share/classes/sun/security/ec/ECDSASignature.java
+++ b/src/jdk/src/share/classes/sun/security/ec/ECDSASignature.java
@@ -169,7 +169,7 @@ abstract class ECDSASignature extends SignatureSpi {
// Nested class for SHA224withECDSA signatures
public static final class SHA224 extends ECDSASignature {
public SHA224() {
- super("SHA-224");
+ super("SHA-224");
}
}
@@ -312,7 +312,7 @@ abstract class ECDSASignature extends SignatureSpi {
int seedBits = params.getOrder().bitLength() + 64;
Optional<ECDSAOperations> opsOpt =
ECDSAOperations.forParameters(params);
- if (opsOpt.isEmpty()) {
+ if (!opsOpt.isPresent()) {
return Optional.empty();
} else {
byte[] sig = signDigestImpl(opsOpt.get(), seedBits, digest,
@@ -342,14 +342,33 @@ abstract class ECDSASignature extends SignatureSpi {
timingArgument |= 1;
try {
- return encodeSignature(
- signDigest(getDigestValue(), s, encodedParams, seed,
- timingArgument));
+ return signDigest(digest, s, encodedParams, seed,
+ timingArgument);
} catch (GeneralSecurityException e) {
throw new SignatureException("Could not sign data", e);
}
}
+ // sign the data and return the signature. See JCA doc
+ @Override
+ protected byte[] engineSign() throws SignatureException {
+
+ if (random == null) {
+ random = JCAUtil.getSecureRandom();
+ }
+
+ byte[] digest = getDigestValue();
+ Optional<byte[]> sigOpt = signDigestImpl(privateKey, digest, random);
+ byte[] sig;
+ if (sigOpt.isPresent()) {
+ sig = sigOpt.get();
+ } else {
+ sig = signDigestNative(privateKey, digest, random);
+ }
+
+ return encodeSignature(sig);
+ }
+
// verify the data and return the result. See JCA doc
@Override
protected boolean engineVerify(byte[] signature) throws SignatureException {
diff --git a/src/jdk/src/share/classes/sun/security/ec/ECKeyPairGenerator.java b/src/jdk/src/share/classes/sun/security/ec/ECKeyPairGenerator.java
index ad78d7b4..0f9b5eb2 100644
--- a/src/jdk/src/share/classes/sun/security/ec/ECKeyPairGenerator.java
+++ b/src/jdk/src/share/classes/sun/security/ec/ECKeyPairGenerator.java
@@ -31,14 +31,16 @@ import java.security.spec.AlgorithmParameterSpec;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.ECParameterSpec;
import java.security.spec.ECPoint;
+import java.security.spec.InvalidParameterSpecException;
+import java.security.spec.*;
+import java.util.Optional;
-import sun.security.ec.NamedCurve;
-import sun.security.ec.ECParameters;
-import sun.security.ec.ECPrivateKeyImpl;
-import sun.security.ec.ECPublicKeyImpl;
import sun.security.jca.JCAUtil;
import sun.security.util.ECUtil;
+import sun.security.util.math.*;
+import sun.security.ec.point.*;
import static sun.security.util.SecurityProviderConstants.DEF_EC_KEY_SIZE;
+import static sun.security.ec.ECOperations.IntermediateValueException;
/**
* EC keypair generator.
@@ -86,17 +88,19 @@ public final class ECKeyPairGenerator extends KeyPairGeneratorSpi {
public void initialize(AlgorithmParameterSpec params, SecureRandom random)
throws InvalidAlgorithmParameterException {
+ ECParameterSpec ecSpec = null;
+
if (params instanceof ECParameterSpec) {
- this.params = ECUtil.getECParameterSpec(null,
- (ECParameterSpec)params);
+ ECParameterSpec ecParams = (ECParameterSpec) params;
+ ecSpec = ECUtil.getECParameterSpec(null, ecParams);
if (this.params == null) {
throw new InvalidAlgorithmParameterException(
"Unsupported curve: " + params);
}
} else if (params instanceof ECGenParameterSpec) {
- String name = ((ECGenParameterSpec)params).getName();
- this.params = ECUtil.getECParameterSpec(null, name);
- if (this.params == null) {
+ String name = ((ECGenParameterSpec) params).getName();
+ ecSpec = ECUtil.getECParameterSpec(null, name);
+ if (ecSpec == null) {
throw new InvalidAlgorithmParameterException(
"Unknown curve name: " + name);
}
@@ -104,8 +108,9 @@ public final class ECKeyPairGenerator extends KeyPairGeneratorSpi {
throw new InvalidAlgorithmParameterException(
"ECParameterSpec or ECGenParameterSpec required for EC");
}
- this.keySize =
- ((ECParameterSpec)this.params).getCurve().getField().getFieldSize();
+ this.params = ecSpec;
+
+ this.keySize = ecSpec.getCurve().getField().getFieldSize();
this.random = random;
}
@@ -154,7 +159,7 @@ public final class ECKeyPairGenerator extends KeyPairGeneratorSpi {
ECParameterSpec ecParams = (ECParameterSpec) params;
Optional<ECOperations> opsOpt = ECOperations.forParameters(ecParams);
- if (opsOpt.isEmpty()) {
+ if (!opsOpt.isPresent()) {
return Optional.empty();
}
ECOperations ops = opsOpt.get();
diff --git a/src/jdk/src/share/classes/sun/security/ec/ECOperations.java b/src/jdk/src/share/classes/sun/security/ec/ECOperations.java
index 2995ef75..c9414006 100644
--- a/src/jdk/src/share/classes/sun/security/ec/ECOperations.java
+++ b/src/jdk/src/share/classes/sun/security/ec/ECOperations.java
@@ -34,6 +34,7 @@ import java.security.ProviderException;
import java.security.spec.ECFieldFp;
import java.security.spec.ECParameterSpec;
import java.security.spec.EllipticCurve;
+import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
@@ -55,17 +56,19 @@ public class ECOperations {
private static final long serialVersionUID = 1;
}
- static final Map<BigInteger, IntegerFieldModuloP> fields = Map.of(
- IntegerPolynomialP256.MODULUS, new IntegerPolynomialP256(),
- IntegerPolynomialP384.MODULUS, new IntegerPolynomialP384(),
- IntegerPolynomialP521.MODULUS, new IntegerPolynomialP521()
- );
-
- static final Map<BigInteger, IntegerFieldModuloP> orderFields = Map.of(
- P256OrderField.MODULUS, new P256OrderField(),
- P384OrderField.MODULUS, new P384OrderField(),
- P521OrderField.MODULUS, new P521OrderField()
- );
+ static final Map<BigInteger, IntegerFieldModuloP> fields = new HashMap<>();
+ static {
+ fields.put(IntegerPolynomialP256.MODULUS, new IntegerPolynomialP256());
+ fields.put(IntegerPolynomialP384.MODULUS, new IntegerPolynomialP384());
+ fields.put(IntegerPolynomialP521.MODULUS, new IntegerPolynomialP521());
+ }
+
+ static final Map<BigInteger, IntegerFieldModuloP> orderFields = new HashMap<>();
+ static {
+ orderFields.put(P256OrderField.MODULUS, new P256OrderField());
+ orderFields.put(P384OrderField.MODULUS, new P384OrderField());
+ orderFields.put(P521OrderField.MODULUS, new P521OrderField());
+ }
public static Optional<ECOperations> forParameters(ECParameterSpec params) {
More information about the jdk8u-dev
mailing list