RFR: 8289301: P11Cipher should not throw out of bounds exception during padding [v2]

[Valerie Peng]

On Tue, 28 Jun 2022 13:20:53 GMT, zzambers <duke at openjdk.org> wrote:

>> SunPkcs11 provider throws out of bounds exception during encryption when specific conditions are met.
>> 
>> Exception:
>> 
>> Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Array index out of range: 32
>> 	at java.base/java.util.Arrays.rangeCheck(Arrays.java:725)
>> 	at java.base/java.util.Arrays.fill(Arrays.java:3308)
>> 	at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher$PKCS5Padding.setPaddingBytes(P11Cipher.java:96)
>> 	at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.implDoFinal(P11Cipher.java:813)
>> 	at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.engineDoFinal(P11Cipher.java:585)
>> 	at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2417)
>> ...
>> 
>> 
>> Details:
>> This problems happens when reqBlockUpdates is true and implUpdate, which does not end on block boundary, is performed followed by final implUpdate, which ends exactly on block boundary. In that case final implUpdate fills padBuffer and then just returns. [1] Following implDoFinal then tries to add padding and throws OOB exception. Problem is, that in this case (input is multiple of block size) whole padding block should be added, but there is no space for it in padBuffer causing OOB exception.
>> 
>> Solution:
>> Solution is to detect this case (implDoFinal is called with full padBuffer) and to perform additional C_EncryptUpdate to free up padBuffer so that padBuffer can than be used to add whole new padding block.
>> 
>> [1] https://github.com/openjdk/jdk/blob/d4eeeb82cb2288973a6a247c54513f7e1c6b58f0/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L622
>
> zzambers has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Bug number and copyright date

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 820:

> 818:                         // unless padBuffer is already full in which case
> 819:                         // EncryptUpdate is performed and whole new padding
> 820:                         // block is created

suggestion on comment: 
// call C_EncryptUpdate first if the padBuffer is full to make room for padding bytes

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 918:

> 916:                         // unless padBuffer is already full in which case
> 917:                         // EncryptUpdate is performed and whole new padding
> 918:                         // block is created

Same suggestion on this as above.

test/jdk/sun/security/pkcs11/Cipher/TestPaddingOOB.java line 27:

> 25:  * @test
> 26:  * @bug 8289301
> 27:  * @summary P11Cipher should not throw OOB exception during padding

nit: add when "reqBlockUpdates" == true

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

PR: https://git.openjdk.org/jdk/pull/9310