Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions sdk/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,12 @@
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-jdk18on</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>
<scope>test</scope>
</dependency>
<!-- Testing Dependencies -->
<dependency>
Expand Down
296 changes: 96 additions & 200 deletions sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java
Original file line number Diff line number Diff line change
@@ -1,39 +1,31 @@
package io.opentdf.platform.sdk;

import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
import org.bouncycastle.cert.X509CertificateHolder;
import org.bouncycastle.crypto.digests.SHA256Digest;
import org.bouncycastle.crypto.generators.HKDFBytesGenerator;
import org.bouncycastle.crypto.params.HKDFParameters;
import org.bouncycastle.jcajce.provider.asymmetric.ec.KeyPairGeneratorSpi;
import org.bouncycastle.jce.ECNamedCurveTable;
import org.bouncycastle.jce.interfaces.ECPrivateKey;
import org.bouncycastle.jce.interfaces.ECPublicKey;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec;
import org.bouncycastle.math.ec.ECPoint;
import org.bouncycastle.openssl.PEMException;
import org.bouncycastle.openssl.PEMParser;
import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
import org.bouncycastle.util.io.pem.*;
import org.bouncycastle.util.io.pem.PemReader;
import org.bouncycastle.jce.spec.ECPublicKeySpec;

import javax.crypto.KeyAgreement;
import java.io.*;
import java.security.*;
import java.security.spec.*;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import java.math.BigInteger;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.KeyFactory;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.Signature;
import java.security.SignatureException;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.ECParameterSpec;
import java.security.spec.ECPoint;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.X509EncodedKeySpec;
import java.util.Base64;
import java.util.Objects;
// https://www.bouncycastle.org/latest_releases.html

