On Fri, 14 May 2021 00:33:12 GMT, Valerie Peng <valer...@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
