221644 - NSS fails NIST path length constraint tests

Status: VERIFIED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

PKITS tests, section 4.3

    NSS fails 4 test cases:  13, 14, 15, 17
    NSS reports "Cert path length constraint is invalid" for all.
    NIST says all 4 tests are valid chains.

This is a crucial aspect of chain validation

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Bishakha,  Please confirm that these are the proper commands for test cases
13 and 14, and please add a comment here with the proper commands for test
cases 15 and 17.  Thanks.

vfychain -d d:/tmp/pkits ValidpathLenConstraintTest13EE.crt \
  pathLenConstraint6subsubsubCA41XCert.crt \
  pathLenConstraint6subsubCA41Cert.crt \
  pathLenConstraint6subCA4Cert.crt \
  pathLenConstraint6CACert.crt \
  TrustAnchorRootCertificate.crt

vfychain -d d:/tmp/pkits ValidpathLenConstraintTest14EE.crt \
  pathLenConstraint6subsubsubCA41XCert.crt \
  pathLenConstraint6subsubCA41Cert.crt \
  pathLenConstraint6subCA4Cert.crt \
  pathLenConstraint6CACert.crt \
  TrustAnchorRootCertificate.crt

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

With thit patch, the test commands given above for test cases 13 and 14 pass.

This patch accomplshes the following:
1. detects and rejects negative path lengths in basic constraints extensions.
2. corrects the path length processing (except that there are still issues
with self-issued intermediate CA certs, which is the subject of another bug).

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Adding potential reviewers to cc list.  
I will wait and ask for review after this patch has been tested against 
test cases 15 and 17.  
This bug is waiting for the instructions for those test cases to be added as
comments.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

The command for case 15 is apparently

vfychain -d d:/tmp/pkits -u 4 \
  ValidSelfIssuedpathLenConstraintTest15EE.crt \
  pathLenConstraint0SelfIssuedCACert.crt \
  pathLenConstraint0CACert.crt \
  TrustAnchorRootCertificate.crt

The command for case 17 is apparently 

vfychain -v -d d:/tmp/pkits -u 4  \
  ValidSelfIssuedpathLenConstraintTest17EE.crt \
  pathLenConstraint1SelfIssuedsubCACert.crt \
  pathLenConstraint1subCACert.crt \
  pathLenConstraint1SelfIssuedCACert.crt \
  pathLenConstraint1CACert.crt \
  TrustAnchorRootCertificate.crt

These tests involve self-issued subordinate CA certs, and NSS fails them.
However, the failure is not unrecognized issuer or untrusted issuer, 
but rather is invalid path length, so I will research this some more.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2

With this patch all the above test cases pass, including the ones containing
self-issued intermediate CA certs!

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 139175 [details] [diff] [review]
patch v2

Julien, please review. Thanks.

Comment 8

[Julien Pierre]

Comment on attachment 139175 [details] [diff] [review]
patch v2

Nelson,

The patch looks good. Based on our discussion of this problem, there may be
some other serious test failures that we should have seen and that the patch
fixes.

Comment 9

[Bishakha Banerjee]

Nelson, I did not use the -u option while doing the tests, used vfychain -d <DB>
cert1.crt cert2.crt cert3.crt..
Results were as notified to you.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Bishakha,

The vfychain command defaults to SSL Client usage.  But many of the PKITS
test certs are explicitly not approved for that usage, and so they will
experience errors.  Please use -u 4 in all the PKITS tests.  Thanks.
BTW, -u 4 means usage of "email signature" (really digital signature of 
any kind).

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Checked in this part of the fix.  

/cvsroot/mozilla/security/nss/lib/certdb/xbsconst.c,v  <--  xbsconst.c
new revision: 1.4; previous revision: 1.3

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Checked in the rest of the fix.

/cvsroot/mozilla/security/nss/lib/certhigh/certvfy.c,v  <--  certvfy.c
new revision: 1.38; previous revision: 1.37