1523484 - libpkix name constraints check treats CN as DNS name when it should not

Status: RESOLVED FIXED

Description

[Fraser Tweedale]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

1. Have a CA with DNS name constraints permitted subtree.

2. Verify a certificate with an Extended Key Usage not containing id-kp-serverAuth (e.g. an OCSP signing certificate), whose Subject DN CN attribute does not resemble a DNS name within the permitted subtree.

3. Verify the certificate (using libpkix):

   NSS_ENABLE_PKIX_VERIFY=1 certutil -d $NSSDB -f $PWDFILE -V -e -n $NICKNAME -u O

Actual results:

Verification fails:

# NSS_ENABLE_PKIX_VERIFY=1 certutil -d $NSSDB -f $PWDFILE -V -e -n $NICKNAME -u O
certutil: certificate is invalid: The Certifying Authority for this certificate is not permitted to issue a certificate with this name.

Expected results:

Verification should succeed. The CN should not be treated as a DNS name unless the certificate being validated is a server authentication certificate (i.e. id-kp-serverAuth is asserted in the Extended Key Usage extension). For compatibility, it may be acceptable to continue treating CN as a DNS name if the certificate does not have the EKU extension.

I will follow up with a patch soon.

Comment 1

[Fraser Tweedale]

Created attachment 9039696 [details] [diff] [review]
nss-ftweedal-0000-Bug-1523484-do-not-treat-CN-as-DNS-name-for-non-serv.patch

Comment 2

[Kai Engert (:kaie:)]

Maybe an RH NSS developer could review this?
Also, fixes should go together with a test.

Comment 3

[Fraser Tweedale]

Comment on attachment 9039696 [details] [diff] [review]
nss-ftweedal-0000-Bug-1523484-do-not-treat-CN-as-DNS-name-for-non-serv.patch

Daiki, maybe you could review this (or pass along to one of the other devs
at Red Hat?)

Comment 4

[Daiki Ueno [:ueno]]

Comment on attachment 9039696 [details] [diff] [review]
nss-ftweedal-0000-Bug-1523484-do-not-treat-CN-as-DNS-name-for-non-serv.patch

Review of attachment 9039696 [details] [diff] [review]:
-----------------------------------------------------------------

The change looks reasonable at first glance, but it needs a test as Kai already pointed out.  Bug 757854 might give you some inspiration.

Comment 5

[Fraser Tweedale]

Thanks Daiki, I'll add a test and provide an updated patch next week (or reach out if I cannot work out what to do). Have a great weekend!

Comment 6

[Fraser Tweedale]

Created attachment 9041097 [details] [diff] [review]
nss-ftweedal-0000-Bug-1523484-do-not-treat-CN-as-DNS-name-for-non-serv.patch

Now with test.

Comment 7

[Daiki Ueno [:ueno]]

Comment on attachment 9041097 [details] [diff] [review]
nss-ftweedal-0000-Bug-1523484-do-not-treat-CN-as-DNS-name-for-non-serv.patch

Review of attachment 9041097 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for the update, it looks good to me.  Try run is ongoing:
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=776c6973b3a389b58a5278391b6e41e95441d2ed

Comment 8

[Daiki Ueno [:ueno]]

https://hg.mozilla.org/projects/nss/rev/db68b6975216