The 2DES encryption algorithm is similar to 3DES. Both perform and encryption step, then a decryption step, followed by another encryption step (EDE). However 2DES uses the same key for both of the encryption (E) steps. NSS does not handle 2DES keys correctly. Rather than reusing the first subkey, it references key material past the end of the key data buffer. As a result the incorrect result is calculated. Workaround: if keys are imported with PK11_ImportSymKey it is possible to import the 2DES key as a 3DES key by expanding the key into a full 3DES key (copying the first subkey into the third position). Workaround: the subkeys can also be imported into normal DES keys and the 2DES encryption (EDE steps) performed by the application. I don't think there is any workaround for 2DES keys generated within the token. (not imported)
This appears to be a reference to the softoken implementations of CKM_DES2_KEY_GEN, CKM_PBE_SHA1_DES2_EDE_CBC, and the validation of CKK_DES2 keys. freebl only supports DES and 3-key triple-DES. 2-key triple-DES can be implemented by using the same key material for the first and third keys in 3-key triple-DES. Therefore, all support for 2-key triple-DES must be done in softoken. Terry, how does a mozilla or NSS user encounter this problem? Does CMS/PKCS7 use this form of encryption? or ? I'm marking this P2, but if it's accessing memory past the end of an allocated block, it should be P1. I'll also take this bug.
There are not any current applications (such as Mozilla) that use 2-key triple DES (2DES). However a future application might need to use it. Currently NSS will access memory beyond the allocated block in the following case. 1) Generate a 2DES key on the token (using the PKCS11 key gen mech) 2) Use it to perform encryption or decryption A possible solution is to expand all 2DES keys into full 3-key buffers. However, other functions such as wrap would need to be reviewed to make sure the correct data (the smaller 2DES key) is used to produce the output.
This raises an interesting question. Are the contents of wrapped keys defined anywhere? If not, do we have any reason to expect interoperability between PKCS11 devices based on wrapped keys? Based on Terry's statement above, I think this is a candidate for P1, but I don't what what is the correct fix, because I don't have any definition of a wrapped DES2 key.
Created attachment 134003 [details] [diff] [review] patch v1 This patch performs 2 main changes: 1. It checks the length of DES keys in handleObject, and 2. It transforms DES2 keys into DES3 keys in CryptInit. I have not yet tested this patch (working from home).
Comment on attachment 134003 [details] [diff] [review] patch v1 Please read bug comments before reviewing patch.
Comment on attachment 134003 [details] [diff] [review] patch v1 This is definately the correct fix, especially as, Nelson noticed, NSS already did key transformations before the decrypt for CDMF. Some caveats: The stricter check in validateSecretKeys is correct, we need to make sure that we or our apps don't depend on the more relaxed behavior (I don't anticipate any problems, since the stricter checks only apply to DES and bad things will happen if they are wrong anyway). I'm not sure what the intent of the patch to NSC_Unwrap is. For DES keys key_length should always be '0' since DES has known fixed length keys. The current code automatically truncates the key to the size of the incoming data. This is probably an error condition (truncating the incoming key to the value in the template, however, is a reasonable thing to do). If this code is meant to handle the case where we get a DES2 key expanded to the full EDE size, then it will fail to do so because keysize will be 0 (not 16) and the new check in validateSecretKey will reject the key with CKR_KEY_SIZE_RANGE. The only iffy thing is the key_length check in Unwrap. Technically we probably should be verifying that the keylength matches the mechanism type and failing if it doesn't. Since the code doesn't do that today, we
/cvsroot/mozilla/security/nss/lib/softoken/softoken.h,v <-- softoken.h new revision: 1.4; previous revision: 1.3 /cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v <-- pkcs11c.c new revision: 1.53; previous revision: 1.52 /cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v <-- pkcs11.c new revision: 1.83; previous revision: 1.82 Terry, please verify this fix with Monday's trunk NSS build.
Terry reports that there is an additional bug in pk11wrap which also must be fixed. Function PK11_GetKeyType takes a key length argument, which is only used for 3DES mechanisms. The function expects the length to be in bits, but the values passed to it are in bytes (at least sometimes). Suggested patch: Index: pk11wrap/pk11slot.c =================================================================== RCS file: /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11slot.c,v retrieving revision 1.71 diff -u -r1.71 pk11slot.c --- pk11wrap/pk11slot.c 12 Sep 2003 20:01:56 -0000 1.71 +++ pk11wrap/pk11slot.c 29 Oct 2003 22:04:03 -0000 @@ -2901,7 +2901,7 @@ case CKM_DES3_MAC: case CKM_DES3_MAC_GENERAL: case CKM_DES3_CBC_PAD: - return (len == 128) ? CKK_DES2 : CKK_DES3; + return (len == 16) ? CKK_DES2 : CKK_DES3; case CKM_DES2_KEY_GEN: case CKM_PBE_SHA1_DES2_EDE_CBC: return CKK_DES2;
Terry, please review my patch in comment 8 above and add your review comments here, (e.g. r= :-)
Thanks Bob. Marking fixed again. /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11slot.c,v <-- pk11slot.c new revision: 1.72; previous revision: 1.71
But wait! There's more! In order to be able to generate DES2 keys, PK11_GetKeyGen must return CKM_DES2_KEY_GEN. This patch enables that. Bob or Terry, please review. diff -u -r1.72 pk11slot.c --- lib/pk11wrap/pk11slot.c 30 Oct 2003 22:31:09 -0000 1.72 +++ lib/pk11wrap/pk11slot.c 31 Oct 2003 00:48:01 -0000 @@ -3077,6 +3077,8 @@ case CKM_DES3_CBC_PAD: case CKM_DES3_KEY_GEN: return CKM_DES3_KEY_GEN; + case CKM_DES2_KEY_GEN: + return CKM_DES2_KEY_GEN; case CKM_CDMF_ECB: case CKM_CDMF_CBC: case CKM_CDMF_MAC:
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11slot.c,v <-- pk11slot.c new revision: 1.73; previous revision: 1.72