On Fri, 8 Jan 2021 20:08:57 GMT, Valerie Peng <valer...@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
