Hi, here is a more plaintext-friendly version of the pull request Markdown body.
NOTE: I did manual wrapping to get a reasonably good representation in the archived version of this email [*], sorry if you are reading this in a <72 characters screen. On the other hand, not wrapping at all would lead to very long lines in wider screens. [*]: https://mail.openjdk.org/pipermail/security-dev/2023-February/034438.html ------------------------------------------------------------------------ We would like to propose an implementation for the JDK-8301553: Support Password-Based Cryptography in SunPKCS11 [1] enhancement requirement. In addition to pursuing the requirement goals and guidelines of JDK-8301553 [1], we want to share the following implementation notes (grouped per altered file): • src/java.base/share/classes/com/sun/crypto/provider/HmacPKCS12PBECore.java (modified) • This file contains the SunJCE implementation for the PKCS #12 General Method for Password Integrity [2] algorithms. It has been modified with the intent of consolidating all parameter checks in a common file (src/java.base/share/classes/sun/security/util/PBEUtil.java), that can be used both by SunJCE and SunPKCS11. This change does not only serve the purpose of avoiding duplicated code but also ensuring alignment and compatibility between different implementations of the same algorithms. No changes have been made to parameter checks themselves. • The new PBEUtil::getPBAKeySpec method introduced for parameters checking takes both a Key and a AlgorithmParameterSpec instance (same as the HmacPKCS12PBECore::engineInit method), and returns a PBEKeySpec instance which consolidates all the data later required to proceed with the computation (password, salt and iteration count). • src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java (modified) • This file contains the SunJCE implementation for the PKCS #5 Password-Based Encryption Scheme [3] algorithms, which use PBKD2 algorithms underneath for key derivation. In the same spirit than for the HmacPKCS12PBECore case, we decided to consolidate common code for parameters validation and default values in a single file (src/java.base/share/classes/sun/security/util/PBEUtil.java), that can serve both SunJCE and SunPKCS11 and ensure compatibility. However, instead of a single static method at the implementation level (see PBEUtil::getPBAKeySpec), we create an instance of an auxiliary class and invoke an instance method (PBEUtil.PBES2Params::getPBEKeySpec). The reason is to persist parameters data that has to be consistent between calls to PBES2Core::engineInit (in its multiple overloads) and PBES2Core::engineGetParameters, given a single PBES2Core instance. In particular, a call to any of these methods can potentially modify the state in an observable way by means of generating a random IV and a salt. Previous to the proposed patch, this data was persisted in the PBES2Core::ivSpec and PBES2Core::salt instance fields. For compatibility purposes, we decided to preserve SunJCE's current behavior. • src/java.base/share/classes/sun/security/util/PBEUtil.java (new file) • This utility file contains the PBE parameters checking routines and default values that are used by both SunJCE and SunPKCS11. These routines are invoked from HmacPKCS12PBECore (SunJCE), PBES2Core (SunJCE), P11PBECipher (SunPKCS11) and P11Mac (SunPKCS11). As previously noted, the goals are to avoid duplicate code and to improve compatibility between different security providers that implement PBE algorithms. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java (modified) • An utility function to determine if the token is NSS is now called. This function is in a common utility class (P11Util) and invoked from P11Key and P11SecretKeyFactory too. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java (modified) • A new type of P11 key is introduced: P11PBEKey. This new type represents a secret key that exists inside the token. Thus, this type inherits from P11SecretKey. At the same time, this type holds data used for key derivation. Thus, this type implements the javax.crypto.interfaces.PBEKey interface. In addition to the conceptual modeling, there are practical advantages of identifying a key by this new P11PBEKey type and holding the data used for derivation: 1) if the key is used in another token (different than the one where it was originally derived), a new derivation must take place; 2) if the key is passed to a non-SunPKCS11 security provider, its key translation method might use derivation data to derive again; and, 3) it's possible to return the PBEKeySpec for the key (see for example P11SecretKeyFactory::engineGetKeySpec). • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Mac.java (modified) • We decided to integrate PBE algorithms to the existing P11Mac service because the changes required have a low impact on the existing code. When the P11Mac instance is created, we use the algorithm to get PBE key information (if available). Only PBE algorithms have this type of information. In the P11Mac::engineInit method, we now need to handle the PBE service case. In such case, if the key is a P11Key, we check parameters and that the key implements javax.crypto.interfaces.PBEKey by calling PBEUtil::checkKeyParams. In other words, the key has to be a P11PBEKey and the parameters used for its derivation must match the ones passed in the invocation to P11Mac::engineInit. If the key is not a P11Key, a PBE derivation is needed. As for the SunJCE case, we go through parameters processing in PBEUtil::getPBAKeySpec. • There are two cases in which we need to call P11SecretKeyFactory::convertKey. One is when the service is not PBE, as we did before the proposed change. In the PBE case, we must call this function because it might be possible that, if the key token is not the same than the service's token, a new key derivation is required. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java (new file) • Contrary to the P11Mac case, we decided to separate PBE Cipher from non-PBE Cipher in a different class. There is some additional complexity or gap between the two that we prefer to keep simple. A PBE Cipher uses a non-PBE Cipher service underneath and forwards most of its operations, but adds wrapping code to potentially derive keys during initialization (see P11PBECipher::engineInit). The code associated to key derivation and parameters consistency checking is analogous to the one described for P11Mac. • P11PBECipher has a P11PBECipher::engineGetParameters method which calls PBEUtil.PBES2Params::getAlgorithmParameters and can potentially initialize an IV and a salt with a random value, as explained in the comments for PBES2Core. • A P11PBECipher service accepts PBE keys only. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java (modified) • The first significant change to this class that we want to discuss is the introduction of the KeyInfo class and the refactoring of the previous keyTypes map. Previous to the proposed change, the key information that we needed to retain for key creation at the PKCS #11 level was simple: key algorithm -> PKCS #11 native key type. With PBE, we must consider not only the algorithm name and key type but also (depending on the case) the mechanism that has to be used for derivation, the underlying derivation function and the key length. As an example, to derive a key for the PBEWithHmacSHA512AndAES_256 algorithm, we need to know that this algorithm maps to a PKCS #11 derivation mechanism value of CKM_PKCS5_PBKD2, a derivation function value of CKP_PKCS5_PBKD2_HMAC_SHA512, a derived key type of CKK_AES —so it can be used in an AES Cipher service— and a key length of 256 bits. A new hierarchy of classes to represent these different entries on the mapping structure has been introduced (see KeyInfo, PBEKeyInfo, AESPBEKeyInfo, PBKDF2KeyInfo and P12MacPBEKeyInfo). The methods to add or find entries in the new map have been adjusted. The previous pseudo key types strategy (HMAC, SSLMAC), that allows any key type to be used in a HMAC service, has not been modified. • The second significant change to this class was in the P11SecretKeyFactory::convertKey method. When checking if a key can be used in a service —notice here that the service can be any of SecretKeyFactory, Cipher or Mac—, the following rules apply: • If the key algorithm matches the service algorithm, the use is allowed • If the key algorithm does not match the service algorithm and the service is not one of the pseudo types, further checks are needed. The KeyInfo structure for the key and service algorithms are obtained from the map and the KeyInfo::checkUse method is invoked. The following principles apply to make a decision: 1) PBE services require a javax.crypto.interfaces.PBEKey of the same algorithm —we cannot use an AES key, for example, in a PBEWithHmacSHA512AndAES_256 Cipher service—, 2) PBKD2 keys can be used on any service —there is no information about the key purpose to make a decision— and 3) keys can be used in a service if their underlying type match —as an example, a PBEWithHmacSHA512AndAES_256 PBE key has an underlying type of CKK_AES and can be used in an AES Cipher service—. • The third significant change to this class was the addition of the P11SecretKeyFactory::derivePBEKey function. This function does different checks and creates the PKCS #11 structures to make a native call to C_GenerateKey to derive the key. They key returned, in case of success, is of P11PBEKey type. It is worth mentioning that this function is the only path to invoke a native key derivation. It's used not only by the PBE P11SecretKeyFactory service but also by PBE Cipher and PBE Mac services when they need to derive a key. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java (modified) • Added some utility functions. One of them is P11Util::encodePassword which serves the purpose of encoding a password in a way that can go through native library truncation (OpenJDK) and reach the PKCS #11 API in the expected encoding. Password encoding must be UTF-8 for PKCS #5 v2.1 derivation and BMPString (UTF-16 big endian) for PKCS #12 v1.1 General Method for Password Integrity derivation. By avoiding a modification to the existing native jCharArrayToCKUTF8CharArray function (p11_util.c), we reduce the risk of breaking existing code. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java (modified) • The new PBE algorithms are registered for SunPKCS11 services. It's worth noting that there is an additional (and optional) requiredMechs array to specify a list of native mechanisms that must be available in the token for the service to be enabled. To explain the need for this structure, we will focus on the HmacPBESHA1 Mac service example. On the one hand, we require the CKM_SHA_1_HMAC mechanism to be available in the token because this is, ultimately, a SHA-1 HMAC operation. However, the CKM_PBA_SHA1_WITH_SHA1_HMAC mechanism must be available as well for the preceding PBE key derivation. In the PBKDF2 case, we leverage on this structure to require a mechanism associated to the derivation function. The assumption is that, for example, if the CKM_PKCS5_PBKD2 and CKM_SHA_1_HMAC mechanisms are available, a PBKDF2 derivation using HMAC SHA-1 underneath will be available. In this case, the derivation function is represented by the CKP_PKCS5_PBKD2_HMAC_SHA1 constant. • As mentioned in JDK-8301553 [1], for some algorithms there isn't a PKCS #11 constant and we use the NSS vendor-specific one. This code can be easily be updated in the future if a constant is introduced to the standard. We don't know if that will ever happen as the newer PKCS #5 derivation for Mac (PBMAC1) might be considered in the future as a PKCS #12 integrity scheme replacement. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_ECDH1_DERIVE_PARAMS.java (modified) • Minor comment fix. This mistake was probably the result of using the CK_PKCS5_PBKD2_PARAMS file as a template and forgetting to update the comment. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_MECHANISM.java (modified) • New constructors for the CK_PBE_PARAMS, CK_PKCS5_PBKD2_PARAMS and CK_PKCS5_PBKD2_PARAMS2 structures that can be used along with a CK_MECHANISM. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_PBE_PARAMS.java (modified) • Some minor adjustments to comments and a constructor to make this class usable with the PKCS #12 General Method for Password Integrity derivation. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_PKCS5_PBKD2_PARAMS.java (modified) • Same than for CK_PBE_PARAMS. It is worth noting that this structure is the one used in PKCS #11 revisions previous to v2.40 Errata 01. Given that NSS has decided to keep using it —even when it's not compliant with the latest revisions of the v2.40 and v3.0 standards—, we make an exception for it. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_PKCS5_PBKD2_PARAMS2.java (new file) • The new structure for passing PBE parameters to the PKCS #11 token in the PKCS #5 v2.1 derivation scheme. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_X9_42_DH1_DERIVE_PARAMS.java (modified) • Same comment than for CK_ECDH1_DERIVE_PARAMS. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java (modified) • More visibility of major and minor versions of the PKCS #11 standard implemented by a token is needed to decide between the CK_PKCS5_PBKD2_PARAMS and CK_PKCS5_PBKD2_PARAMS2 structures. • src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java (modified) • New constants added. • src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_convert.c (modified) • Adjustments made to work with the structures to pass parameters to the token for PBE derivation. It's worth noting that native PBKD2 parameter structures have a tag before the data so we can execute the correct logic to free up resources, once the operation is completed. This is how we differentiate a CK_PKCS5_PBKD2_PARAMS from a CK_PKCS5_PBKD2_PARAMS2 one. • src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c (modified) • Adjustments to work with the new PBE parameter structures. • A bug affecting non-null Java arrays whose length is 0 and need to be converted to native PKCS #11 arrays has been fixed. For these arrays, it was possible that some platforms return NULL as a result of calling memory allocation functions when the size was 0 and an OutOfMemory exception was incorrectly thrown. • src/jdk.crypto.cryptoki/share/native/libj2pkcs11/pkcs11wrapper.h (modified) • Native constants and structures added. Test files • test/jdk/sun/security/pkcs11/Cipher/PBECipher.java (new file) • Tests the PBE Cipher service in SunPKCS11, cross-comparing results against SunJCE (if available) and static data. • The PBE Cipher service is tested with different types of keys: derived from data in a PBEParameterSpec, derived with a SunPKCS11 SecretKeyFactory service, derived from data in AlgorithmParameters and derived from data contained in a javax.crypto.interfaces.PBEKey instance. • test/jdk/sun/security/pkcs11/KeyStore/ImportKeyToP12.java (new file) • Tests that, for several PBE algorithms, PKCS #12 key stores (with Privacy and Integrity) written using SunPKCS11 underneath can be read using SunJCE underneath. • test/jdk/sun/security/pkcs11/Mac/MacSameTest.java (modified) • This test was not expecting PBE services to be available in the SunPKCS11 security provider, and needs to invoke a new function (PKCS11Test::generateKey) to generate a random key (password). • test/jdk/sun/security/pkcs11/Mac/PBAMac.java (new file) • Similar to PBECipher but these are the possible types of keys: derived from data in a PBEParameterSpec, derived with a SunPKCS11 SecretKeyFactory service and derived from data contained in a javax.crypto.interfaces.PBEKey instance. • test/jdk/sun/security/pkcs11/Mac/ReinitMac.java (modified) • Same issue fixed than for MacSameTest. • test/jdk/sun/security/pkcs11/PKCS11Test.java (modified) • Functions to generate random keys or passwords for PBE and non-PBE algorithms. • test/jdk/sun/security/pkcs11/SecretKeyFactory/TestPBKD.java (new file) • In addition to testing derived keys for different algorithms against SunJCE and static assertion data, this test asserts: 1) different types of valid and invalid key conversions, and 2) invalid or inconsistent parameters passed for key derivation. Keys are derived with data contained in a PBEKeySpec or in a javax.crypto.interfaces.PBEKey instance. • Both an empty and a unicode password, containing a non-ASCII character, are used during this test. Testing • No regressions have been observed in the jdk/sun/security/pkcs11 category (SunPKCS11). • No regressions have been observed in the jdk/com/sun/crypto/provider category (SunJCE). • No regressions have been observed in the JDK Tier-1 category. • Anecdotally, a partial version of the proposed patch containing Cipher and Mac changes is shipped in Red Hat Enterprise Linux builds of OpenJDK 17 since November 2022, without any known issues at this moment. This contribution is co-authored between fferr...@redhat.com and mba...@redhat.com. We are both under the cover of the OCA agreement per our employer (Red Hat). We look forward to sharing this new feature for the benefit of the broad OpenJDK community and users. [1] https://bugs.openjdk.org/browse/JDK-8301553 [2] https://datatracker.ietf.org/doc/html/rfc7292#appendix-B [3] https://datatracker.ietf.org/doc/html/rfc8018#section-6.2 ------------------------------------------------------------------------ Regards, -- Francisco Ferrari Bihurriet <fferr...@redhat.com> Senior Software Engineer Red Hat Argentina (https://www.redhat.com) A00A 468A 5506 5D16 9851 E3A9 CD8B C3EF 772B BD6F
