Support OpenSSL backend and implement message digest - #1492
Support OpenSSL backend and implement message digest#1492KostasTsiounis wants to merge 3 commits into
Conversation
ff3a354 to
26a5ca2
Compare
|
I know we talked about contexts and one for FIPS and one for non-FIPS. Could we use the Property query instead? Does using this have any performance ramifications? |
04bc9c2 to
d4a4c94
Compare
1b76bd7 to
31add61
Compare
8920c23 to
0e09aab
Compare
c43bb68 to
117b14c
Compare
| @SuppressWarnings("restricted") | ||
| protected static boolean loadIfExists(File libraryFile) { | ||
| String libraryName = libraryFile.getAbsolutePath(); | ||
| System.out.println("Library name: " + libraryName); |
There was a problem hiding this comment.
Need to remove this System.out.println and the others or convert them to debugs.
There was a problem hiding this comment.
Left from debug. Removed.
| } | ||
|
|
||
| public static NativeOCKAdapterNonFIPS getInstance() { | ||
| System.out.println("Using OCK non-FIPS adapter."); |
There was a problem hiding this comment.
Left from debug. Removed.
|
|
||
| @Override | ||
| public void validateLibraryLocation() throws ProviderException, OpenSSLException { | ||
| /*if (NativeOpenSSLImplementation.requirePreloadOSSL == false) { |
There was a problem hiding this comment.
Do we not need this to maybe help us determine where the OpenSSL DLL is coming from?
There was a problem hiding this comment.
We could add a JNI method to do this, but the question is whether we need this given that it's not like OCK that is provided through a binary. In this case, the user provides their own OpenSSL library.
|
|
||
| if (!(digestAlgoChars = (const char *)(*env)->GetStringUTFChars(env, digestAlgo, NULL))) { | ||
| throwOSSLException(env, 0, "DIGEST_create: GetStringUTFChars() failed"); | ||
| goto cleanup; |
There was a problem hiding this comment.
This should be a return here instead of goto.
| } | ||
|
|
||
| // Everything succeeded. Set digestId to created EVP_MD_CTX. | ||
| digestId = (jlong)((intptr_t)mdCtx); |
There was a problem hiding this comment.
Should this be (intptr_t) (intptr_t*) instead?
There was a problem hiding this comment.
The type intptr_t is supposed to hold pointers itself, so we don't need a intptr_t *.
|
|
||
| try { | ||
| //long osslContextId = NativeOpenSSLImplementation.initializeOSSL(this.useFIPSMode); | ||
| long osslContextId = 0; |
There was a problem hiding this comment.
It appears in this case OpenSSL context is never initialized since the real call is commented out
There was a problem hiding this comment.
That is because the OpenSSL context is not yet needed in the use case I'm introducing, given that we decided not to do FIPS at the moment. So, I only left this here for future use and is not needed now.
| } | ||
|
|
||
| public static NativeOpenSSLAdapterNonFIPS getInstance() { | ||
| System.out.println("Using OpenSSL non-FIPS adapter."); |
There was a problem hiding this comment.
Using System.out.println pollutes stdout in production
There was a problem hiding this comment.
Sorry, those were leftovers from debugging. Removing them.
| * and must match those defined in native/openssl/ExceptionCodes.h. | ||
| */ | ||
| public static final int GKR_FIPS_MODE_INVALID = 0x00000001; | ||
| public static final int GKR_OCK_ATTACH_FAILED = 0x00000002; |
There was a problem hiding this comment.
Error constants like GKR_OCK_ATTACH_FAILED are OCK-specific
There was a problem hiding this comment.
Removed the 2 that are irrelevant and kept the two that are used in common parts. Code here can be reworked in a future PR to not use these constants at all.
| import java.util.Hashtable; | ||
| import java.util.Map; | ||
|
|
||
| public class OpenSSLException extends NativeException { |
There was a problem hiding this comment.
NativeException appears to be an OCK-specific base class
There was a problem hiding this comment.
I'm not sure what you mean here. NativeException is supposed to be a superclass for all exceptions coming from native code. So, native code that uses OCK throws an OCKException, whereas the new digest code that uses OpenSSL throws an OpenSSLException.
There was a problem hiding this comment.
In NativeException.java, the naming in the following gives me the impression they are used for OCK.
public static final int GKR_FIPS_MODE_INVALID = -1;
public static final int GKR_OCK_ATTACH_FAILED = -1;
public static final int GKR_DECRYPT_FINAL_BAD_PADDING_ERROR = -1;
public static final int GKR_UNSPECIFIED = -1;
There was a problem hiding this comment.
Yeah, I can see why. But I only kept them for the reasons mentioned in #1492 (comment). We'll remove them in the future.
| #include "Utils.h" | ||
| #include <string.h> | ||
|
|
||
| int debug = 0; // FIXME |
There was a problem hiding this comment.
All gslogMessage calls always print to stderr regardless of the flag value
There was a problem hiding this comment.
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.
dea939a to
1bb6a35
Compare
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>
1bb6a35 to
555ccec
Compare
With this change, all appropriate code is added to support the use of
OpenSSLas a backend. That includes:OpenSSL.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