Closed Bug 419794 Opened 14 years ago Closed 13 years ago
work out key API for ns
From 415799 comment #11: The PK11_ImportSymKey can and will fail on most FIPS devices (including our own FIPS module). The problem is FIPS modules do not allow import or export of plain-text keys. This means any crypto interface that uses plain-text keys will not support only of these modes/devices. Exporting such an API can get your own customers in trouble (just like my allowing it is potentially getting you in trouble). Ideally you need to design an API where keys are exported as real objects. There may actually be such an object in the mozilla crypto world for symmetric operations. Anyway you should consider modifying this api to take a key, then you can set the key to some plaintext key (still won't work in fips mode), but you have an API that could in the future move to something that can handle real tokens.
I had to fix nsKeyObjectFactory because the key produced was assigned wrong mechanism and operation. Creation of HMAC crypto context failed when key was created with CKA_ENCRYPT op and RC4 slot. Disputable is name of new enum 'HMAC' in nsIKeyObject. Probably better way is to have separate constants for HMAC_SHA1, HMAC_MD5 anyway, enhanced test proofs that using CKA_SHA_1_HMAC when creating the key works for all algorithms. API is also simpler to use. I also added test against test vectors.
Opps, sorry, missed to add the new test.
Comment on attachment 306017 [details] [diff] [review] API moved to use nsIKeyObject + fix in nsKeyObjectFactory r+ Thanks for the quick work! RE different HMAC key types. Currently PKCS #11 accepts CKK_GENERIC_SECRET for all HMACS, but the pkcs #11 group is just now finalizing defining specific key types for each HMAC. Upshot. Either way will work (one HMAC key type or one per HMAC), Each have advantages and disadvantages. (If you specify the HMAC specifically today, NSS will map all the keys to CKK_GENERIC_SECRET). bob
Attachment #306017 - Flags: review?(rrelyea) → review+
1. I added usage of CKM_GENERIC_SECRET_KEY_GEN for HMAC keys in nsKeyObjectFactory as Bob suggested in previous comment. 2. I changed ISUPPORTS implementation of nsKeyObjectFactory to be thread safe, because test failed on assertion about not thread-safety of that object. nsKeyObjectFactory uses PK11 functions that are (AFAIK) thread safe. This step is disputable because it might be solved a different way - nsUrlClassifierDBServiceWorker should get the service on main thread and also release it on it. But this might be very complicated and the service is still used on a different thread. David, your opinion?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Attachment #306284 - Flags: review?(dveditz) → superreview?(dveditz)
Comment on attachment 306284 [details] [diff] [review] API use nsIKeyObject + fix nsKeyModule + nsICryptoHMAC usage fix sr=dveditz
Attachment #306284 - Flags: superreview?(dveditz) → superreview+
Does nsIKeyModule.idl need its IID revved due to the addition of |const short HMAC|? Checking in security/manager/ssl/public/nsIKeyModule.idl; /cvsroot/mozilla/security/manager/ssl/public/nsIKeyModule.idl,v <-- nsIKeyModule.idl new revision: 1.3; previous revision: 1.2 done Checking in security/manager/ssl/src/nsKeyModule.cpp; /cvsroot/mozilla/security/manager/ssl/src/nsKeyModule.cpp,v <-- nsKeyModule.cpp new revision: 1.4; previous revision: 1.3 done Checking in security/manager/ssl/src/nsNSSComponent.cpp; /cvsroot/mozilla/security/manager/ssl/src/nsNSSComponent.cpp,v <-- nsNSSComponent.cpp new revision: 1.161; previous revision: 1.160 done Checking in security/manager/ssl/tests/test_hmac.js; /cvsroot/mozilla/security/manager/ssl/tests/test_hmac.js,v <-- test_hmac.js new revision: 1.2; previous revision: 1.1 done Checking in netwerk/base/public/nsICryptoHMAC.idl; /cvsroot/mozilla/netwerk/base/public/nsICryptoHMAC.idl,v <-- nsICryptoHMAC.idl new revision: 1.2; previous revision: 1.1 done Checking in toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp; /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v <-- nsUrlClassifierDBService.cpp new revision: 1.70; previous revision: 1.69 done Checking in toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp; /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp,v <-- nsUrlClassifierHashCompleter.cpp new revision: 1.6; previous revision: 1.5 done Checking in toolkit/components/url-classifier/tests/unit/test_streamupdater.js; /cvsroot/mozilla/toolkit/components/url-classifier/tests/unit/test_streamupdater.js,v <-- test_streamupdater.js new revision: 1.7; previous revision: 1.6 done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
You need to log in before you can comment on or make changes to this bug.