softoken: unified MAC implementation
Categories
(NSS :: Libraries, enhancement, P1)
Tracking
(Not tracked)
People
(Reporter: alexander.m.scheel, Assigned: alexander.m.scheel)
References
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0
Steps to reproduce:
As I hinted in a previous bug I'd like to submit changes necessary for SCP 03 to be implemented via NSS. When I started that bug, I didn't have a copy of the spec, hence some confusion. Now I do!
There's two parts to working SCP03 support in NSS:
- CMAC (done) -- necessary for MACing as hinted, but also for the KDF.
- the SCP03 KDF, a NIST SP800-108 KBKDF variant.
I've been working on the latter in my fork on GitHub. That's not ready yet, but in the process I realized there's a bit of code that needs "a" MAC and could support HMAC, CMAC, or XCBC MAC.
That's mostly:
- The HMACs/CMACs themselves
- Softoken's IKE implementation
- The new KBKDF implementation (sans XCBC).
- I'm sure others I've forgotten about.
So, I'd like to get this started with a cleanup attempt: unify HMAC+CMAC into a mechanism-switched interface. Call it with the mechanism type of the MAC (not the hash!) and get something usable, with common semantics, out.
This opens the doors for other improvements in the future:
- Return code could be checked when updating the MAC, if desired. This requires updating
sftk_doMACInit
to condition around whether or not the function could be called and its return code checked. I'm not sure if it is worth it. I've left it alone for now. - Bringing XCBC into the fold and updating softoken IKE to use the common interface.
I don't have much interest now in working on that (I'd prefer to get the KBKDF cleaned up so I can submit it), but I'm mentioning it in case someone else wishes to do that. I'd consider that out of scope currently.
Actual results:
More work to support both HMAC+CMAC.
Expected results:
Unified MAC interface.
Assignee | ||
Comment 1•3 years ago
|
||
Simplify HMAC and CMAC construction and usage behind a common PKCS#11
interface. This ensures that return values can be checked by the caller
for both MACs if desired and simplifies the implementation of KDFs and
PRFs capable of using both.
Updated•3 years ago
|
Comment 2•3 years ago
|
||
Comment 3•3 years ago
|
||
Coverity has found a defect in https://hg.mozilla.org/projects/nss/rev/3147585149f0e907b39bda15e804ec8fb33a0138
*** CID 1455952: Null pointer dereferences (FORWARD_NULL)
/lib/softoken/sftkhmac.c: 235 in sftk_MAC_Init()
229
230 ret = sftk_MAC_InitRaw(ctx, mech,
231 (const unsigned char *)keyval->attrib.pValue,
232 keyval->attrib.ulValueLen, isFIPS);
233
234 done:
>>> CID 1455952: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "keyval" to "sftk_FreeAttribute", which dereferences it.
235 sftk_FreeAttribute(keyval);
236 return ret;
237 }
238
239 CK_RV
240 sftk_MAC_InitRaw(sftk_MACCtx *ctx, CK_MECHANISM_TYPE mech, const unsigned char *key, unsigned int key_len, PRBool isFIPS)
It appears to be a correct analysis. We'll need to fix this this cycle. Can you make a fixup, Alexander? Just guarding with a if (keyval)
should be fine.
Comment 4•3 years ago
|
||
Nevermind, attaching a patch, marking you as a reviewer.
Comment 5•3 years ago
|
||
Comment 6•3 years ago
|
||
Description
•