The NIST NISCC tests contain some test cases that (I am told) have self-issued certs in the middle of the cert chain. NSS fails all these tests. One such test is Section 4.13: Name Constraints, case 19. Others are documented in other mozilla NSS bugs about NIST.
The reference to NISCC above is mistaken. It should have read PKITS.
The command for the Name Constraints test 19 is apparently vfychain -v -d d:/tmp/pkits -u 4 \ ValidDNnameConstraintsTest19EE.crt \ nameConstraintsDN1SelfIssuedCACert.crt \ nameConstraintsDN1CACert.crt \ TrustAnchorRootCertificate.crt NSS fails this test, claiming PROBLEM WITH THE CERT CHAIN: CERT 2. CN=nameConstraints DN1 CA,O=Test Certificates,C=US [Certificate Authorit y]: ERROR -8080: The Certifying Authority for this certificate is not permitted to issue a certificate with this name. The test is supposed to declare the chain valid.
Adding reviewers to CC list. I have found the problem for test case 19, and am about to attach a patch for that test case. Bishakha, After this patch and the fix for bug 221644 are checked in, all the PKITS tests must be redone.
Created attachment 139228 [details] [diff] [review] patch v1 - both unified and context diffs I included both the unified and context diffs. unified diffs are considered the norm for mozilla bugs, but in this case, I think it is much easier to read the context diff. The context diff shows that the change is really just adding one test, and indenting a bunch of code.
Comment on attachment 139228 [details] [diff] [review] patch v1 - both unified and context diffs Nelson, "cvs diff -uw" is a good way to show that the change is just adding one test and indenting a bunch of code. (The indentation is screwed up by the -w flag but it clears shows what the new test is.)
Created attachment 139247 [details] [diff] [review] patch v2 - from diff -uw This patch combines the fix for this bug, and the fix for bug 221644. It incorporates review feedback from Wan-Teh, and it further clarifies and simplifies the self-issued test.
taking bug. Adding reviewers to CC list.
Comment on attachment 139247 [details] [diff] [review] patch v2 - from diff -uw Wan-Teh, please review
Now that the pkits test script has been fixed, we see that the above patch fixes some but not all of these issues. With that patch in place, the following self-issued cert test cases still fail: Invalid Basic Self-Issued Old With New Test2 Invalid Basic Self-Issued New With Old Test5 Invalid Basic Self-Issued CRL Signing Key Test7 These are all negative test cases that previously were reported as succeeding.
Regarding comment 9, the reason that tests 2, 5 and 7 of section 4.5 fail is that the pkits.sh script is not loading the requisite CRLs. The cert chain verifications done in these tests are supposed to fail because a CA cert has been revoked, which can only be detected if the CRL is loaded. So, the real bug here is that the CRLs are not being loaded. I will file a separate bug about that, and change this bug so that it only covers test case 4.13.19
Comment on attachment 139247 [details] [diff] [review] patch v2 - from diff -uw Removing review request from this obsolete patch. New patch forthcoming.
Created attachment 139378 [details] [diff] [review] patch v3 - diff -uw, supplemental to patch v2 for bug 221644
Comment on attachment 139378 [details] [diff] [review] patch v3 - diff -uw, supplemental to patch v2 for bug 221644 This patch is to be applied after (on top of) patch v2 from bug 221644. This patch modifies code added in that patch. Wan-Teh, please review.
Comment on attachment 139228 [details] [diff] [review] patch v1 - both unified and context diffs remove old review request for this obsolete patch
Created attachment 139702 [details] test script with CRLs (for bug 231230 ) here is the revised script, with CRLs loaded, for the previous 6 sections. With the CRLS loaded, the particular tests 2,5 and 7 pass. Test 4.13.38 still fails. Some notes about the loading of CRLs for these tests. After discussing with both Nelson and Julien, I chose to import the particular cert associated with the CRL also into the database. I could have used the crlutil -B option to bypass the check and inport the CRL anyway, but upon discussion with Julien, I think it is more correct to the NIST tests not to do this. Hence, when a particular test is over, I delete both the CRL and associated cert from the database. Another note about the loading of CRLs is that, one should run each NIST test section in complete. This is because if test1 uses testCRL1, and test2 also uses it, I do not delete testCRL1 from the database after the completion of test1. However, if test3 does not use testCRL1, then it *is* deleted after test2 is over. The newer sections of the complete test suite (alongwith the modified previous sections) to be attached to the original bug (177398) shortly.
Comment on attachment 139378 [details] [diff] [review] patch v3 - diff -uw, supplemental to patch v2 for bug 221644 r=wtc. Suggest two comment changes. 1. Explain why subjectIsSelfIssued has the value PR_FALSE in the first iteration of the for loop. 2. In this comment: >+ /* This cert will be the subject cert in the next loop. change "This cert" to "The issuer cert".
/cvsroot/mozilla/security/nss/lib/certhigh/certvfy.c,v <-- certvfy.c new revision: 1.39; previous revision: 1.38 Tbanks for the review.
All name constarints tests pass now, pulled cvd this morning. Hence, marking this verified. I happened to have this bug open, I'm going to do likewise for the other NIST bugs as well.
Comment on attachment 139702 [details] test script with CRLs (for bug 231230 ) (I think this attachment was intended for bug 231230 ) This latest version of pkits.sh puts all the certs into the cert db, and then only supplies the leaf (EE) cert on the vfychain command line. That does not simulate the actual behavior as used in client programs. Please put ONLY the trusted CA certs (the ones with trust flags), and those few others that are needed to import CLRs, into the certdb, and supply all other CA certs on the vfychain command line. That's important for valid test results.