137.89 KB, application/pdf
Robert Relyea: review-
17.58 KB, patch
|Details | Diff | Splinter Review|
56.31 KB, patch
|Details | Diff | Splinter Review|
18.84 KB, patch
Wan-Teh Chang: review+
Wan-Teh Chang: checked-in+
|Details | Diff | Splinter Review|
In order to get TLS 1.2 working in FIPS mode, softoken will have to implement new mechanisms. However, it seems like the PKCS#11 interface for the TLS 1.2 mechanisms was never standardized; at least, there isn't any mention of them in the PKCS#11 2.30 drafts. The mechanisms themselves are easy to implement and can reuse most of the code from the existing CKM_TLS mechanisms. In a 2007 email to the CRYPTOKI mailing list, Bob Relyea suggested these mechanisms be named CKM_TLS_1_2_PRF, CKM_TLS_1_2_MASTER_KEY_DERIVE, CKM_TLS_1_2_MASTER_KEY_DERIVE_DH, and CKM_TLS_1_2_KEY_AND_MAC_DERIVE, and work analagously to the existing CKM_TLS_* mechanisms. However, it seems like there was no progress on the issue after that email and so these constants were never assigned or documented anywhere except his email. It may take some time to get these mechanisms into the standard, I think it would be OK to put them in the vendor-defined space (CKM_VENDOR_DEFINED) for now, so that work on TLS 1.2 is not held up unreasonably. I think it is very important for these mechanisms to be added before the next round of FIPS 140-2 certification, as I believe that libssl's TLS 1.2 implementation cannot be made FIPS compliant if the PKCS#11 module doesn't implement them. (I would love to be proven wrong though.)
I think the above assessment is correct except for this part: > before the next round of FIPS 140-2 certification, As I understand it, if we make changes (e.g. bug fixes) to existing parts of our recently FIPS 140-2 certified module, we can get those changes certified under FIPS 140-2 rules. But any major new features (and I believe that new mechanisms for TLS 1.2 would qualify as major new features) would need to be part of a new certification, not a recertification, and any new certification beginning now would be against FIPS 140-3, not 140-2, IINM. If I'm mistaken about that, I hope Bob or Wan-Teh will correct this.
FIPS 140-3 is still a draft. I don't know when FIPS 140-3 will be ratified.
NSS has traditionally used the undocumented (non-standard?) CKM_TLS_PRF_GENERAL mechanism for computing the finished hashes. I read Nelson's proposal for this in the CRYPTOKI mailing list archive and I agree that it is more intuitive for this mechanism to be thought of as a hash/digest function. However, CKM_TLS_PRF_GENERAL requires at least two calls into the PKCS#11 modile (C_DigestInit and C_Digest) per finished message, whereas CKM_TLS_PRF does all its work in one call. Plus, CKM_TLS_PRF can be implemented without heap allocations whereas CKM_TLS_PRF_GENERAL cannot (because it must store state across multiple calls). So, I think for TLS 1.2 it makes more sense to go with an interface mimicing CKM_TLS_PRF. Also, in his proposal, Nelson mentioned TLS Extractor (now known as TLS Key Material Exporters, RFC5705) as a potential use case for CKM_TLS_PRF_GENERAL. However, CKM_TLS_PRF_GENERAL cannot be used to implement RFC5705 in a FIPS-compliant way. Instead, a new mechanism for C_DeriveKey would need to be created to generate a keyblock, and then the CKM_EXTRACT_KEY_FROM_KEY would need to be used to derive the actual keys from the keyblock. So, it looks like there is no use for a TLS 1.2 version of CKM_TLS_PRF_GENERAL. AFAICT, CKM_TLS_MASTER_KEY_DERIVE allows the application to extract the first two bytes out of basically any 48-byte secret key, so that the application can check the client hello version number embedded in those two bytes. This seems completely wrong to me; either a new key type needs to be defined, or the handling of the version number needs to be moved to the unwrapping mechanism. In my prototype, I have implemented the latter approach. Specifically, I created a CKM_TLS_P_SHA256_RSA_PRE_MASTER_KEY_UNWRAP mechanism. The mechanism takes a client hello number as its mechanism input. It unwraps the premaster secret and compares the embedded version number with the version number given. It returns the same error code if any of the following happen: the version numbers do not match, the premaster secret isn't exactly 48 bytes long, or there is some other decoding error during RSA decryption. IMO, this is clearly a much better interface as it reduces the amount of leakage out of the module to the absolute minimum that is possible except for disabling RSA key exchange completely. If the application would want to to peek at what the embedded version number was, it can decrypt the encoded PMS, if it is allowed to do so. With this new unwrapping mechanism, CKM_TLS_1_2_MASTER_KEY_DERIVE can be defined identically, regardless of the key exchange mechanism. In particular, RSA would be handled the same as D-H. The mechanism parameter would be a direct pointer to the CK_SSL3_RANDOM_DATA structure, and it would not deal with the version number at all. So, there would be no CKM_TLS_1_2_MASTER_KEY_DERIVE_DH. (Or, put another way, there would be no RSA-specific master key derivation mechanism, and CKM_TLS_1_2_MASTER_KEY_DERIVE_DH would be renamed CKM_TLS_1_2_MASTER_KEY_DERIVE). Finally, although I've been using the prefix "CKM_TLS_1_2" here for consistency with Bob's proposal, I am actually using the prefix CKM_TLS_P_SHA256. Bob's proposal had an interesting idea, where the mechanism identifier for a hash function would be used included as a parameter for all mechanisms. However, in looking over the PKCS#11 documentation for other mechanisms, it seems like the mechanism identifier is supposed to identify all algorithms to be used in the operation. I think Bob's approach would be very useful, but I imagine it would be problematic given that applications are not expecting to manage pairs of algorithms in this way. Accordingly, I have been using the prefix CKM_TLS_P_SHA256_ (actually CKM_NSS_TLS_P_SHA256_) to identify all the mechanisms. Or, if you don't like all of that, a CKM_SHA256_HMAC_KEY_DERIVATION mechanism could be defined and implemented. It is possible to implement P_SHA256 (and thus the TLS 1.2 PRF) in a FIPS-able way in the application using such a mechanism in conjunction with CKM_EXTRACT_KEY_FROM_KEY and CKM_CONCATENATE_BASE_AND_KEY. It would be quite a hack though, because you would need to use the TLS 1.0/1.1 CKM_TLS_MASTER_KEY_DERIVE to extract the version number from the encoded PMS to do the rollback attack check. (You'd check the version, then throw away the MD5/SHA1 based key and gen using the application-defined TLS 1.2 PRF/P_SHA256 function to derive a master key). But, given that CKM_SHA256_HMAC_KEY_DERIVATION is also unspecified, it is better to just define the new TLS mechanisms.
Regarding CKM_TLS12_MASTER_KEY_DERIVE vs CKM_TLS_P_SHA256_RSA_PRE_MASTER_KEY_UNWRAP: Using CKM_TLS_MASTER_KEY_DERIVE, one cannot implement the proposed handling of the RSA-encrypted PMS to prevent the various attacks against TLS's use of RSA encryption. The rules RFC5246 section-7.4.3 are: 1. Generate a string R of 46 random bytes 2. Decrypt the message to recover the plaintext M 3. If the PKCS#1 padding is not correct, or the length of message M is not exactly 48 bytes: pre_master_secret = ClientHello.client_version || R else if ClientHello.client_version <= TLS 1.0, and version number check is explicitly disabled: pre_master_secret = M else: pre_master_secret = ClientHello.client_version || M[2..47] Note that the "else if" case would never be executed in a TLS 1.2 mechanism because a TLS server wouldn't use the P_SHA256 PRF if ClientHello.client_version wasn't at least 0x0303 (TLS 1.2). Further, it is important that the premaster secret be destroyed as soon as possible, so it makes sense for the mechanism to immediately derive the master secret and return the master secret key instead of the premaster secret key. That is, a premaster secret key should never be created. Thus, a better name for the mechanism would be CKM_TLS_P_SHA256_RSA_MASTER_KEY_UNWRAP. The mechanism parameter would be a CK_SSL3_MASTER_KEY_DERIVE_PARAMS_PTR, and it would be defined to operate exactly as follows (with no steps ever skipped, in order to prevent timing attacks): 1. Generate a string R of 46 random bytes 2. Decrypt the message to recover the plaintext M 3. If the PKCS#1 padding is not correct, or the length of message M is not exactly 48 bytes: pre_master_secret = ClientHello.client_version || R else: pre_master_secret = ClientHello.client_version || M[2..47] 4. Derive the master secret from pre_master_secret and the client/server random parameters in the mechanism parameter, as in CKM_TLS_1_2_MASTER_KEY_DERIVE. 5. Return the derived master key. CKM_TLS_1_2_MASTER_KEY_DERIVE (was CKM_TLS_MASTER_KEY_DERIVE_DH for TLS 1.0/1.1) would still be needed to derive a master key from a premaster key derived from some other key exchange algorithm.
Windows's CNG SSL API is similar to the API I proposed here. http://msdn.microsoft.com/en-us/library/ff468670(v=VS.85).aspx http://msdn.microsoft.com/en-us/library/ff468670(v=VS.85).aspx
Created attachment 457908 [details] Proposal for TLS 1.2 section in PKCS #11 This is a proposal for PKCS #11 with TLS 1.2 section Mostly copy/paste of TLSv1 section. Overview: 0. PMS generation remains mostly the same comparing to TLSv1, so it's untouched 1. Key derivation functions require knowing which particular hash func should be used in HMAC, so I propose to define new structures struct CK_TLS12_PRF_PARAMS; struct CK_TLS12_MASTER_KEY_DERIVE_PARAMS; struct CK_TLS12_KEY_MAT_PARAMS; 2. Cipher suite exportability property disappeared in TLS 1.2, so it's removed it from CK_TLS12_KEY_MAT_PARAMS
Attachment #457908 - Flags: review?(rrelyea)
Comment on attachment 457908 [details] Proposal for TLS 1.2 section in PKCS #11 The general proposal looks good, but I think you need to define some constants for the PRF file. At least define the ones we need to implement;). bob
Attachment #457908 - Flags: review?(rrelyea) → review-
Actually i looks like the PRF files are defines as CKM_MECHANISM_TYPES, which I like (though I don't know if the cryptoki group would like them). So you just need to explain what types of mechanisms and what they mean (So I specify CKM_SHA1, CKM_SHA1_HMAC, or CKM_SHA1_TLS_PRF (the last one was made up). Otherwise it looks good. bob
Created attachment 462394 [details] [diff] [review] softoken_TLS12_mechanisms_v0.patch Implementation of mechanisms described in proposal posted earlier. softoken code only. This relies on TLS12_PRF() function which is available after applying patch from 480514(https://bugzilla.mozilla.org/attachment.cgi?id=456684) (not for review.)
Created attachment 470908 [details] [diff] [review] Alternate implementation strategy for PRF-based functions Here's an alternate implementation strategy that keeps the common logic for all the SSL/TLS versions together. To demonstrate why this is helpful, I left some new code in the patch that improves the implementation of the SSL 3.0 and TLS 1.0 code as well. I'd appreciate feedback on the approach. The patch consolidates the two SSL3 PRF implementations into a single implementation in FreeBL. Then, it provides a wrapper function that can be passed an ID for the SSL 3.0 PRF, the TLS 1.0/1.1 PRF, or a TLS 1.2 PRF. I think this FreeBL will make the libssl code that uses it simpler because the ID for the PRF can just be stored as part of the handshake state, eliminating some scattered clumps of version-dependent logic in the libssl bypass mode code. Note that in the proposed PKCS#11 interface, there is no way to discover which PRFs the token supports. That is why I just encoded the PRF algorithm into the mechanism ID, because there's already a query mechanism that can be used to determine which mechanisms the token supports. Also, in TLS 1.2, a cipher suite can use *any* PRF, not just P_hash. For example, it could use the NIST SP800-56A/B PRF, or the RFC5869 PRF or whatever it wants. Because of that, using just the ID of a hash algorithm is a little unclear. That is why I prefixed the names with "TLS_P_". I did not include the patch that handles the PMS version check logic. I will present that separately.
Assignee: nobody → agl
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.15
Created attachment 727323 [details] [diff] [review] Add NSS vendor-defined PKCS #11 mechanisms for TLS 1.2, by Adam Langley Adam originally attached this patch to bug 480514. I noticed it could be separated from the rest of his TLS 1.2 patch. So I checked it in first. Pushed to the NSS hg repository (NSS 3.15): https://hg.mozilla.org/projects/nss/rev/3c57b01376b9
Comment on attachment 727323 [details] [diff] [review] Add NSS vendor-defined PKCS #11 mechanisms for TLS 1.2, by Adam Langley I forgot to describe this patch. It added four NSS vendor-defined PKCS #11 mechanisms for TLS 1.2: CKM_NSS_TLS_PRF_GENERAL_SHA256 CKM_NSS_TLS_MASTER_KEY_DERIVE_SHA256 CKM_NSS_TLS_KEY_AND_MAC_DERIVE_SHA256 CKM_NSS_TLS_MASTER_KEY_DERIVE_DH_SHA256 They are the same as the standard PKCS #11 mechanisms for TLS except that they use P_SHA256 (P_hash with hash=SHA-256) as the PRF. This approach is simple. The disadvantage is that it requires four new mechanisms to use P_SHA384 as the PRF.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.