Support OpenSSL backend and implement message digest by KostasTsiounis · Pull Request #1492 · IBM/OpenJCEPlus

KostasTsiounis · 2026-05-22T20:18:34Z

With this change, all appropriate code is added to support the use of OpenSSL as a backend. That includes:

Main Java class to load the libraries required and offer native methods.
Adapter Java classes to smoothly integrate without changes with existing code.
Makefiles to compile and link required C code to use OpenSSL.
Additional test tags and general test setup to allow the execution of appropriate tests with the new backend.

Additionally, the first set of algorithms, namely the message digests, are implemented. The aforementioned tags are, also, added to the corresponding tests.

Signed-off-by: Kostas Tsiounis kostas.tsiounis@ibm.com

johnpeck-us-ibm · 2026-06-05T19:10:28Z

I know we talked about contexts and one for FIPS and one for non-FIPS. Could we use the Property query instead?
sha256 = EVP_MD_fetch(NULL, "SHA2-256", "fips=yes"); - FIPS
sha256 = EVP_MD_fetch(NULL, "SHA2-256", "provider=default"); - Non-FIPS.
Note the NULL for the context.

Does using this have any performance ramifications?

thu-ibm · 2026-08-05T19:19:41Z

+#include "Utils.h"
+#include <string.h>
+
+int debug = 0;  // FIXME


All gslogMessage calls always print to stderr regardless of the flag value

This is following the same pattern that all OCK native code has. Do you think we should change this to have different levels of logging? We could open an issue to do this everywhere if we want.

JinhangZhang · 2026-08-13T18:39:57Z

+ * Signature: (JJI)V
+ */
+JNIEXPORT void JNICALL
+Java_com_ibm_crypto_plus_provider_openssl_NativeOpenSSLImplementation_DIGEST_1digest_1and_1reset__JJI(


According to the parameters, this should be _JJJI

Updated comment and method signature.

JinhangZhang · 2026-08-13T19:22:40Z

-    static private ConcurrentLinkedQueueLong contexts[];
-
-    static private int runtimeContextNum[];
+    private static final Map<NativeInterface, Integer[]> runtimeContextNumPerBackend = new HashMap<>();


I have a concern here by using two hashmaps. If I understand correctly, the backend cache state is published through two separate maps. Another thread can observe contextsPerBackend initialized before the corresponding runtimeContextNumPerBackend entry is published, skip initialization, and later dereference a null runtimeContextNum.

I'm not sure I follow the train of thought here. Are you talking about the static class variables? These will be assigned to a new hashmap during the class initialization. When a thread actually attempts to get a context, it will go through the double check and synchronization lock in lines 75-78. The context queue per backend is also a concurrent one.

Did you mean something else that I missed here?

Yes, I’m referring to the static per-backend maps, but the issue is not the initialization of the maps themselves or the ConcurrentLinkedQueue.

Consider two threads entering getContext() for a backend that has not been initialized yet. Both threads can initially read contexts == null and runtimeContextNum == null. Thread A acquires the lock first, initializes both entries, and releases the lock. Thread B then acquires the lock and re-reads only contextsPerBackend. Since contexts is now non-null, it skips the initialization block. However, Thread B never re-reads runtimeContextNumPerBackend, so its local runtimeContextNum variable is still null from the read before acquiring the lock.

Maybe ConcurrentHashMap instead of Map ?

I make sure to set runtimeContextNum in the synchronized block if contexts has already been set.

JinhangZhang · 2026-08-13T19:48:27Z

+        goto cleanup;
+    }
+
+    rc = EVP_DigestFinal_ex(mdCtx, digestBytesNative, (unsigned int *)&digestLen);


Is there any reason why we call EVP_DigestFinal_ex twice? If you want to get the output buffer size, i believe you can do something like

int digestLen = EVP_MD_CTX_get_size(mdCtx); digestBytes = (*env)->NewByteArray(env, digestLen); unsigned int actualLen = 0; EVP_DigestFinal_ex( mdCtx, digestBytesNative, &actualLen);

Changed to suggested API. It looks like EVP_DigestFinal_ex doesn't guarantee proper size.

jasonkatonica · 2026-08-14T11:41:12Z

