From e882f7b0ec85ff981a24f69f6997330b63aeff6e Mon Sep 17 00:00:00 2001 From: Tao Liu Date: Wed, 22 Apr 2026 17:10:11 -0400 Subject: [PATCH 1/3] Validate XDH native secret derivation Add return-code checks for the native XDH secret derivation path, including ICC_EVP_PKEY_derive_init, ICC_EVP_PKEY_derive_set_peer, and the ICC_EVP_PKEY_derive call used to query the secret length. Also validate the derived secret length against the allocated Java byte array length before returning the result. Signed-off-by: Tao Liu --- src/main/native/ock/ECKey.c | 104 +++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 36 deletions(-) diff --git a/src/main/native/ock/ECKey.c b/src/main/native/ock/ECKey.c index b04270801..bc132611b 100644 --- a/src/main/native/ock/ECKey.c +++ b/src/main/native/ock/ECKey.c @@ -2240,6 +2240,7 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD unsigned char *secretBytesNative = NULL; jboolean isCopy = 0; size_t secret_key_len = 0; + size_t allocated_len = 0; int rc = 0; if (debug) { @@ -2250,56 +2251,87 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD NULL); /* Set private key */ if (NULL == gen_ctx) { throwOCKException(env, 0, "NULL from ICC_EVP_PKEY_CTX_new"); + goto cleanup; + } + + rc = ICC_EVP_PKEY_derive_init(ockCtx, gen_ctx); + if (ICC_OSSL_SUCCESS != rc) { + throwOCKException(env, 0, "ICC_EVP_PKEY_derive_init failed"); + goto cleanup; + } + + rc = ICC_EVP_PKEY_derive_set_peer(ockCtx, gen_ctx, ockPubXecKey); /* Set public key */ + if (ICC_OSSL_SUCCESS != rc) { + throwOCKException(env, 0, "ICC_EVP_PKEY_derive_set_peer failed"); + goto cleanup; + } + + if (secretBufferSize > 0) { + secret_key_len = secretBufferSize; } else { - ICC_EVP_PKEY_derive_init(ockCtx, gen_ctx); - ICC_EVP_PKEY_derive_set_peer(ockCtx, gen_ctx, - ockPubXecKey); /* Set public key */ - if (secretBufferSize > 0) { - secret_key_len = secretBufferSize; - } else { - ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, - &secret_key_len); /* Get secret key size */ + rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, &secret_key_len); /* Get secret key size */ + if (ICC_OSSL_SUCCESS != rc) { + throwOCKException(env, 0, + "ICC_EVP_PKEY_derive failed to get secret size"); + goto cleanup; } - secretBytes = (*env)->NewByteArray( - env, secret_key_len); /* Create Java secret bytes array with size */ - if (NULL == secretBytes) { - throwOCKException(env, 0, "NewByteArray failed"); - } else { - secretBytesNative = - (unsigned char *)((*env)->GetPrimitiveArrayCritical( - env, secretBytes, &isCopy)); - if (NULL == secretBytesNative) { - throwOCKException(env, 0, - "NULL from GetPrimitiveArrayCritical"); - } else { - rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, secretBytesNative, - &secret_key_len); - if (ICC_OSSL_SUCCESS != rc) { - throwOCKException( - env, 0, "ICC_EVP_PKEY_derive failed to derive a key"); - } - ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx); - (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, - secretBytesNative, 0); - if (debug) { - gslogFunctionExit(functionName); - } - return secretBytes; - } + if (secret_key_len == 0) { + throwOCKException(env, 0, "Derived secret size is zero"); + goto cleanup; } } + allocated_len = secret_key_len; + + secretBytes = (*env)->NewByteArray(env, allocated_len); /* Create Java secret bytes array with size */ + if (NULL == secretBytes) { + throwOCKException(env, 0, "NewByteArray failed"); + goto cleanup; + } + + secretBytesNative = (unsigned char *)(*env)->GetPrimitiveArrayCritical( + env, secretBytes, &isCopy); + if (NULL == secretBytesNative) { + throwOCKException(env, 0, "NULL from GetPrimitiveArrayCritical"); + goto cleanup; + } + + rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, secretBytesNative, &secret_key_len); + if (ICC_OSSL_SUCCESS != rc) { + throwOCKException(env, 0, "ICC_EVP_PKEY_derive failed to derive a key"); + goto cleanup; + } + + if (secret_key_len > allocated_len) { + throwOCKException(env, 0, + "Derived secret length exceeds allocated output buffer"); + goto cleanup; + } + + ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx); + (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, + secretBytesNative, 0); + + if (debug) { + gslogFunctionExit(functionName); + } + + return secretBytes; + +cleanup: if (NULL != gen_ctx) { ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx); + gen_ctx = NULL; } if (NULL != secretBytesNative) { - (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, - secretBytesNative, 0); + (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0); + secretBytesNative = NULL; } if (NULL != secretBytes) { (*env)->DeleteLocalRef(env, secretBytes); + secretBytes = NULL; } if (debug) { From c05c357500078b4dbea797b52df3885e6ecd01f9 Mon Sep 17 00:00:00 2001 From: Tao Liu Date: Wed, 29 Apr 2026 11:28:16 -0400 Subject: [PATCH 2/3] Validate XDH derived secret size before allocation --- src/main/native/ock/ECKey.c | 49 +++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src/main/native/ock/ECKey.c b/src/main/native/ock/ECKey.c index bc132611b..6ba1407af 100644 --- a/src/main/native/ock/ECKey.c +++ b/src/main/native/ock/ECKey.c @@ -2239,8 +2239,8 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD jbyteArray secretBytes = NULL; unsigned char *secretBytesNative = NULL; jboolean isCopy = 0; + size_t required_len = 0; size_t secret_key_len = 0; - size_t allocated_len = 0; int rc = 0; if (debug) { @@ -2266,24 +2266,29 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD goto cleanup; } - if (secretBufferSize > 0) { - secret_key_len = secretBufferSize; - } else { - rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, &secret_key_len); /* Get secret key size */ - if (ICC_OSSL_SUCCESS != rc) { - throwOCKException(env, 0, - "ICC_EVP_PKEY_derive failed to get secret size"); - goto cleanup; - } - if (secret_key_len == 0) { - throwOCKException(env, 0, "Derived secret size is zero"); - goto cleanup; - } + /* + * Always query the required secret size before deriving the secret. + * This avoids deriving into a caller-provided buffer that may be too small. + */ + rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, &required_len); /* Get secret key size */ + if (ICC_OSSL_SUCCESS != rc) { + throwOCKException(env, 0, + "ICC_EVP_PKEY_derive failed to get secret size"); + goto cleanup; + } + if (required_len == 0) { + throwOCKException(env, 0, "Derived secret size is zero"); + goto cleanup; } - allocated_len = secret_key_len; + if ((secretBufferSize > 0) && ((size_t)secretBufferSize < required_len)) { + throwOCKException(env, 0, "Provided secret buffer size is smaller than required size"); + goto cleanup; + } - secretBytes = (*env)->NewByteArray(env, allocated_len); /* Create Java secret bytes array with size */ + secret_key_len = (secretBufferSize > 0) ? (size_t)secretBufferSize : required_len; + + secretBytes = (*env)->NewByteArray(env, secret_key_len); /* Create Java secret bytes array with size */ if (NULL == secretBytes) { throwOCKException(env, 0, "NewByteArray failed"); goto cleanup; @@ -2302,15 +2307,11 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD goto cleanup; } - if (secret_key_len > allocated_len) { - throwOCKException(env, 0, - "Derived secret length exceeds allocated output buffer"); - goto cleanup; - } - ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx); - (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, - secretBytesNative, 0); + gen_ctx = NULL; + + (*env)->ReleasePrimitiveArrayCritical(env, secretBytes, secretBytesNative, 0); + secretBytesNative = NULL; if (debug) { gslogFunctionExit(functionName); From 356d07e514a2ccde886e0cc25026098681b428a0 Mon Sep 17 00:00:00 2001 From: Tao Liu Date: Wed, 6 May 2026 13:22:41 -0400 Subject: [PATCH 3/3] Remove secretBufferSize --- .../crypto/plus/provider/XDHKeyAgreement.java | 11 +---------- .../plus/provider/base/NativeInterface.java | 2 +- .../ibm/crypto/plus/provider/base/XECKey.java | 4 ++-- .../plus/provider/ock/NativeOCKAdapter.java | 4 ++-- .../provider/ock/NativeOCKImplementation.java | 2 +- src/main/native/ock/ECKey.c | 19 ++++++++++--------- 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/ibm/crypto/plus/provider/XDHKeyAgreement.java b/src/main/java/com/ibm/crypto/plus/provider/XDHKeyAgreement.java index 6d94b1676..a140df54f 100644 --- a/src/main/java/com/ibm/crypto/plus/provider/XDHKeyAgreement.java +++ b/src/main/java/com/ibm/crypto/plus/provider/XDHKeyAgreement.java @@ -102,17 +102,8 @@ protected Key engineDoPhase(Key key, boolean lastPhase) } try { - int secrectBufferSize = 0; - String curveName = ((NamedParameterSpec) xdhPublicKeyImpl.getParams()).getName(); - if (NamedParameterSpec.X25519.getName().equalsIgnoreCase(curveName)) { - secrectBufferSize = SECRET_BUFFER_SIZE_X25519; // X25519 secret buffer size - } else if (NamedParameterSpec.X448.getName().equalsIgnoreCase(curveName)) { - secrectBufferSize = SECRET_BUFFER_SIZE_X448; // X448 secret buffer size - } else { - secrectBufferSize = 0; // Let OCK decide the size - } this.secret = XECKey.computeECDHSecret(genCtx, - ockXecKeyPub.getPKeyId(), ockXecKeyPriv.getPKeyId(), secrectBufferSize, provider); + ockXecKeyPub.getPKeyId(), ockXecKeyPriv.getPKeyId(), provider); } catch (NativeException e) { //Validate the secret value for a small order point condition. byte orValue = (byte) 0; diff --git a/src/main/java/com/ibm/crypto/plus/provider/base/NativeInterface.java b/src/main/java/com/ibm/crypto/plus/provider/base/NativeInterface.java index e779a6f54..f1e984b63 100644 --- a/src/main/java/com/ibm/crypto/plus/provider/base/NativeInterface.java +++ b/src/main/java/com/ibm/crypto/plus/provider/base/NativeInterface.java @@ -548,7 +548,7 @@ public byte[] ECKEY_computeECDHSecret(long pubEcKeyId, long privEcKeyId) throws NativeException; public byte[] XECKEY_computeECDHSecret(long genCtx, - long pubEcKeyId, long privEcKeyId, int secrectBufferSize) throws NativeException; + long pubEcKeyId, long privEcKeyId) throws NativeException; public byte[] ECKEY_signDatawithECDSA(byte[] digestBytes, diff --git a/src/main/java/com/ibm/crypto/plus/provider/base/XECKey.java b/src/main/java/com/ibm/crypto/plus/provider/base/XECKey.java index 59910be84..8dcb5bfd6 100644 --- a/src/main/java/com/ibm/crypto/plus/provider/base/XECKey.java +++ b/src/main/java/com/ibm/crypto/plus/provider/base/XECKey.java @@ -68,7 +68,7 @@ public static XECKey generateKeyPair(int curveNum, int pub_size, OpenJCEPlusProv } public static byte[] computeECDHSecret(long genCtx, long pubId, - long privId, int secrectBufferSize, OpenJCEPlusProvider provider) throws NativeException { + long privId, OpenJCEPlusProvider provider) throws NativeException { if (pubId == 0) throw new IllegalArgumentException("The public key parameter is not valid"); if (privId == 0) @@ -76,7 +76,7 @@ public static byte[] computeECDHSecret(long genCtx, long pubId, NativeInterface nativeInterface = provider.isFIPS() ? NativeOCKAdapterFIPS.getInstance() : NativeOCKAdapterNonFIPS.getInstance(); byte[] sharedSecretBytes = nativeInterface.XECKEY_computeECDHSecret( - genCtx, pubId, privId, secrectBufferSize); + genCtx, pubId, privId); //OCKDebug.Msg (debPrefix, methodName, "pubId :" + pubId + " privId :" + privId + " sharedSecretBytes :", sharedSecretBytes); return sharedSecretBytes; } diff --git a/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKAdapter.java b/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKAdapter.java index 777fcc92c..350221e19 100644 --- a/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKAdapter.java +++ b/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKAdapter.java @@ -1112,9 +1112,9 @@ public byte[] ECKEY_computeECDHSecret(long pubEcKeyId, long privEcKeyId) throws } @Override - public byte[] XECKEY_computeECDHSecret(long genCtx, long pubEcKeyId, long privEcKeyId, int secrectBufferSize) + public byte[] XECKEY_computeECDHSecret(long genCtx, long pubEcKeyId, long privEcKeyId) throws OCKException { - return NativeOCKImplementation.XECKEY_computeECDHSecret(ockContext.getId(), genCtx, pubEcKeyId, privEcKeyId, secrectBufferSize); + return NativeOCKImplementation.XECKEY_computeECDHSecret(ockContext.getId(), genCtx, pubEcKeyId, privEcKeyId); } @Override diff --git a/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKImplementation.java b/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKImplementation.java index 11872ea6a..8517153f2 100644 --- a/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKImplementation.java +++ b/src/main/java/com/ibm/crypto/plus/provider/ock/NativeOCKImplementation.java @@ -722,7 +722,7 @@ static public native byte[] ECKEY_computeECDHSecret(long ockContextId, long pubE long privEcKeyId) throws OCKException; static public native byte[] XECKEY_computeECDHSecret(long ockContextId, long genCtx, - long pubEcKeyId, long privEcKeyId, int secrectBufferSize) throws OCKException; + long pubEcKeyId, long privEcKeyId) throws OCKException; static public native byte[] ECKEY_signDatawithECDSA(long ockContextId, byte[] digestBytes, diff --git a/src/main/native/ock/ECKey.c b/src/main/native/ock/ECKey.c index 6ba1407af..b4d6f5fa3 100644 --- a/src/main/native/ock/ECKey.c +++ b/src/main/native/ock/ECKey.c @@ -2227,7 +2227,7 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_ECKEY_1computeECDH JNIEXPORT jbyteArray JNICALL Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECDHSecret( JNIEnv *env, jclass thisObj, jlong ockContextId, jlong genCtx, - jlong pubXecKeyId, jlong privXecKeyId, jint secretBufferSize) { + jlong pubXecKeyId, jlong privXecKeyId) { static const char *functionName = "NativeInterface_XECKEY_1computeECDHSecret"; @@ -2268,7 +2268,7 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD /* * Always query the required secret size before deriving the secret. - * This avoids deriving into a caller-provided buffer that may be too small. + * This avoids deriving into a buffer with an incorrect size. */ rc = ICC_EVP_PKEY_derive(ockCtx, gen_ctx, NULL, &required_len); /* Get secret key size */ if (ICC_OSSL_SUCCESS != rc) { @@ -2281,14 +2281,9 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD goto cleanup; } - if ((secretBufferSize > 0) && ((size_t)secretBufferSize < required_len)) { - throwOCKException(env, 0, "Provided secret buffer size is smaller than required size"); - goto cleanup; - } + secret_key_len = required_len; - secret_key_len = (secretBufferSize > 0) ? (size_t)secretBufferSize : required_len; - - secretBytes = (*env)->NewByteArray(env, secret_key_len); /* Create Java secret bytes array with size */ + secretBytes = (*env)->NewByteArray(env, secret_key_len); /* Create Java secret bytes array */ if (NULL == secretBytes) { throwOCKException(env, 0, "NewByteArray failed"); goto cleanup; @@ -2307,6 +2302,12 @@ Java_com_ibm_crypto_plus_provider_ock_NativeOCKImplementation_XECKEY_1computeECD goto cleanup; } + if (secret_key_len != required_len) { + throwOCKException(env, 0, + "Derived secret size does not match required size"); + goto cleanup; + } + ICC_EVP_PKEY_CTX_free(ockCtx, gen_ctx); gen_ctx = NULL;