On Fri, 17 Sep 2021 23:22:21 GMT, Valerie Peng <valer...@openjdk.org> wrote:
> Anyone has time to review this RFE for adding AES cipher with KW, KWP modes
> support to SunPKCS11 provider?
>
> The main changes are in only one new class, i.e. P11KeyWrapCipher.java, which
> is the CipherSpi impl for the native PKCS11 key wrap mechanisms. When testing
> against NSS library, it seems that they only support the single part enc/dec
> PKCS11 APIs, so have to use a new class as existing P11Cipher class relies on
> the multi part enc/dec PKCS11 APIs and do not support key wrapping/unwrapping.
>
> The rest are minor code refactoring and updates for the PKCS11 Exception
> class.
> The new regression tests are adapted from existing key wrap regression tests
> for SunJCE provider.
>
> Thanks,
> Valerie
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java
line 824:
> 822: } else if (e.match(CKR_ENCRYPTED_DATA_INVALID) ||
> 823: e.match(CKR_GENERAL_ERROR)) {
> 824: // CKR_GENERAL_ERROR is Solaris-specific workaround
With Solaris no longer support, this could be removed. Are you leaving it for
backporting?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyWrapCipher.java
line 57:
> 55: * doFinal() is called.
> 56: *
> 57: * @since 18
Is there only suppose to be one space between `@since` and 18?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyWrapCipher.java
line 129:
> 127: if (algoParts[0].startsWith("AES")) {
> 128: // need 3 parts
> 129: if (algoParts.length != 3) {
At this point in the code, isn't it already certain to be a valid transform?
The SunPKCS11 entries are limited to the valid transforms. Additionally do
you really want AssertionError? Not NoSuchAlgorithmException?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyWrapCipher.java
line 437:
> 435: protected byte[] engineDoFinal(byte[] in, int inOfs, int inLen)
> 436: throws IllegalBlockSizeException, BadPaddingException {
> 437: int minOutLen = doFinalLength(inLen);
nit: seems like this could be maxOutLen given it's the length used to allocate
out[]. It can't be any larger, otherwise the operations would fail
test/jdk/sun/security/pkcs11/Cipher/KeyWrap/TestCipherKeyWrapperTest.java line
65:
> 63: // com/sun/crypto/provider/Cipher/KeyWrap/TestCipherKeyWrapperTest.java
> 64: public class TestCipherKeyWrapperTest extends PKCS11Test {
> 65: private static final int LINIMITED_KEYSIZE = 128;
Did you mean this to be LIMITED_KEYSIZE?
-------------
PR: https://git.openjdk.java.net/jdk/pull/5569
