On Thu, 7 Jan 2021 20:51:02 GMT, Martin Balao <mba...@openjdk.org> wrote:
> > @valeriepeng are you okay with this reasoning? > I changed my mind around this decision and I'm inclined not to make any code changes to P11Signature. Only a documentation note that reflects this analysis should be needed. First of all, what I described here [1], about the NSS Software Token behavior, matches the PKCS#11 v2.20 standard [2]: 1) "A call to C_SignFinal always terminates the active signing operation unless it returns CKR_BUFFER_TOO_SMALL or is a successful call"; and, 2) "A call to C_Sign always terminates the active signing operation unless it returns CKR_BUFFER_TOO_SMALL". In addition, as described here [3], CKR_BUFFER_TOO_SMALL is not expected to reach P11Signature Java code because it's handled by the libj2pkcs11 OpenJDK native library. Thus, cancelling a potentially active operation is not required by the standard nor needed. If we find a non-compliant implementation of the PKCS#11 standard in the future, we might want to revisit this decision. Even if the performance cost of cancelling an operation 'just in case' should be affordable, we might unnecessarily get into errors such as CKR_OPERATION_NOT_INITIALIZED. The P11Cipher case is different because the size of the output buffer (the one that may lead to a CKR_BUFFER_TOO_SMALL error) is a user input and the error visible to OpenJDK Java code [4] [5] [6] [7]. In addition, and contrary to the PKCS#11 v2.20 standard -which states "A call to C_EncryptUpdate which results in an error other than CKR_BUFFER_TOO_SMALL terminates the current encryption operation."-, the NSS Software Token may not terminate the operation on other error types [8] [9]. This is why we need to always cancel from P11Cipher. -- [1] - https://git.openjdk.java.net/jdk/pull/1901#issuecomment-756312031 [2] - https://www.cryptsoft.com/pkcs11doc/v220/group__SEC__11__11__SIGNING__AND__MACING__FUNCTIONS.html [3] - https://git.openjdk.java.net/jdk/pull/1901#issuecomment-756376546 [4] - https://github.com/openjdk/jdk/blob/cd94606c0c2dbf0a7f6d08dcc27f787ed080ac15/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c#L280 [5] - https://github.com/openjdk/jdk/blob/cd94606c0c2dbf0a7f6d08dcc27f787ed080ac15/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c#L228 [6] - https://github.com/openjdk/jdk/blob/cd94606c0c2dbf0a7f6d08dcc27f787ed080ac15/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c#L463 [7] - https://github.com/openjdk/jdk/blob/cd94606c0c2dbf0a7f6d08dcc27f787ed080ac15/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c#L514 [8] - https://github.com/nss-dev/nss/blob/561b982921f15902088a3f05b1cdf561f5f64cb7/lib/softoken/pkcs11c.c#L1356 [9] - https://github.com/nss-dev/nss/blob/561b982921f15902088a3f05b1cdf561f5f64cb7/lib/softoken/pkcs11c.c#L1380 ------------- PR: https://git.openjdk.java.net/jdk/pull/1901
