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

[Michael StJohns]

In line

On 5/21/2021 5:01 PM, Xue-Lei Andrew Fan wrote:
> 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/AESParameters.java line 50:
>
>> 48:
>> 49:     public AESParameters() {
>> 50:         core = new BlockCipherParamsCore(AESConstants.AES_BLOCK_SIZE, 4, 8);
> A cipher object may not take different IV sizes at the same time.  I was just wondering how it could be used in practice.  Maybe something like:

The mode is KW - it has a fixed length 8 byte non-iv integrity tag.    
KWP is a special case of KW where there's still an 8 byte tag, but part 
of it is interpreted by KWP to figure out how much padding was 
included.   KW (AKA RFC3394) permits user (actually specification 
specified) IV values.  KWP (aka RFC5649) does not.

I'd treat KWP as a final (in the Java final sense) extension to KW with 
a fixed AIV flag value and a defined interpretation for the 8 byte AIV 
tag.  E.g. if you try to specify an IV for KWP, it should fail.   If 
someone else wants to do something like KWP or even twiddle with the 4 
byte AIV, let them do their own KW wrap around - which they should be 
able to do that via the KW/NoPadding model by specifying their own AIV.  
That should improve interoperability by preventing some monkey see 
monkey do errors.

This is sort of one reason I was arguing for AES/KW/KWPPadding rather 
than AES/KWP/NoPadding.


Mike



>
>
> AlgorithmParameters algParams = AlgorithmParameters.getInstance("AES");
> algParams.init(ivParameterSpec);
>
> The IV parameter is given with the init() method.  Then, it may be not necessary to construct the BlockCipherParamsCore object will all potential IV sizes.  See the comments in BlockCipherParamsCore.
>
> src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java line 52:
>
>> 50:     private byte[] iv = null;
>> 51:
>> 52:     private int[] moreSizes = null;
> The moreSizes is not used other than the init() method field.  It may be not easy to check the specific size if we cache all supported sized in the object.  For example, if the required IV size if 8 bytes, it may be a problem about how to make sure the iv size is 8 bytes exactly for a specific algorithm.
>
> Maybe, we could just have a ivSize filed.  The default value is block_size, which coupe be set with the init(ivParameterSpec) method.
>
>
>      ....
>      private int ivSize;
>      ...
>     BlockCipherParamsCore(int blkSize) {
>         block_size = blkSize;
>         ivSize = blkSize;
>      }
>      ...
>     void init(AlgorithmParameterSpec paramSpec) {
>          ivSize = ...;  // reset IV size.
>      }
>
>      // use ivSize while encoding and decoding.
>
> src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java line 81:
>
>> 79:                         expectedLen + " bytes long");
>> 80:         }
>> 81:         iv = tmpIv.clone();
> The moreSizes is not used after initialization.  The iv/tmpIv could be a value other than the block_size.   The getEncoded() method would use the iv value for the encoding.  While in the decoding method init(byte[]) method, the IV sizes other block_size is not considered, and IOE will be thrown.  Could this be a problem?
>
> -------------
>
> PR: https://git.openjdk.java.net/jdk/pull/2404