On Tue, 12 Jan 2021 23:20:11 GMT, Martin Balao <mba...@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
