Support for PBES1 using native acceleration #997
Conversation
dev-koan
force-pushed
the
implement_pbe1
branch
from
614b0ef to
d2d4e50
Compare
dev-koan
requested review from
JinhangZhang,
KostasTsiounis,
johnpeck-us-ibm and
taoliult
| Cipher | PBEWithSHA1AndRC2_128 | |X | | | ||
| Cipher | PBEWithSHA1AndRC4_40 | |X | | | ||
| Cipher | PBEWithSHA1AndRC4_128 | |X | | | ||
| Cipher | PBEWithHmacSHA1AndAES_128 | |X | | |
There was a problem hiding this comment.
Any reason we are unable to support PBEWithMD5AndTripleDES which is a S1 PBE algorithm?
There was a problem hiding this comment.
PBEWithMD5AndTripleDES is a sun proprietary algorithm and doesn't have an OID available for it which we can use.
|
|
||
| public static final class SHA1AndRC2_40 extends PBEParameters { | ||
| public SHA1AndRC2_40() throws NoSuchAlgorithmException { | ||
| super("PBEWithSHA1RC2_40"); |
There was a problem hiding this comment.
Do these names need to match the algorithm name? If so seems like this should be PBEWithSHA1AndRC2_40 ?
There was a problem hiding this comment.
Similar problem below for PBEWithSHA1RC2_128
There was a problem hiding this comment.
Perhaps a simple test to validate the algorithm name used on a getInstance matches the name of within the parameters created could be added?
There was a problem hiding this comment.
Updated the subclass names.
We do have a test to compare the algorithms (
| this.iterationCount = pbespec.getIterationCount(); | ||
|
|
||
| if (keySalt != null && (!Arrays.equals(pbespec.getSalt(), keySalt))) { | ||
| throw new InvalidAlgorithmParameterException("Different iteration count between key and params"); |
There was a problem hiding this comment.
Should the error message here state salt not iteration count?
| } | ||
|
|
||
| // Get the NID. | ||
| id = ICC_OBJ_txt2nid(ockCtx, text); |
There was a problem hiding this comment.
Is this the method that is intermittantly failing at the moment on non aarch platforms?
There was a problem hiding this comment.
I kind of remember this kind of issue when doing the ML-KEM work. It had to do with over writing memory. Once, I found where I was not allocating the right amounts this seemed to go away. This maybe something similar.
There was a problem hiding this comment.
Thank you for your suggestion there John.
We were able to fix this by using GetStringUTFChars instead of GetPrimitiveArrayCritical and accordingly passing a jstring instead of jbyteArray.
src/main/native/PBE.c
Outdated
| passwordLength = (*env)->GetArrayLength(env, password); | ||
|
|
||
| /* | ||
| * Get the input data. Check for null is not required becuase that is |
There was a problem hiding this comment.
Spelling of because instead of becuase
| saltNative = NULL; | ||
| } | ||
| if (NULL != passwordNative) { | ||
| (*env)->ReleasePrimitiveArrayCritical(env, password, |
There was a problem hiding this comment.
I wasnt sure in this spot if we have a copy of the password or not allocated above with GetPrimitiveArrayCritical. From what i can tell this API may or may not be a copy so not sure we can zero out sensitive data ( password ) here.
dev-koan
force-pushed
the
implement_pbe1
branch
5 times, most recently
from
0007042 to
c9002b2
Compare
This update adds support for PBES1, PBEKeyFactory, and PBEParameters for PBES1 algorithms using native acceleration for OpenJCEPlus provider. PBE cipher tests have also been updated for better test coverage. Signed-off-by: Dev Agarwal <[email protected]>
dev-koan
force-pushed
the
implement_pbe1
branch
from
96baacb to
1789a89
Compare
3 participants
This update adds support for PBES1, PBEKeyFactory, and PBEParameters for PBES1 algorithms using native acceleration for OpenJCEPlus provider. PBE cipher tests have also been updated for better test coverage.
Signed-off-by: Dev Agarwal [email protected]