Skip to content

Disable a few EC curves in computing shared secret #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+136 −42
Open

Disable a few EC curves in computing shared secret #826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/main/java/com/ibm/crypto/plus/provider/ECDHKeyAgreement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.crypto.SecretKey;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.SecretKeySpec;
import sun.security.util.ObjectIdentifier;

public final class ECDHKeyAgreement extends KeyAgreementSpi { // implements
// AlgorithmStatus
Expand All @@ -44,6 +45,7 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi { // implements
private ECPublicKey ecPublicKey = null;
private ECPrivateKey ecPrivateKey = null;
private int secretLen;
private static boolean disableSmallCurve = Boolean.parseBoolean(System.getProperty("openjceplus.disableSmallerECKeySizeForSharedKeyComputing", "true"));

public ECDHKeyAgreement(OpenJCEPlusProvider provider) {
// System.out.println ("In ECDHKeyAgreement");
Expand Down Expand Up @@ -164,6 +166,29 @@ protected byte[] engineGenerateSecret() throws IllegalStateException {
throw new IllegalStateException("Wrong state");
}

if (provider.isFIPS() && disableSmallCurve) {
ECNamedCurve ecPubKeyNamedCurve = ECParameters.getNamedCurve(this.ecPublicKey.getParams());
ECNamedCurve ecPriKeyNamedCurve = ECParameters.getNamedCurve(this.ecPrivateKey.getParams());
if (ecPubKeyNamedCurve == null || ecPriKeyNamedCurve == null) {
throw new IllegalStateException("Curve name is not recognized or not supported");
}
ObjectIdentifier oidPubKey = ECNamedCurve.getOIDFromName(ecPubKeyNamedCurve.getName());
ObjectIdentifier oidPriKey = ECNamedCurve.getOIDFromName(ecPriKeyNamedCurve.getName());
Copy link
Member

@johnpeck-us-ibm johnpeck-us-ibm Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other check is to make sure that the curves are the same and the OIDs are the same. They should not be be different.

Copy link
Collaborator Author

@JinhangZhang JinhangZhang Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, either public or private, which one you think could be more reasonable should be left here?

Copy link
Member

@johnpeck-us-ibm johnpeck-us-ibm Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The curve name and OID should mean the same thing. So., since the curve name is first I would use that to do the comparison to validate the keys are compatible.

Copy link
Collaborator Author

@JinhangZhang JinhangZhang Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that ecPubKeyNamedCurve.getName() will return secp224r1 but will return NIST P-256/NIST P-384/NIST P-521 for others, open another issue to track this different behaviour. In this case, will use the OID instead


// check if curve is FIPS allowance and if it is allowed for ECDH shared secret computation
if (!((oidPubKey.toString()).equals("1.2.840.10045.3.1.7") // secp256r1 [NIST P-256, X9.62 prime256v1]
|| (oidPubKey.toString()).equals("1.3.132.0.33") // secp224r1 [NIST P-224]
|| (oidPubKey.toString()).equals("1.3.132.0.34") // secp384r1 [NIST P-384]
|| (oidPubKey.toString()).equals("1.3.132.0.35")) ||
!((oidPriKey.toString()).equals("1.2.840.10045.3.1.7") // secp256r1 [NIST P-256, X9.62 prime256v1]
|| (oidPriKey.toString()).equals("1.3.132.0.33") // secp224r1 [NIST P-224]
|| (oidPriKey.toString()).equals("1.3.132.0.34") // secp384r1 [NIST P-384]
|| (oidPriKey.toString()).equals("1.3.132.0.35")) // secp521r1 [NIST P-521]
) {
throw new IllegalStateException(ecPubKeyNamedCurve.getName() + " curve is not supported in FIPS for calculating the shared secret");
}
}

// Reset the key agreement here (in case anything goes wrong)
generateSecret = false;
byte[] secret = null;
Expand Down
101 changes: 60 additions & 41 deletions src/test/java/ibm/jceplus/junit/base/BaseTestECDH.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@
public class BaseTestECDH extends BaseTestJunit5 {

static final byte[] origMsg = "this is the original message to be signed".getBytes();
static ECGenParameterSpec algParameterSpec_192k1, algParameterSpec_256r1;
static ECGenParameterSpec algParameterSpec_192r1, algParameterSpec_256r1;

static KeyPairGenerator kpgA = null;
static KeyPairGenerator kpgB = null;

static KeyPair keyPairA_192k1, keyPairA_256r1;
static KeyPair keyPairB_192k1, keyPairB_256r1;
static KeyPair keyPairA_192r1, keyPairA_256r1;
static KeyPair keyPairB_192r1, keyPairB_256r1;

private boolean isMulti = false;

Expand All @@ -70,21 +70,21 @@ synchronized static void generateParams(String provider_name) {
try {

//String provider_name = "OpenJCEPlus";
String curveName_192k1 = "secp192k1";
String curveName_192r1 = "secp192r1";
String curveName_256r1 = "secp256r1";

algParameterSpec_192k1 = new ECGenParameterSpec(curveName_192k1);
algParameterSpec_192r1 = new ECGenParameterSpec(curveName_192r1);
algParameterSpec_256r1 = new ECGenParameterSpec(curveName_256r1);

kpgA = KeyPairGenerator.getInstance("EC", provider_name);
kpgA.initialize(algParameterSpec_192k1);
keyPairA_192k1 = kpgA.generateKeyPair();
kpgA.initialize(algParameterSpec_192r1);
keyPairA_192r1 = kpgA.generateKeyPair();
kpgA.initialize(algParameterSpec_256r1);
keyPairA_256r1 = kpgA.generateKeyPair();

kpgB = KeyPairGenerator.getInstance("EC", provider_name);
kpgB.initialize(algParameterSpec_192k1);
keyPairB_192k1 = kpgB.generateKeyPair();
kpgB.initialize(algParameterSpec_192r1);
keyPairB_192r1 = kpgB.generateKeyPair();
kpgB.initialize(algParameterSpec_256r1);
keyPairB_256r1 = kpgB.generateKeyPair();

Expand Down Expand Up @@ -112,15 +112,15 @@ public void setUp() {
* @throws Exception
*/
@Test
public void testECDH_secp192k1() throws Exception {
public void testECDH_secp192r1() throws Exception {

String curveName = "secp192k1";
String curveName = "secp192r1";

ECGenParameterSpec ecgn = new ECGenParameterSpec(curveName);

compute_ecdh_key(curveName, ecgn);
if (isMulti)
compute_ecdh_key_with_global_key(curveName, algParameterSpec_192k1);
compute_ecdh_key_with_global_key(curveName, algParameterSpec_192r1);

}

Expand All @@ -142,27 +142,34 @@ public void testECDH_ECSpec() throws Exception {

String methodId = "ECDHECParamSpec";

// These values were copied from ECNamedCurve.java in IBMJCEFIPS
String sfield_p256 = "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F";
String sa_p256 = "0000000000000000000000000000000000000000000000000000000000000000";
String sb_p256 = "0000000000000000000000000000000000000000000000000000000000000007";
String sx_p256 = "79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798";
String sy_p256 = "483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8";
String sorder_p256 = "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141";
// secp256r1 / prime256v1 (NIST P-256)
String sfield_p256r1 = "FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF";
String sa_p256r1 = "FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC";
String sb_p256r1 = "5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B";
String sx_p256r1 = "6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296";
String sy_p256r1 = "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5";
String sorder_p256r1 = "FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551";

BigInteger p = new BigInteger(sfield_p256, 16);
BigInteger p = new BigInteger(sfield_p256r1, 16);
ECField field = new ECFieldFp(p);

EllipticCurve curve = new EllipticCurve(field, new BigInteger(sa_p256, 16),
new BigInteger(sb_p256, 16));
ECPoint g = new ECPoint(new BigInteger(sx_p256, 16), new BigInteger(sy_p256, 16));
BigInteger order = new BigInteger(sorder_p256, 16);
EllipticCurve curve = new EllipticCurve(
field,
new BigInteger(sa_p256r1, 16),
new BigInteger(sb_p256r1, 16)
);

ECPoint g = new ECPoint(
new BigInteger(sx_p256r1, 16),
new BigInteger(sy_p256r1, 16)
);

BigInteger order = new BigInteger(sorder_p256r1, 16);
int cofactor = 1;

ECParameterSpec ecParamSpec = new ECParameterSpec(curve, g, order, cofactor);

compute_ecdh_key(methodId, ecParamSpec);

}

void compute_ecdh_key(String idString, AlgorithmParameterSpec algParameterSpec)
Expand Down Expand Up @@ -271,14 +278,18 @@ void compute_ecdh_key(String idString, AlgorithmParameterSpec algParameterSpec)
throw e;
}

// Generate the key bytes
byte[] sharedSecretA = keyAgreeA.generateSecret();
byte[] sharedSecretB = keyAgreeB.generateSecret();
// System.out.println(methodName + " sharedSecretA = " + BaseUtils.bytesToHex(sharedSecretA));
// System.out.println(methodName + " sharedSecretB = " + BaseUtils.bytesToHex(sharedSecretB));

assertTrue(Arrays.equals(sharedSecretA, sharedSecretB));

try {
// Generate the key bytes
byte[] sharedSecretA = keyAgreeA.generateSecret();
byte[] sharedSecretB = keyAgreeB.generateSecret();
assertTrue(Arrays.equals(sharedSecretA, sharedSecretB));
} catch (IllegalStateException ise) {
if (getProviderName().equals(("OpenJCEPlusFIPS"))) {
if (idString.equals("secp192r1")) {
assertTrue(ise.getMessage().equals("NIST P-192 curve is not supported in FIPS for calculating the shared secret"));
}
}
Copy link
Member

@jasonkatonica jasonkatonica Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this test have an else to throw the exception ise ? Yes, only the OpenJCEPlusFIPS provider should get the above message and failure. However if OpenJCEPlus gets an exception incorrectly this test would still pass.

This comment applies to all tests modified that catch (IllegalStateException ise) then assert the exception message for OpenJCEPlusFIPS provider.

}
}

/*
Expand Down Expand Up @@ -333,9 +344,9 @@ void compute_ecdh_key_with_global_key(String idString, AlgorithmParameterSpec al
final String methodName = "compute_ecdh_key_with_global_key" + "_" + idString;
KeyPair keyPairA = null, keyPairB = null;
switch (idString) {
case "secp192k1":
keyPairA = keyPairA_192k1;
keyPairB = keyPairB_192k1;
case "secp192r1":
keyPairA = keyPairA_192r1;
keyPairB = keyPairB_192r1;
break;

case "secp256r1":
Expand Down Expand Up @@ -399,11 +410,19 @@ void compute_ecdh_key_with_global_key(String idString, AlgorithmParameterSpec al
throw e;
}

// Generate the key bytes
byte[] sharedSecretA = keyAgreeA.generateSecret();
byte[] sharedSecretB = keyAgreeB.generateSecret();
System.out.println(methodName + " sharedSecretB = " + BaseUtils.bytesToHex(sharedSecretB));
assertTrue(Arrays.equals(sharedSecretA, sharedSecretB));
try {
// Generate the key bytes
byte[] sharedSecretA = keyAgreeA.generateSecret();
byte[] sharedSecretB = keyAgreeB.generateSecret();
System.out.println(methodName + " sharedSecretB = " + BaseUtils.bytesToHex(sharedSecretB));
assertTrue(Arrays.equals(sharedSecretA, sharedSecretB));
} catch (IllegalStateException ise) {
if (getProviderName().equals(("OpenJCEPlusFIPS"))) {
if (idString.equals("secp192r1")) {
assertTrue(ise.getMessage().equals("NIST P-192 curve is not supported in FIPS for calculating the shared secret"));
}
}
}
}

@Test
Expand Down
52 changes: 51 additions & 1 deletion src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright IBM Corp. 2024
* Copyright IBM Corp. 2024, 2025
*
* This code is free software; you can redistribute it and/or modify it
* under the terms provided by IBM in the LICENSE file that accompanied
Expand All @@ -8,9 +8,20 @@
package ibm.jceplus.junit.openjceplusfips;

import ibm.jceplus.junit.base.BaseTestECDHKeyAgreementParamValidation;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.spec.ECGenParameterSpec;
import javax.crypto.KeyAgreement;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

@TestInstance(Lifecycle.PER_CLASS)
public class TestECDHKeyAgreementParamValidation extends BaseTestECDHKeyAgreementParamValidation {
Expand All @@ -21,4 +32,43 @@ public void beforeAll() throws Exception {
setProviderName(Utils.TEST_SUITE_PROVIDER_NAME);
}

@ParameterizedTest
@ValueSource(strings = {"secp192r1", "secp224r1"})
public void testECDHKeyAgreementSharedSecretComputation(String curveName) {
assumeTrue(
Boolean.getBoolean("openjceplus.disableSmallerECKeySizeForSharedKeyComputing"),
"Property not true; skipping"
);

try {
KeyPair alice = genECKeyPair(curveName);
KeyPair bob = genECKeyPair(curveName);

ecdhSharedSecretComputation(alice.getPrivate(), bob.getPublic());
ecdhSharedSecretComputation(bob.getPrivate(), alice.getPublic());

fail("Curve " + curveName + " worked unexpectedly");
} catch (Exception e) {
if (curveName.equals("secp192r1")) {
assertTrue(e.getMessage().equals("NIST P-192 curve is not supported in FIPS for calculating the shared secret"));
} else {
assertTrue(e.getMessage().equals("NIST P-224 curve is not supported in FIPS for calculating the shared secret"));
}
}
}

private KeyPair genECKeyPair(String curveName) throws Exception {
KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC", getProviderName());
kpg.initialize(new ECGenParameterSpec(curveName));
KeyPair kp = kpg.generateKeyPair();
return kp;
}

private byte[] ecdhSharedSecretComputation(PrivateKey priv, PublicKey peerPub) throws Exception {
KeyAgreement ka = KeyAgreement.getInstance("ECDH", getProviderName());
ka.init(priv);
ka.doPhase(peerPub, true);
return ka.generateSecret();
}

}