RFR: 8264849: Add KW and KWP support to PKCS11 provider

Anthony Scarpino ascarpino at openjdk.java.net
Thu Sep 30 20:00:31 UTC 2021


On Fri, 17 Sep 2021 23:22:21 GMT, Valerie Peng <valeriep at 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



More information about the security-dev mailing list