231223 - NSS fails NIST PKITS name constraints test case 38

Status: VERIFIED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

NSS (trunk) is failing two PKITS name constraints test cases.
I do not know if this is a regression or if these failures were simply 
unreported previously.

Valid RFC822 nameConstraints Test25
Invalid DNS nameConstraints Test38

I will investigate these test failures.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

The failure of test 25 was because the pkits.sh script had the wrong cert
file names for that test case.  See bug 231221 for more details.
That has now been fixed.  

So, the remaining test case of interest is case 38.

This is a place where the NIST test clearly contradicts RFC 3280. 
We must wait for some authoritative statement of what is correct before
we change this NSS code AGAIN.

NIST says, about this test case (section 4.13.38, pages 82-83):

   In this test, the intermediate certificate includes a nameConstraints 
   extension that specifies a single permitted subtree. The end entity 
   certificate includes a subjectAltName extension with a dNSName that falls 
   outside that subtree. The permitted subtree is “testcertificates.gov” and 
   the subjectAltName is “mytestcertificates.gov”.

   Expected Result: The path should not validate successfully.

But RFC 3280 says, converning dNSName constraints (section 4.2.1.11, page 38)

   DNS name restrictions are expressed as foo.bar.com.  Any DNS name
   that can be constructed by simply adding to the left hand side of the
   name satisfies the name constraint.  For example, www.foo.bar.com
   would satisfy the constraint but foo1.bar.com would not.

Clearly, mytestcertificates.gov is a name that can be constructed by simply
adding "my" to the left hand side of the name "testcertificates.gov".  

I am lowering this bug to P2, because IMO we should not hold up any future 
NSS 3.x release for it.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

This patch makes NSS pass PKITS test 38, and does not introduce any regressions

in any of the test cases now in pkits.sh.  

I have sent email to the PKITS mailing list to ask about this, but I expect
the answer to be that we should implement the rules that this patch follows.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 139257 [details] [diff] [review]
patch v1

Wan-Teh, I know this is your favorite code :) 
Please review.

Comment 4

[Wan-Teh Chang]

Comment on attachment 139257 [details] [diff] [review]
patch v1

r=wtc.	The proposed change is a sensible interpretation
of RFC 3280, and your patch correctly implement that.

We should wait until we've got a reply on the PKITS mailing
list to check this in.

We should also obtain the answers to the questions and
"probably not" in the new comment:

>+** .foo.bar.com      www.foo.bar.com      matches    matches? disallowed?
...
>+** .foo.bar.com     www..foo.bar.com      matches    probably not

With this patch applied, perhaps compareURIN2C can simply
call compareDNSN2C after checking that the constraint is
NOT zero-length and is a domain name.  Or, do you think
this is risky if we change compareDNSN2C again and forget
to update compareURIN2C?

Comment 5

[Wan-Teh Chang]

If the constraint begins with a dot ".", are you sure
that NIST's PKITS test suite still requires that any
name added to the left hand side end in a dot "."?

I believe this is what the questions in the new comment
are about, right?

>+** .foo.bar.com      www.foo.bar.com      matches    matches? disallowed?
...
>+** .foo.bar.com     www..foo.bar.com      matches    probably not

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

in answer to the question in comment 5, pkits doesn't state a new set of 
rules for name constraints.  Rather, it imposes test cases, from which we
can only infer their rules.  

I believe their implied rules want a matching name to appear to be a 
valid domain name, of which the constraint is a higher-level domain.
Clearly, a ".." doesn't match those rules, which is why I coded it as 
I did.

I propose to wait to the end of this week for a reply on the PKITS 
mailinglist, and failing that, to check the patch in on Friday.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

/cvsroot/mozilla/security/nss/lib/certdb/genname.c,v  <--  genname.c
new revision: 1.25; previous revision: 1.24

Comment 8

[Wan-Teh Chang]

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