Closed
Bug 419794
Opened 15 years ago
Closed 15 years ago
work out key API for nsICryptoHMAC
Categories
(Core :: Security: PSM, defect, P2)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla1.9beta5
People
(Reporter: dcamp, Assigned: mayhemer)
References
Details
Attachments
(1 file, 2 obsolete files)
20.42 KB,
patch
|
dveditz
:
superreview+
|
Details | Diff | Splinter Review |
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.
Flags: blocking1.9?
![]() |
Assignee | |
Comment 1•15 years ago
|
||
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.
Attachment #306014 -
Flags: review?(rrelyea)
Attachment #306014 -
Flags: review?(dveditz)
![]() |
Assignee | |
Updated•15 years ago
|
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•15 years ago
|
||
Opps, sorry, missed to add the new test.
Attachment #306014 -
Attachment is obsolete: true
Attachment #306017 -
Flags: review?(rrelyea)
Attachment #306017 -
Flags: review?(dveditz)
Attachment #306014 -
Flags: review?(rrelyea)
Attachment #306014 -
Flags: review?(dveditz)
Comment 3•15 years ago
|
||
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+
Updated•15 years ago
|
Flags: tracking1.9? → blocking1.9?
![]() |
Assignee | |
Comment 4•15 years ago
|
||
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?
Attachment #306017 -
Attachment is obsolete: true
Attachment #306284 -
Flags: review?(dveditz)
Attachment #306017 -
Flags: review?(dveditz)
Updated•15 years ago
|
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Updated•15 years ago
|
Version: unspecified → Trunk
![]() |
Assignee | |
Updated•15 years ago
|
Attachment #306284 -
Flags: review?(dveditz) → superreview?(dveditz)
Comment 5•15 years ago
|
||
Comment on attachment 306284 [details] [diff] [review] API use nsIKeyObject + fix nsKeyModule + nsICryptoHMAC usage fix sr=dveditz
Attachment #306284 -
Flags: superreview?(dveditz) → superreview+
![]() |
Assignee | |
Updated•15 years ago
|
Keywords: checkin-needed
Comment 6•15 years ago
|
||
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: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
Updated•15 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•