AES KW support

[sergey-gubenko]

Add support for the AesKeyWrapSpi.
AES KW is defined in RFC-3394.
This new cipher should be initialized as AES with KW and NoPadding.

Current ACCP support AES key wrap with padding:

https://github.com/corretto/amazon-corretto-crypto-provider/blob/main/src/com/amazon/corretto/crypto/provider/AesKeyWrapPaddingSpi.java

I have source code for the above that I can contribute available.

[geedo0]

Hi there, thanks for reaching out. I see that support for AES-KW exists inside AWS-LC here and it should be possible to add support for it to ACCP with the correct input validation in the Java layer. The only thing that gives me pause is this comment in the header file. To be fair, this same comment exists for AES-KWP which as you've pointed out already exists in ACCP. Can you provide some more details about your use case? We'll have to discuss this as a team to make sure the risks/benefits work out favorably.

// These functions implement AES Key Wrap mode, as defined in RFC 3394. They
// should never be used except to interoperate with existing systems that use
// this mode.

[sergey-gubenko]

Thanks, our use case is that we're using the not padded version of AES KW to wrap symmetric keys. The existing KWP implementation ACCP supports doesn't help because it's a different algorithm producing different non-compatible with KW results. I have an implementation of AesKeyWrapSpi available to contribute including the known answer tests provided by NIST https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Algorithm-Validation-Program/documents/mac/KWVS.pdf.

[geedo0]

After talking with the team, we're open to reviewing a PR for AES-KW. Development-wise, was this largely derivative of the existing AES-KWP implementation or did you have to do something substantially different?

[sergey-gubenko]

Thanks, I would say it's largely a derivative, the KW logic is similar to what KWP does today, except for the actual key wrap unwrap logic, I'm using AES_wrap_key instead of AES_wrap_key_padded, test cases are also different.

[sergey-gubenko]

Hi Gerardo, can you, please, take a look at the AES KW PR when you have a chance?

#498

[geedo0]

Ack, thanks for the contribution!

[sergey-gubenko]

Hi Gerardo, thanks a lot for taking a look at the PR, how do you want me to resolve the comments, in a separate commit or update the existing ones?

[geedo0]

No problem, sorry for the delay on getting back to you. A separate commit on the same branch would be helpful. That let's me see the inter-revision diffs from the PR tool.

[sergey-gubenko]

Hi Gerardo, sorry for a basic question here, what is the right process to get this reviewed? Are we waiting for someone else from your team to review this?

[geedo0]

Confirmed that one of my colleagues is in the midst of reviewing it now. We need two approvals to merge. Do you need a new release of ACCP into Maven after this is merged?

[sergey-gubenko]

hi Gerardo, we don't need a new release after this is merged, but would welcome if there were one.

[sergey-gubenko]

I also have for review a small PR to migrate KWP tests to the new thread class, plus fixing the testEngineGetParametersAndIv test. It seems like original intention was to test 4 different modes instead of running Cipher.ENCRYPT_MODE 4 times. Can you, please, take a look when you have a chance?

https://github.com/corretto/amazon-corretto-crypto-provider/pull/501/files