On Fri, 15 Jan 2021 20:28:28 GMT, Martin Balao <mba...@openjdk.org> wrote:
>> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an >> invalid block size), we now cancel the operation before returning the >> underlying Session to the Session Manager. This allows to use the returned >> Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error >> would be raised from the PKCS#11 library. >> >> The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is >> introduced as part of this PR. >> >> No regressions found in jdk/sun/security/pkcs11. > > Martin Balao has updated the pull request incrementally with one additional > commit since the last revision: > > Removing the encryption-update path in CancelMultipart test as it depends > on a know bug to cause a PKCS#11 error. Just made some feedback regarding comments, the actual changes look good. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 638: > 636: (new ShortBufferException().initCause(e)); > 637: } > 638: reset(true); Per PKCS#11 spec, "A call to C_EncryptUpdate which results in an error other than CKR_BUFFER_TOO_SMALL terminates the current encryption operation.", so I'd expect comment here to explain why we are doing reset(true). If not mentioning the known NSS behavior which triggered this change, at least comment the bug id so we don't lost track of the reason for the switch. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 756: > 754: (new ShortBufferException().initCause(e)); > 755: } > 756: reset(true); Comment for line 638 apply here as well. ------------- Marked as reviewed by valeriep (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1901
