RFR: 8248268: Support KWP in addition to KW [v7]

[Xue-Lei Andrew Fan]

On Fri, 14 May 2021 00:33:12 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded classes which extend FeedbackCipher and used in KeyWrapCipher class. To minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto operation over the same input buffer which is allocated and managed by KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
> 
>  - Merge master into JDK-8248268
>  - Minor update to address review comments.
>  - Changed AESParameters to allow 4-byte, 8-byte IVs and removed
>    KWParameters and KWPParameters.
>  - Refactor code to reduce code duplication
>    Address review comments
>    Add more test vectors
>  - Changed AlgorithmParameters impls to register under AES/KW/NoPadding and
>    AES/KWP/NoPadding
>  - Restored Iv algorithm parameters impl.
>  - 8248268: Support KWP in addition to KW
>    
>    Updated existing AESWrap support with AES/KW/NoPadding cipher
>    transformation. Added support for AES/KWP/NoPadding and
>    AES/KW/PKCS5Padding support to SunJCE provider.

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 176:

> 174:             return ConstructKeys.constructPublicKey(encoding, keyAlgorithm);
> 175:         default:
> 176:             throw new RuntimeException("Unsupported key type");

Good improvement to thrown an exception rather than return "null"!

Is NoSuchAlgorithmException fitted better the specification?  See also the comment in  KeyWrapCipher.engineUnwrap().

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 190:

> 188:                 return constructKey(encoding, keyAlgorithm, keyType);
> 189:             } else {
> 190:                 byte[] encoding2 = Arrays.copyOfRange(encoding, ofs, ofs + len);

The array will be copied again in the X509EncodedKeySpec and PKCS8EncodedKeySpec.  It is out of this scope, but it may be a performance improvement if adding X509EncodedKeySpec and PKCS8EncodedKeySpec constructors that accept array offset, so that the array copy here could be avoid.

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 192:

> 190:                 byte[] encoding2 = Arrays.copyOfRange(encoding, ofs, ofs + len);
> 191:                 try {
> 192:                     return constructKey(encoding2, keyAlgorithm, keyType);

The two constructKey methods are depended on each other.  It may improve the readability if only having one-way dependence, by moving the switch-block logic from the former constructKey() to the later one.


 static final Key constructKey(byte[] encoding, String keyAlgorithm,
       int keyType) throws InvalidKeyException, NoSuchAlgorithmException {
    return constructKey(encoding, 0. encoding.length, keyAlgorithm, keyType);
}


With this re-org, the array copy could be avoid if the key type is unknown.

src/java.base/share/classes/com/sun/crypto/provider/KeyWrapCipher.java line 710:

> 708:             outLen = helperDecrypt(dataBuf, dataIdx);
> 709:             return ConstructKeys.constructKey(dataBuf, 0, outLen,
> 710:                     wrappedKeyAlgorithm, wrappedKeyType);

Per the specification of engineUnwrap,  maybe, the ConstructKeys.constructKey() implementation could throw NoSuchAlgorithmException if wrappedKeyType is unknown.  See the common in the the ConstructKeys.constructKey() implementation.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2404