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

[Valerie Peng]

On Sat, 22 May 2021 00:45:27 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> 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 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.

Right, maybe there is no constructor which takes offset and length due to trying to avoid the cost of validating them. I recall seeing other *Spec classes whose list of constructors are similar to X509EncodedKeySpec/PKCS8EncodedKeySpec as well.

> 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.

Yes, I will refactor the two constructKey() methods as you suggested.

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

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