232737 - PKITS cases 4.5.1 4.5.2 4.6.15 4.6.17 4.13.19 fail: ERROR -8159

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

This bug is a replacement for bug 232479, which obscured the real problem.
I will attach my current working copy of pkits.sh, which shows this 
problem clearly.  

In test 4.5.1, a.k.a. "Valid Basic Self-Issued Old With New Test1", 
the attempt to verify the chain returns error -8159, claiming that the CRL
has an invalid format.  However, since the CRL was previously imported without
error, this error should not occur.

The full log output is:

certutil -d PKITSdb -A -t ",," -n BasicSelfIssuedNewKeyCACert -i \
../../../../PKITS_DATA/certs/BasicSelfIssuedNewKeyCACert.crt

crlutil -d PKITSdb -I -i \
../../../../PKITS_DATA/crls/BasicSelfIssuedNewKeyCACRL.crl

vfychain -d PKITSdb -u 4 \
../../../../PKITS_DATA/certs/ValidBasicSelfIssuedOldWithNewTest1EE.crt \
../../../../PKITS_DATA/certs/BasicSelfIssuedNewKeyOldWithNewCACert.crt \
../../../../PKITS_DATA/certs/BasicSelfIssuedNewKeyCACert.crt

Chain is bad, -8159 = New CRL has an invalid format.
PROBLEM WITH THE CERT CHAIN:
CERT 0. CN=Valid Basic Self-Issued Old With New EE Certificate Test1,O=Test Cert
ificates,C=US :
  ERROR -8159: New CRL has an invalid format.
CERT 1. CN=Basic Self-Issued New Key CA,O=Test Certificates,C=US [Certificate Au
thority]:
  ERROR -8159: New CRL has an invalid format.
pkits.sh ERROR: Valid Basic Self-Issued Old With New Test1 failed 3

The stack at the place where the error is set is:

PORT_SetError(int -8159) line 178
DPCache_Lookup(0x0050c4dc, 0x00504ccc, 0x0012fd84) line 1398 + 10 bytes
CERT_CheckCRL(0x00504c48, 0x00506c78, 0x00000000, 0x0003d23554b2ed9c, 
              0x00000000) line 1873 + 22 bytes
SEC_CheckCRL(0x004de5b8, 0x00504c48, 0x00506c78, 0x0003d23554b2ed9c, 0x00000000)
             line 245 + 27 bytes
cert_VerifyCertChain(0x004de5b8, 0x00504c48, 1, 0x0012fed8, 4, 
             0x0003d23554b2ed9c, 0x00000000, 0x00000000, 0x0012fed0) 
             line 807 + 29 bytes
CERT_VerifyCertificate(0x004de5b8, 0x00504c48, 1, 0x10, 0x0003d23554b2ed9c, 
             0x00000000, 0x00000000, 0x00000000) line 1371 + 45 bytes
main(int 0x00000008, char * * 0x004d4280,

The line of code that sets this error is

SECStatus DPCache_Lookup(CRLDPCache* cache, SECItem* sn, CERTCrlEntry** returned)
{
    CERTSignedCrl* crl = NULL;
    CERTCrlEntry* acrlEntry = NULL;
    if (!cache || !sn) {
        /* no cache or SN to look up, this is bad */
        PORT_SetError(SEC_ERROR_INVALID_ARGS);
        return SECFailure;
    }
    if (0 != cache->invalid) {
        /* the cache contains a bad CRL, consider all certs revoked
           as a security measure */
-->     PORT_SetError(SEC_ERROR_CRL_INVALID);
        return SECFailure;
    }

I'm asking Julien to look at this, because he's our CRL expert.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

*** Bug 232479 has been marked as a duplicate of this bug. ***

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: My current working pkits.sh test script

This version of pkits.sh shows all the commands being run and their 
outputs.  It also breaks the log output up into easy to identify and 
read sections.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Because this is a CRL failure that renders all certs invalid,
I think this deserves P1 for 3.9.1.  

Comment 4

[Julien Pierre]

Nelson,

The primary reason why the cache would be set to the invalid state is if we did
not have the issuer certificate needed to verify the CRL.

This could happen if for example the CRL was imported without issuer
verification (using the -B flag on crlutil to bypass checks), or if the issuer
certificate was deleted after the import of the CRL.

Another possible reason is if the CRL has invalid DER, or invalid extensions.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Jullien, agreed, but none of those situations seems to be what's going on here.

From the test script output, we see that
a) certutil imports the CA cert wihtout error or complaint.
b) crlutil imports the CRL without error or complaint (and without -B),
The attemtp to validate a chain that uses that CRL gets this error message.