public class ECKeyPair {

private static final int SHA256_BYTES = 32;

static {
Security.addProvider(new BouncyCastleProvider());
}
private static final String EC_ALGORITHM = "EC";

private final ECCurve curve;

Expand All @@ -42,37 +34,28 @@ public enum ECAlgorithm {
ECDSA
}

private static final BouncyCastleProvider BOUNCY_CASTLE_PROVIDER = new BouncyCastleProvider();

private KeyPair keyPair;

public ECKeyPair() {
this(ECCurve.SECP256R1, ECAlgorithm.ECDH);
this(ECCurve.SECP256R1);
}

public ECKeyPair(ECCurve curve, ECAlgorithm algorithm) {
public ECKeyPair(ECCurve curve) {
this.curve = Objects.requireNonNull(curve);
KeyPairGenerator generator;

try {
// Should this just use the algorithm vs use ECDH only for ECDH and ECDSA for
// everything else.
if (algorithm == ECAlgorithm.ECDH) {
generator = KeyPairGeneratorSpi.getInstance(ECAlgorithm.ECDH.name(), BOUNCY_CASTLE_PROVIDER);
} else {
generator = KeyPairGeneratorSpi.getInstance(ECAlgorithm.ECDSA.name(), BOUNCY_CASTLE_PROVIDER);
}
} catch (NoSuchAlgorithmException e) {
KeyPairGenerator generator = KeyPairGenerator.getInstance(EC_ALGORITHM);
generator.initialize(new ECGenParameterSpec(this.curve.getCurveName()));
this.keyPair = generator.generateKeyPair();
} catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException e) {
throw new RuntimeException(e);
}
}

ECGenParameterSpec spec = new ECGenParameterSpec(this.curve.getCurveName());
try {
generator.initialize(spec);
} catch (InvalidAlgorithmParameterException e) {
throw new RuntimeException(e);
}
this.keyPair = generator.generateKeyPair();
static ECPublicKey publicKeyFromPem(String pem) throws InvalidKeySpecException, NoSuchAlgorithmException {
String pemData = pem.replaceAll("-----(BEGIN|END) [A-Z ]+-----", "").replaceAll("\\s", "");
byte[] der = Base64.getDecoder().decode(pemData);
KeyFactory keyFactory = KeyFactory.getInstance("EC");
return (ECPublicKey) keyFactory.generatePublic(new X509EncodedKeySpec(der));
Comment on lines +54 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, find and read the ECKeyPair.java file around the mentioned lines
fd -t f "ECKeyPair.java" sdk/src/main

Repository: opentdf/java-sdk

Length of output: 117


🏁 Script executed:

# Get the full content of ECKeyPair.java to understand the context
wc -l sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java

Repository: opentdf/java-sdk

Length of output: 121


🏁 Script executed:

# Read the file with context around lines 54-58
cat -n sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java | head -80 | tail -50

Repository: opentdf/java-sdk

Length of output: 1988


🏁 Script executed:

# Search for all calls to publicKeyFromPem to see how exceptions are handled
rg "publicKeyFromPem" -A 5 -B 2

Repository: opentdf/java-sdk

Length of output: 8132


Wrap Base64.getDecoder().decode() to align exception contract with callers.

Base64.getDecoder().decode(pemData) throws IllegalArgumentException when the Base64 input is malformed. Since this method declares only throws InvalidKeySpecException, NoSuchAlgorithmException, the unchecked exception escapes to call sites in TDF.java and KASClient.java that only catch the declared exceptions. This breaks the exception contract and allows unexpected runtime exceptions to leak instead of being wrapped in SDKException.

Catch IllegalArgumentException and convert it to InvalidKeySpecException to match the method's declared exception contract.

Suggested fix
     static ECPublicKey publicKeyFromPem(String pem) throws InvalidKeySpecException, NoSuchAlgorithmException {
-        String pemData = pem.replaceAll("-----(BEGIN|END) [A-Z ]+-----", "").replaceAll("\\s", "");
-        byte[] der = Base64.getDecoder().decode(pemData);
-        KeyFactory keyFactory = KeyFactory.getInstance("EC");
-        return (ECPublicKey) keyFactory.generatePublic(new X509EncodedKeySpec(der));
+        try {
+            String pemData = pem.replaceAll("-----(BEGIN|END) [A-Z ]+-----", "").replaceAll("\\s", "");
+            byte[] der = Base64.getDecoder().decode(pemData);
+            KeyFactory keyFactory = KeyFactory.getInstance("EC");
+            return (ECPublicKey) keyFactory.generatePublic(new X509EncodedKeySpec(der));
+        } catch (IllegalArgumentException e) {
+            throw new InvalidKeySpecException("invalid EC public key PEM", e);
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java` around lines 54 -
58, The method publicKeyFromPem currently lets IllegalArgumentException from
Base64.getDecoder().decode(pemData) escape, violating its declared throws; wrap
the Base64 decoding in a try/catch that catches IllegalArgumentException and
rethrows a new InvalidKeySpecException (including the original exception as the
cause and a clear message), then continue with KeyFactory.generatePublic as
before so callers that catch InvalidKeySpecException get the wrapped error.

}

public ECPublicKey getPublicKey() {
Expand All @@ -88,197 +71,110 @@ ECCurve getCurve() {
}

public String publicKeyInPEMFormat() {
StringWriter writer = new StringWriter();
PemWriter pemWriter = new PemWriter(writer);

try {
pemWriter.writeObject(new PemObject("PUBLIC KEY", this.keyPair.getPublic().getEncoded()));
pemWriter.flush();
pemWriter.close();
} catch (IOException e) {
throw new RuntimeException(e);
}

return writer.toString();
return toPem("PUBLIC KEY", this.keyPair.getPublic().getEncoded());
}

public String privateKeyInPEMFormat() {
StringWriter writer = new StringWriter();
PemWriter pemWriter = new PemWriter(writer);

try {
pemWriter.writeObject(new PemObject("PRIVATE KEY", this.keyPair.getPrivate().getEncoded()));
pemWriter.flush();
pemWriter.close();
} catch (IOException e) {
throw new RuntimeException(e);
}

return writer.toString();
return toPem("PRIVATE KEY", this.keyPair.getPrivate().getEncoded());
}

public int keySize() {
return this.keyPair.getPrivate().getEncoded().length * 8;
}
Comment on lines 81 to 83
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The keySize() method incorrectly calculates the key size by using the length of the DER-encoded private key (PKCS#8), which includes significant overhead from headers and metadata. For Elliptic Curve keys, the key size should be the field size of the curve in bits (e.g., 256 for P-256).

Suggested change
public int keySize() {
return this.keyPair.getPrivate().getEncoded().length * 8;
}
public int keySize() {
return getPublicKey().getParams().getCurve().getField().getFieldSize();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this seems legit but I'm not sure how many breaking-ish changes we want in this pull


public byte[] compressECPublickey() {
return ((ECPublicKey) this.keyPair.getPublic()).getQ().getEncoded(true);
}

public static String getPEMPublicKeyFromX509Cert(String pemInX509Format) {
try {
PEMParser parser = new PEMParser(new StringReader(pemInX509Format));
X509CertificateHolder x509CertificateHolder = (X509CertificateHolder) parser.readObject();
parser.close();
SubjectPublicKeyInfo publicKeyInfo = x509CertificateHolder.getSubjectPublicKeyInfo();
JcaPEMKeyConverter converter = new JcaPEMKeyConverter().setProvider(BOUNCY_CASTLE_PROVIDER);
ECPublicKey publicKey = null;
try {
publicKey = (ECPublicKey) converter.getPublicKey(publicKeyInfo);
} catch (PEMException e) {
throw new RuntimeException(e);
}

// EC public key to pem formated.
StringWriter writer = new StringWriter();
PemWriter pemWriter = new PemWriter(writer);

pemWriter.writeObject(new PemObject("PUBLIC KEY", publicKey.getEncoded()));
pemWriter.flush();
pemWriter.close();
return writer.toString();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

public static byte[] compressECPublickey(String pemECPubKey) {
try {
KeyFactory ecKeyFac = KeyFactory.getInstance("EC", "BC");
PemReader pemReader = new PemReader(new StringReader(pemECPubKey));
PemObject pemObject = pemReader.readPemObject();
PublicKey pubKey = ecKeyFac.generatePublic(new X509EncodedKeySpec(pemObject.getContent()));
return ((ECPublicKey) pubKey).getQ().getEncoded(true);
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
} catch (IOException e) {
throw new RuntimeException(e);
} catch (InvalidKeySpecException e) {
throw new RuntimeException(e);
} catch (NoSuchProviderException e) {
throw new RuntimeException(e);
}
}

public static String publicKeyFromECPoint(byte[] ecPoint, String curveName) {
try {
// Create EC Public key
ECNamedCurveParameterSpec ecSpec = ECNamedCurveTable.getParameterSpec(curveName);
ECPoint point = ecSpec.getCurve().decodePoint(ecPoint);
ECPublicKeySpec publicKeySpec = new ECPublicKeySpec(point, ecSpec);
KeyFactory keyFactory = KeyFactory.getInstance("ECDSA", "BC");
PublicKey publicKey = keyFactory.generatePublic(publicKeySpec);

// EC Public keu to pem format.
StringWriter writer = new StringWriter();
PemWriter pemWriter = new PemWriter(writer);
pemWriter.writeObject(new PemObject("PUBLIC KEY", publicKey.getEncoded()));
pemWriter.flush();
pemWriter.close();
return writer.toString();
} catch (InvalidKeySpecException e) {
throw new RuntimeException(e);
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
} catch (NoSuchProviderException e) {
throw new RuntimeException(e);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

public static ECPublicKey publicKeyFromPem(String pemEncoding) {
try {
PEMParser parser = new PEMParser(new StringReader(pemEncoding));
SubjectPublicKeyInfo publicKeyInfo = (SubjectPublicKeyInfo) parser.readObject();
parser.close();

JcaPEMKeyConverter converter = new JcaPEMKeyConverter().setProvider(BOUNCY_CASTLE_PROVIDER);
return (ECPublicKey) converter.getPublicKey(publicKeyInfo);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

public static ECPrivateKey privateKeyFromPem(String pemEncoding) {
try {
PEMParser parser = new PEMParser(new StringReader(pemEncoding));
PrivateKeyInfo privateKeyInfo = (PrivateKeyInfo) parser.readObject();
parser.close();

JcaPEMKeyConverter converter = new JcaPEMKeyConverter().setProvider(BOUNCY_CASTLE_PROVIDER);
return (ECPrivateKey) converter.getPrivateKey(privateKeyInfo);
} catch (IOException e) {
throw new RuntimeException(e);
}
return encodeCompressedPoint((ECPublicKey) this.keyPair.getPublic());
}

public static byte[] computeECDHKey(ECPublicKey publicKey, ECPrivateKey privateKey) {
try {
KeyAgreement aKeyAgree = KeyAgreement.getInstance("ECDH", "BC");
KeyAgreement aKeyAgree = KeyAgreement.getInstance("ECDH");
aKeyAgree.init(privateKey);
aKeyAgree.doPhase(publicKey, true);
return aKeyAgree.generateSecret();
} catch (NoSuchAlgorithmException | NoSuchProviderException | InvalidKeyException e) {
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
throw new RuntimeException(e);
}
}

private static final String HMAC_SHA_256 = "HmacSHA256";
/**
* Returns a HKDF key derived from the provided salt and secret
* that is 32 bytes (256 bits) long.
*/
public static byte[] calculateHKDF(byte[] salt, byte[] secret) {
byte[] key = new byte[SHA256_BYTES];
HKDFParameters params = new HKDFParameters(secret, salt, null);

HKDFBytesGenerator hkdf = new HKDFBytesGenerator(SHA256Digest.newInstance());
hkdf.init(params);
hkdf.generateBytes(key, 0, key.length);
return key;
try {
// RFC 5869: if salt is absent, substitute a zero-filled buffer of Hash output size.
byte[] effectiveSalt = (salt == null || salt.length == 0) ? new byte[SHA256_BYTES] : salt;
Mac hmac = Mac.getInstance(HMAC_SHA_256);
hmac.init(new SecretKeySpec(effectiveSalt, HMAC_SHA_256));
byte[] prk = hmac.doFinal(secret);

// HKDF-Expand with empty info and L = 32 (a single HMAC block).
hmac.init(new SecretKeySpec(prk, HMAC_SHA_256));
hmac.update((byte) 0x01);
return hmac.doFinal();
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
throw new RuntimeException(e);
}
}

public static byte[] computeECDSASig(byte[] digest, ECPrivateKey privateKey) {
public static byte[] computeECDSASig(byte[] data, ECPrivateKey privateKey) {
try {
Signature ecdsaSign = Signature.getInstance("SHA256withECDSA", "BC");
Signature ecdsaSign = Signature.getInstance("SHA256withECDSA");
ecdsaSign.initSign(privateKey);
ecdsaSign.update(digest);
ecdsaSign.update(data);
return ecdsaSign.sign();
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
} catch (NoSuchProviderException e) {
throw new RuntimeException(e);
} catch (InvalidKeyException e) {
throw new RuntimeException(e);
} catch (SignatureException e) {
} catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) {
throw new RuntimeException(e);
}
}

public static Boolean verifyECDSAig(byte[] digest, byte[] signature, ECPublicKey publicKey) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

There is a typo in the method name verifyECDSAig (missing 's'). It should be verifyECDSASig. Additionally, the parameter name digest should be renamed to data for consistency with the fact that SHA256withECDSA performs hashing on the input.

Suggested change
public static Boolean verifyECDSAig(byte[] digest, byte[] signature, ECPublicKey publicKey) {
public static Boolean verifyECDSASig(byte[] data, byte[] signature, ECPublicKey publicKey) {

try {
Signature ecdsaVerify = Signature.getInstance("SHA256withECDSA", "BC");
Signature ecdsaVerify = Signature.getInstance("SHA256withECDSA");
ecdsaVerify.initVerify(publicKey);
ecdsaVerify.update(digest);
return ecdsaVerify.verify(signature);
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
} catch (NoSuchProviderException e) {
throw new RuntimeException(e);
} catch (InvalidKeyException e) {
throw new RuntimeException(e);
} catch (SignatureException e) {
} catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) {
Comment on lines +122 to +139
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect whether callers pass raw payloads or precomputed digests into the ECKeyPair ECDSA helpers.
rg -n --type=java -C3 '\b(computeECDSASig|verifyECDSAig)\s*\('
rg -n --type=java -C3 'MessageDigest|getInstance\("SHA-256"|digest\s*\('

Repository: opentdf/java-sdk

Length of output: 8366


🏁 Script executed:

# Search for all usages across the codebase
rg -n --type=java '\b(computeECDSASig|verifyECDSAig)\b' -A2 -B2

# Check for Javadoc or comments near the method definitions
grep -n -B5 "public static.*computeECDSASig\|public static.*verifyECDSAig" sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java

# Search for any references in Manifest or TDF that might interact
rg -n --type=java 'ECKeyPair\.' sdk/src/main/java/io/opentdf/platform/sdk/

Repository: opentdf/java-sdk

Length of output: 3138


🏁 Script executed:

sed -n '115,145p' sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java

Repository: opentdf/java-sdk

Length of output: 1298


Fix parameter naming and method name typo before finalizing public SDK API contract.

verifyECDSAig() has a misleading parameter name (digest) but receives raw messages—both methods use SHA256withECDSA, which hashes internally, so the parameter should be named message to match the actual behavior. Additionally, the method name itself has a typo: verifyECDSAig should be verifyECDSASig. Since these are new SDK methods with no external callers yet (only test usage with raw message bytes), correct the naming and add Javadoc clarifying the message-based contract now to prevent API stability issues later.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk/src/main/java/io/opentdf/platform/sdk/ECKeyPair.java` around lines 122 -
139, Rename the method verifyECDSAig to verifyECDSASig and rename its parameter
from digest to message (since both computeECDSASig and verifyECDSASig call
Signature.getInstance("SHA256withECDSA") which hashes internally), update any
internal references/tests to use the new name/parameter, and add Javadoc to both
computeECDSASig(byte[] data, ECPrivateKey privateKey) and verifyECDSASig(byte[]
message, ECPublicKey publicKey) clarifying they accept the raw message bytes
(not a precomputed digest) and that the SHA-256 hashing is performed internally
by the signature algorithm.

throw new RuntimeException(e);
}
}
}

private static String toPem(String type, byte[] der) {
String b64 = Base64.getEncoder().encodeToString(der);
StringBuilder sb = new StringBuilder();
sb.append("-----BEGIN ").append(type).append("-----\n");
for (int i = 0; i < b64.length(); i += 64) {
sb.append(b64, i, Math.min(i + 64, b64.length())).append('\n');
}
sb.append("-----END ").append(type).append("-----\n");
return sb.toString();
}

private static byte[] encodeCompressedPoint(ECPublicKey publicKey) {
ECPoint w = publicKey.getW();
ECParameterSpec params = publicKey.getParams();
int size = (params.getCurve().getField().getFieldSize() + 7) / 8;
byte[] x = toFixedLength(w.getAffineX(), size);
byte[] result = new byte[size + 1];
result[0] = (byte) (w.getAffineY().testBit(0) ? 0x03 : 0x02);
System.arraycopy(x, 0, result, 1, size);
return result;
}

private static byte[] toFixedLength(BigInteger value, int length) {
byte[] bytes = value.toByteArray();
if (bytes.length == length) {
return bytes;
}
byte[] result = new byte[length];
if (bytes.length > length) {
// BigInteger.toByteArray() may prepend a zero sign byte; strip it.
System.arraycopy(bytes, bytes.length - length, result, 0, length);
} else {
System.arraycopy(bytes, 0, result, length - bytes.length, bytes.length);
}
return result;
}
}
Loading
Loading