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

[Martin Balao]

On Fri, 8 Jan 2021 20:08:57 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Because a C_EncryptUpdate call that returns with an error here [1] implies that a session (with an active operation) is returned to the Session Manager here [2] [3]. For decryption, where we have proper padding on the Java side while doing an update, the test exercises the doFinal path. Decryption/Encryption is anecdotal here: what the test wants is coverage on both update and doFinal paths.
>> 
>> --
>> [1] - https://github.com/openjdk/jdk/blob/1cc09ccaef9a3695dd2862e3ee121e141e0a8a13/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L584
>> [2] - https://github.com/openjdk/jdk/blob/1cc09ccaef9a3695dd2862e3ee121e141e0a8a13/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L631
>> [3] - https://github.com/openjdk/jdk/blob/1cc09ccaef9a3695dd2862e3ee121e141e0a8a13/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L423
>
> 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

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

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