Comment 6

[Julien Pierre]

Nelson,

I looked into this.

We have the following situation :
- the end-entity cert (EE) has an authorityKeyID of K1
- there are two CA certs CA1 and CA2 with the same subject, but different
subjectKeyIDs K1 and K2
- CA2 is "newer" than CA1 .
- the CRL has an authorityKeyID of K2
- during the crlutil import, we look up the issuer for the CRL by subject name.
We find CA2, because it is the newest of the two CA certs. The CRL verifies
against CA2, so we import.
- during the vfychain, we construct a cert chain starting with EE cert. The
issuer cert is CA1 .
- we then call SEC_CheckCRL to verify the revocation status of EE against its
issuer CA1
- the CRL cache finds a CRL for CA1 after looking it up only by subject
- the CRL cache tries to verify  the CRL against CA1, but that fails because the
key does not match

This is a manifestation of a known bug , bugzilla 217387, "AuthorityKeyID is not
supported for CRLs". The steps to fix this are described in that defect, but
they are quite involved.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Changing subject to show which test cases are affected by this.

Comment 8

[Slavomir Katuscak]

Causes also failures of tests 4.4.19 - 4.4.21 (new ones).

Comment 9

[Slavomir Katuscak]

Is this bug fixed already (maybe as consequence of fix of another bug) ? When I run those tests on my machine (latest trunk version), I didn't get ERROR -8159 in any test.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

No, the underlying root cause has not been fixed.  
The failure is probabilistic, depending on which cert is found first 
in a list.  perhaps something has caused that order to change, but the 
root cause (ignoring subject key IDs) remains.

Comment 11

[Slavomir Katuscak]

This bug is probably only consequence of some other bug.

At the time when it was detected (also for tests 4.4.19 and 4.4.21) testing logic for every test was:
1. Import certs + CRLs to DB.
2. Run test.
3. Delete certs + CRLs from DB.

With this scenario can happen that after some tests certs/CRLs are not deletable and DB is not correctly cleaned for next test.

Current logic is:
1. Import certs + CRLs to DB.
2. Run test.
3. Restore DB to previous state (using backup of DB directory).

When DB is restored instead of just deleting certs/CRLs, this bug is no more detected.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Slavo, 
doesn't your comment 11 imply that we have a bug in the cert/CRL deletion code?

Comment 13

[Slavomir Katuscak]

Not really, I just localized the tests that were not correctly cleaned and it's just tests 4.4.19 - 4.4.21 where 2 certs have the same name (that doesn't match file name). It is only script problem how to clean up DB and this is already solved. However it doesn't mean that there wasn't another problem related to this in past.

I tried to run old version of pkits.sh (1.19) using old scenario of DB cleaning and ERROR -8159 wasn't seen even with old version of this script, this means that something in code was changed and we don't see this bug any more.

Comment 14

[Slavomir Katuscak]

I did some more experiments to find why this bug is no more seen. I try to run tests on older version of NSS to localize which patch caused it.

Finally I found that I was able to detect this error up to build from 2009-04-18 (CET), on build from 2009-04-19 this bug is no more detected. I looked on CVS changes and I found out that at evening 2009-04-17 PDT (at my timezone it was already 2009-04-18) there were checked in more patches related to bug 321755. These patches are related to CRLDP and after they are applied, ERROR -8159 is no more detected.

Comment 15

[Kai Engert [:KaiE:]]

Bugs that are currently assigned to Julien => assigning to nobody.
Search for 20100628-kaie-jp