On Mon, 27 May 2024 14:22:51 GMT, Francisco Ferrari Bihurriet <fferr...@openjdk.org> wrote:
>> Francisco Ferrari Bihurriet has updated the pull request with a new target >> base due to a merge or a rebase. The incremental webrev excludes the >> unrelated changes brought in by the merge/rebase. The pull request contains >> seven additional commits since the last revision: >> >> - Revert re-arrangement of native methods parameters >> >> This reverts commit 0a777e94229723376e1264e87cbf0ba805dc736f, except for >> the copyright which is retained as 2024. >> >> NOTE: new calls of the same methods are retained in the re-arrangement >> style, as we didn't introduce this re-arrangement, it was already >> present in most of the calls inside ::implUpdate() and ::implDoFinal(). >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> >> - Merge 'openjdk/master' into JDK-8330843 >> - 8330842: Add AES CBC with Ciphertext Stealing (CTS) SunPKCS11 tests >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> >> - 8330842: Support AES CBC with Ciphertext Stealing (CTS) in SunPKCS11 >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> >> - Fix cipher tests logging >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> >> - Implement integer constants as enum >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> >> - Arrange parameters of native methods evenly >> >> C_EncryptUpdate / C_DecryptUpdate / C_EncryptFinal / C_DecryptFinal >> >> If the call doesn't fit on a single line, use the following order: >> long hSession, >> [ long directIn, byte[] in, int inOfs, int inLen, ] >> long directOut, byte[] out, int outOfs, int outLen >> >> [ ]: optional, not present in C_EncryptFinal / C_DecryptFinal >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> > > The [PKCS #<wbr/>11 definition of > `CKM_AES_CTS`](https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/pkcs11-curr-v3.0.html#_Toc30061253) > refers to the [Addendum to NIST Special Publication 800-38A, "Recommendation > for Block Cipher Modes of Operation: Three Variants of Ciphertext Stealing > for CBC Mode"](https://doi.org/10.6028/NIST.SP.800-38A-Add) document. > However, the standard does not prescribe which of the three CTS variants > described by the document to use. This left the door open for PKCS #<wbr/>11 > implementers to make different assumptions and create interoperability issues. > > We are aware that the _NSS Software Token_ implemented the CS1 variant (see > [lib/freebl/cts.c](https://github.com/nss-dev/nss/blob/NSS_3_90_RTM/lib/freebl/cts.c#L64-L65)), > but [public clarification had to be given in NSS Bug > 373108](https://bugzilla.mozilla.org/show_bug.cgi?id=373108#c7). That's why > we chose `CS1` as the `cipherTextStealingVariant` default. > >> As for the different variations, are you aware of different vendors >> supporting different variations? > > At the moment of developing this enhancement, we were not aware of any other > variant in use, but we had only evaluated NSS. The standard has a clear > ambiguity, so there was a latent problem. Now, I have found another open > source project using CS3 instead (see below). > >> This assumes prior knowledge on the variation used by underlying PKCS11 >> library, right? > > Unfortunately yes, but it gives the user a chance to adjust for their PKCS11 > library without the need of a _SunPKCS11_ update from our side. > >> Are all these variations under "CTS" name? > > Yes, they are known as "variants" —according to the mentioned NIST addendum— > or "formats" —according to > [Wikipedia](https://en.wikipedia.org/wiki/Ciphertext_stealing#Ciphertext_format > "Wikipedia: Ciphertext stealing - Ciphertext format")—, always in the > context of "Cipher Text Stealing". > >> Are they generally supported by all? > > According to _Wikipedia_, the most popular alternative is CS3. It also > happens to be the Kerberos choice, and so the one implemented by _SunJCE_'s > `AES/CTS/NoPadding`. However, NSS chose CS1 as it was the first one described > in the NIST addendum, referred by the PKCS #<wbr/>11 standard. > > In the sake of interoperability, the PKCS #<wbr/>11 standard should choose > one of the three variants. But since it is too late, PKCS #<wbr/>11 > implementers supporting `CKM_AES_CTS` have already made their choice. > > ### OP-TEE provides a PKCS #<wbr/>11 interface, and has chosen CS... > I'm personally okay with not having a default value and forcing users to > define the configuration property to enable CTS. Perhaps we can document in > the CSR/guide that the NSS Software Token requires CS1 as an example. > @franferrax what do you think? @martinuy: I agree, it makes sense to require the `cipherTextStealingVariant` configuration to be explicitly set in order for _SunPKCS11_ to register the _Cipher_ services that depend on `CKM_AES_CTS` (`AES*/CTS/NoPadding` algorithm names). Done in 997777e86c6fa03f070dcf0f219813c11cb480ce, I will adjust the CSR tomorrow. ### New `cipherTextStealingVariant` behaviour We slightly changed the error message format when an invalid value is passed. Here it is an example for `cipherTextStealingVariant=CS4`: sun.security.pkcs11.ConfigurationException: cipherTextStealingVariant must be one of [CS1, CS2, CS3], read: Token[CS4], line 33 If no value is passed, the `CKM_AES_CTS` mechanism is [handled as if it were disabled by the configuration](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L1302-L1308). This is the message logged when running with `-Djava.security.debug=sunpkcs11`: Mechanism CKM_AES_CTS: ulMinKeySize: 16 ulMaxKeySize: 32 flags: 768 = CKF_ENCRYPT | CKF_DECRYPT DISABLED due to an unspecified cipherTextStealingVariant in configuration As an exception, to improve the user experience, when no value is passed but the token is NSS ([by means of the token label](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java#L57-L64)), `cipherTextStealingVariant` [is assumed to be `CS1`](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Token.java#L425-L430). ------------- PR Comment: https://git.openjdk.org/jdk/pull/18898#issuecomment-2138360201
