564334 - The lookup key for the CRL cache should include not only the issuer's name but also the issuer's key ID

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Wan-Teh Chang]

[Nelson, I can't find the existing bug for this problem.  The closest
thing I can find is bug 233118, but it's too long for me to parse.]

There are two CAs with the name
"CN=Network Solutions Certificate Authority,O=Network Solutions L.L.C.,C=US".

One is an intermediate CA, subordinate to UTN-USERFirst-Hardware.  The key ID
is 3c:41:e2:8f:08:08:a9:4c:25:89:8d:6d:c5:38:d0:fc:85:8c:62:17.

The other is a root CA, but is also cross-certified by UTN-USERFirst-Hardware.
The key ID is 21:30:c9:fb:00:d7:4e:98:da:87:aa:2a:d0:a7:2e:b1:40:31:a7:4c.

I found that a CRL issued by the root CA is incorrectly considered to have
an invalid signature because our CRL cache code uses the intermediate CA
cert to verify the signature.

Although I haven't tracked this down completely, I have strong evidences
that show this is because the lookup key for the CRL cache is simply the
issuer's name.  To differentiate these two CAs with the same name, the
lookup key needs to includes the key ID as well.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Isn't this bug 217387 ?

Comment 2

[Wan-Teh Chang]

Nelson, thanks for the bug number.  I'll see if that's the bug
I tried to find.

This bug is nontrivial to fix.  I estimate it needs at least
2-3 days of serious coding.  Here is one place that needs changing:

pkix_crlchecker.c:

409     PKIX_CHECK(
410         PKIX_PL_Cert_GetIssuer(cert, &issuerName, plContext),
411         PKIX_CERTGETISSUERFAILED);
412     PKIX_CHECK(
413         PKIX_CRLSelector_Create(issuer, dpList, date, &crlSelector, plContext),
414         PKIX_CRLCHECKERSETSELECTORFAILED);
415     /* Fetch crl and store in a local cert store */
416     for (crlStoreIndex = 0;crlStoreIndex < numCrlStores;crlStoreIndex++) {
417         PKIX_CertStore_CRLCallback getCrlsFn;
418 
419         PKIX_CHECK(
420             PKIX_List_GetItem(state->certStores, crlStoreIndex,
421                               (PKIX_PL_Object **)&certStore,
422                               plContext),
423             PKIX_LISTGETITEMFAILED);
424         
425         PKIX_CHECK(
426             PKIX_CertStore_GetCRLCallback(certStore, &getCrlsFn,
427                                           plContext),
428             PKIX_CERTSTOREGETCRLCALLBACKFAILED);
429         
430         PKIX_CHECK(
431             (*getCrlsFn)(certStore, crlSelector, &nbioContext,
432                       &crlList, plContext),
433             PKIX_GETCRLSFAILED);
434 
435         PKIX_CHECK(
436             (*storeImportCrlFn)(localStore, issuerName, crlList, plContext),
437             PKIX_CERTSTOREFAILTOIMPORTCRLLIST);

The storeImportCrlFn call on line 436 needs to be passed the
issuer's key ID as well.

The CRLs issued by the "Network Solutions Certificate Authority"
intermediate CA do not have the authority key identifier CRL
extension, so we will need to get the issuer key ID from the
issuer certificate.

Comment 3

[Wan-Teh Chang]

Another note to myself: AcquireDPCache is an important function
when exploring the code for this bug:
http://mxr.mozilla.org/security/ident?i=AcquireDPCache

We need to add a subjectKeyID argument to AcquireDPCache, so
we need to inspect all the callers of AcquireDPCache and see
which ones must provide a non-NULL subjectKeyID to AcquireDPCache.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last months and this bug has priority 'P2'.
:beurdouche, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 5

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.