RFR: 8261355: No data buffering in SunPKCS11 Cipher encryption when the underlying mechanism has no padding [v2]

Martin Balao mbalao at openjdk.java.net
Thu Mar 25 22:20:29 UTC 2021


On Tue, 2 Mar 2021 13:16:24 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Martin Balao has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>> 
>>  - Avoid overriding buffered bytes with padding in the doFinal call.
>>  - Only do encryption block-size buffering for NSS
>>  - 8261355: No data buffering in SunPKCS11 Cipher encryption when the underlying mechanism has no padding
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 595:
> 
>> 593:                     // NSS throws up when called with data not in multiple
>> 594:                     // of blocks. Try to work around this by holding the
>> 595:                     // extra data in padBuffer.
> 
> Well, I am not sure if other PKCS#11 libraries are like NSS which requires input size to be multiple of blocks for every multi-part encryption/decryption calls. We are paying the cost of buffering non-blocksize data ourselves and the associated byte copying as a result. Oh-well.
> 
> With this change, you should also update the implDoFinal() impl which calls paddingObj.setPaddingBytes(byte[], int) for encryption and writes the padding bytes "after" the existing buffered bytes, i.e. padBufferLen. Otherwise, the existing buffered bytes may be overwritten w/ padding bytes and things will fail. The new regression test should cover this scenario also. It currently only tests the changes made to update() calls.

I've pushed a new proposal to limit the performance impact of Java-side buffering to the NSS library. This adds to the previous conditions: the operation has to be encryption and the mechanism must not have native padding. If we realize in the future that other libraries are affected as well, we can easily extend the scope.

In regards to the implDoFinal bug, well spotted! Fixed in this new proposal and the test has been enhanced to cover not only this case but also different padding sizes and different block numbers.

Branch rebased (today) to the latest master.

Look forward to your comments.

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

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


More information about the security-dev mailing list