RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v3]

[Valerie Peng]

On Tue, 12 Jan 2021 23:20:11 GMT, Martin Balao <mbalao at openjdk.org> wrote:

>> I understand that the intention/design is to trigger the error condition on various code paths causing the active session to be returned to pool and verify if this issue is fixed or not. What I don't get is that why AES/ECB/NoPadding cipher encrypting a byte[1] w/ an update (where more input could still be given) would trigger an error. The stack trace for the encryption update call (which is the first test, the session should be freshly allocated and not a reused one with active state) is below:
>> 
>> java.security.ProviderException: update() failed
>>         at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.implUpdate(P11Cipher.java:632)
>>         at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.engineUpdate(P11Cipher.java:529)
>>         at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.engineUpdate(P11Cipher.java:517)
>>         at java.base/javax.crypto.Cipher.update(Cipher.java:1832)
>>         at CancelMultipart$LeakByteArray.doOperation(CancelMultipart.java:125)
>>         at CancelMultipart$SessionLeaker.leakAndTry(CancelMultipart.java:63)
>>         at CancelMultipart.executeTest(CancelMultipart.java:158)
>>         at CancelMultipart.main(CancelMultipart.java:140)
>>         at PKCS11Test.premain(PKCS11Test.java:171)
>>         at PKCS11Test.testNSS(PKCS11Test.java:568)
>>         at PKCS11Test.main(PKCS11Test.java:207)
>>         at CancelMultipart.main(CancelMultipart.java:131)
>>         at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>         at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>>         at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>         at java.base/java.lang.reflect.Method.invoke(Method.java:567)
>>         at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>>         at java.base/java.lang.Thread.run(Thread.java:831)
>> Caused by: sun.security.pkcs11.wrapper.PKCS11Exception: CKR_DATA_LEN_RANGE
>>         at jdk.crypto.cryptoki/sun.security.pkcs11.wrapper.PKCS11.C_EncryptUpdate(Native Method)
>>         at jdk.crypto.cryptoki/sun.security.pkcs11.P11Cipher.implUpdate(P11Cipher.java:584)
>>         ... 17 more
>
> The update fails because the native mechanism (CKM_AES_ECB) has no padding and OpenJDK does not buffer data in the Java side for encryption [1] (this is a bug that I'll address soon). As a result, there is a PKCS#11 call with an invalid length and we get the error that ends up returning the session to the Session Manager. I just realized that when we fix the previous padding-bug, this test path won't work anymore. CKR_BUFFER_TOO_SMALL errors on updates do not lead to a reset call in the OpenJDK side (contrary to doFinal), so they wouldn't be useful for the test. I'll investigate if there is a way to trigger the path. Otherwise we should keep the doFinal path only. I'd still force a reset if there is an error other than CKR_BUFFER_TOO_SMALL in the update.
> 
> --
> [1] - https://github.com/openjdk/jdk/blob/1cc09ccaef9a3695dd2862e3ee121e141e0a8a13/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L583

It's an update call, isn't padding occur when doFinal() is called for encryption?
In any case, it's best for the test case to not have this bug dependency. I am ok if you can only test doFinal path only.

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

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