Comment 34 for bug 312536

A small plea (because I think we made a mistake in filing this bug)

to everyone else: this bug is filed against NSS, and I think the NSS team has made it clear that they're talking about enabling library functionality (which will be the majority of my comment).

Anyone interested in Firefox policy should please ignore this bug and campaign in a newsgroup or a Core/Firefox component. This bug is about NSS which is focussed on the underlying API.

It's unfortunate that the bug wasn't originally written to limit its scope to something like "enable applications to ask NSS to stop honoring certain classes of digital certificates", but that's hindsight.

---

the api looks technically supports the requirements.

but my first read of it totally missed the way you were using the MASK. It's clever and does what's necessary but I really don't like the method name because i was totally mislead.

Below please find what I ended up writing because I couldn't properly read your API:

I'm slightly worried.

if the NSS policy is later: ~FOO_BIT
and we hard code that does SetPolicy( ... ~BAR_BIT ...)
then we've stepped on ~FOO_BIT. This is likely to happen.
So either all example code needs to be of this form:

...
#define NSS_ALG_DEFAULT_POLICY 0xffffffff

SECStatus rv;
PRUint32 policy = NSS_ALG_DEFAULT_POLICY;
rv = NSS_GetAlgorithmPolicy(SEC_OID_MD5, &policy);
if (...FAILED(rv)) ...

policy ^= NSS_ALG_USABLE_IN_CERT_SIGNATURE;
rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, policy, 0);
if (...FAILED(rv)) ...

or you should seriously consider a different API.

There are two possibilities.

One:

#define NSS_ENABLE_ALGORITHM PRBIT(31)
#define NSS_DISABLE_ALGORITHM 0 * NSS_ENABLE_ALGORITHM

#define NSS_ALG_USABLE_IN_CERT_SIGNATURE PRBIT(0)

extern SECStatus
NSS_SetAlgorithmPolicy(SECOidTag tag, PRUint32 policyChanges);

extern SECStatus
NSS_GetAlgorithmPolicy(SECOidTag tag, PRUint32 *pValue);

to disable an algorithm:

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_DISABLE_ALGORITHM | NSS_ALG_USABLE_IN_CERT_SIGNATURE);

to enable an algorithm:

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ENABLE_ALGORITHM | NSS_ALG_USABLE_IN_CERT_SIGNATURE);

Yes, I'm aware my api wastes one bit (it's also obviously possible to have distinct enable/disable methods).

---
yes I eventually understood what the mask was doing.

But it's complicated, with your API, to enable something one has to do:
rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ALG_USABLE_IN_CERT_SIGNATURE, NSS_ALG_USABLE_IN_CERT_SIGNATURE);

or

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ALG_USABLE_IN_CERT_SIGNATURE, 0xffffffff);

neither of these make me happy.

--

While I'm commenting, this may sound stupid, but are we really going to be miserly and try to use 1 bit to represent both CRLs and Certs?

iiuc CRLs merely contain lists of serial numbers. While I'd hope that the CAs would be using SHA1 (or better [and iiuc better isn't supported]), and while I can understand that a similar risk of collision for an MD5 signature over a random sequence of serial numbers exists, as I haven't heard much information about the current state of CRLs, I think at least tentatively it'd be nice from an API perspective for it to be its own distinct bit, even if all consumers use NSS_ALG_USABLE_IN_CERT_SIGNATURE | NSS_ALG_USABLE_IN_CRL_SIGNATURE.