+Java_com_ibm_crypto_plus_provider_openssl_NativeOpenSSLImplementation_DIGEST_1update(
+    JNIEnv *env, jclass thisObj, jlong osslContextId, jlong digestId,
+    jbyteArray data, jint offset, jint dataLen) {
+    //static const char *functionName = "NativeOSSLImplementation.DIGEST_update";


Comment can be removed.

jasonkatonica · 2026-08-14T12:03:43Z

+    if (digestLen < 0) {
+        throwOSSLException(env, 0, "DIGEST_digest_and_reset_internal: The specified data length is negative");
+        return 0;
+    }


Perhaps we can skip this check since a unsigned int could not be < 0

jasonkatonica · 2026-08-14T12:08:04Z

+void osslCheckStatus(void) {
+    unsigned long errCode;
+
+    while ((errCode = ERR_get_error()) == 1) {


Are we sure all errors return as a value of 1 ? The OpenSSL doc for this API ERR_get_error indicates a value of 0 if there are no errors left to read from the queue.

while ((errCode = ERR_get_error()) != 0)

Changed to suggestion.

jasonkatonica · 2026-08-14T12:14:38Z

+    static char printBuffer[4096];
+
+    va_start(formatArgs, formatString);
+    charsPrinted = vsprintf(printBuffer, formatString, formatArgs);


Could we use vsnprintf instead of vsprintf in all our new openssl code? If formatString were large here or was missing a null terminator I think it would overlay printBuffer. In general all our new code we should try to make as bounded as possible. Something like the following in various places:

vsnprintf(printBuffer, sizeof(printBuffer), formatString, formatArgs);

Switched to vsnprintf.

jasonkatonica · 2026-08-14T13:23:27Z

-    static private ConcurrentLinkedQueueLong contexts[];
-
-    static private int runtimeContextNum[];
+    private static final Map<NativeInterface, Integer[]> runtimeContextNumPerBackend = new HashMap<>();


Maybe ConcurrentHashMap instead of Map ?

jasonkatonica · 2026-08-14T13:33:49Z

+Java_com_ibm_crypto_plus_provider_openssl_NativeOpenSSLImplementation_DIGEST_1digest_1and_1reset__JJI(
+    JNIEnv *env, jclass thisObj, jlong osslContextId, jlong digestId,
+    jlong digestBytes, jint length) {
+    //static const char *functionName = "NativeOSSLImplementation.DIGEST_digest_and_reset";


Any reason to keep these method names through the code? If we are going to use them then we should add debugging pathing, if not i think we could eliminate these. We could always later copy the value from the comment for the method for the Method: name such that the value would not be lost.

Followed the same pattern as OCK initially, but I removed them now.

jasonkatonica · 2026-08-14T13:35:58Z

+    @Override
+    public String getLibraryBuildDate() {
+        if (libraryBuildDate == unobtainedValue) {
+            libraryBuildDate = NativeOpenSSLImplementation.getLibraryBuildDate();;


Two ;; characters.

jasonkatonica · 2026-08-14T13:38:51Z

+    // value has not yet been obtained.  We do this because some values
+    // may be null and we only want to query the value one time.
+    //
+    private static String libraryBuildDate = unobtainedValue;


Does this have to be static ? I could see how there are two threads trying to set libraryBuildDate below might cause some issues?

They would just set it to the same value. I don't think there is a need to keep one per instance.

jasonkatonica · 2026-08-14T13:41:31Z

+    EVP_MD_free(md);
+    md = NULL;
+
+    (*env)->ReleaseStringUTFChars(env, digestAlgo, digestAlgoChars);


We might need a null check on digestAlgoChars here? If an exception is thrown prior to line 41 then digestAlgoChars would be null.

If an exception is thrown that early, we don't go to cleanup. We just return 0.

JinhangZhang · 2026-08-14T15:10:11Z

+    public static NativeOpenSSLAdapterNonFIPS getInstance() {
+        if (instance == null) {
+            instance = new NativeOpenSSLAdapterNonFIPS();
+        }


Is it guaranteed that NativeOpenSSLAdapterNonFIPS.getInstance() returns a single instance under concurrent first access? getInstance() itself is not synchronized and its instance field is not volatile, so two threads may construct different adapter instances.

This seems particularly relevant now that Digest uses the NativeInterface instance itself as the key for contextsPerBackend. Two adapter instances representing the same OpenSSL backend would therefore create separate digest caches.

You are right. Changed to do a double check with synchronization in between.

With this change, all appropriate code is added to support the use of OpenSSL as a backend. That includes: - Main Java class to load the libraries required and offer native methods. - Adapter Java classes to smoothly integrate without changes with existing code. - Makefiles to compile and link required C code to use OpenSSL. - Additional test tags and general test setup to allow the execution of appropriate tests with the new backend. Additionally, the first set of algorithms, namely the message digests, are implemented. The aforementioned tags are, also, added to the corresponding tests. Signed-off-by: Kostas Tsiounis <kostas.tsiounis@ibm.com>

KostasTsiounis force-pushed the openssl_backend branch from ff3a354 to 26a5ca2 Compare May 22, 2026 20:25

KostasTsiounis force-pushed the openssl_backend branch from 04bc9c2 to d4a4c94 Compare June 16, 2026 18:37

KostasTsiounis force-pushed the openssl_backend branch 2 times, most recently from 1b76bd7 to 31add61 Compare July 7, 2026 20:28

KostasTsiounis force-pushed the openssl_backend branch 2 times, most recently from 8920c23 to 0e09aab Compare July 21, 2026 15:39

jasonkatonica requested a review from thu-ibm July 21, 2026 18:14

KostasTsiounis force-pushed the openssl_backend branch from c43bb68 to 117b14c Compare July 31, 2026 00:05

KostasTsiounis marked this pull request as ready for review July 31, 2026 00:05

KostasTsiounis requested review from JinhangZhang, Mohit-Rajbhar100698, jasonkatonica, johnpeck-us-ibm and taoliult July 31, 2026 00:06

johnpeck-us-ibm requested changes Aug 4, 2026

View reviewed changes