Support OpenSSL backend and implement message digest

[KostasTsiounis]

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]

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]

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

[KostasTsiounis]

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]

According to the parameters, this should be _JJJI

[KostasTsiounis]

Updated comment and method signature.

[JinhangZhang]

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.

[KostasTsiounis]

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?

[JinhangZhang]

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.

[jasonkatonica]

Maybe ConcurrentHashMap instead of Map ?

[KostasTsiounis]

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

[JinhangZhang]

I still have one concern here. Consider a backend that has not been initialized yet, then there are two threads.

Thread A enters the synchronized block and creates both contexts and runtimeContextNum. Then, thread A executes:

contextsPerBackend.put(this.nativeInterface, contexts);

but has not yet executed:

runtimeContextNumPerBackend.put(this.nativeInterface, runtimeContextNum);

At this point, thread B enters getContext() and performs the initial reads outside the synchronized block:

contexts = contextsPerBackend.get(this.nativeInterface);
runtimeContextNum = runtimeContextNumPerBackend.get(this.nativeInterface);

Thread B can observe:

contexts != null
runtimeContextNum == null

Since contexts != null, Thread B skips the synchronized block entirely, so the new re-read of runtimeContextNumPerBackend inside the lock is never reached.

I think either the two backend states need to be published atomically as one object, or both the get and put need to use the same synchronization.

[KostasTsiounis]

I updated to check both variables and wait for the synchronized block if either of them is not set. That way I think we can guarantee that no thread will move on without having values for both. See d3e59ad.

[JinhangZhang]

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);

[KostasTsiounis]

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

[jasonkatonica]

Maybe ConcurrentHashMap instead of Map ?

[JinhangZhang]

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.

[KostasTsiounis]

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

[KostasTsiounis]

I added some additional guards for z/OS, since OpenSSL is not available there with e6a77cb

[jasonkatonica]

Can we bump this to the latest LTS release that is currently in service for openssl? Also please define this as a static final constant.