197964 - incorrect handling of certificate with two CN's

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Pal'o Simo]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030313
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030313

Mozilla gives me a message "Security Error: Domain Name Mismatch" when I try to
connect to a https site which have correct certificate (Mozilla says). The
certificate viewer shows that in the "Subject" field there are 2 CN records -
the first showed has the same domain name I try to connect to, the second is
some old alias for the domain.
I think the warning is incorrect.

Reproducible: Always

Steps to Reproduce:
1. connect to https://two.fujitsu.sk

Actual Results:  
Security Error: Domain Name Mismatch

Expected Results:  
nothing - just connect

I've got replies, that IE does not show error on this, maybe Netscape doesn't,
too... not tested

Comment 1

[John Unruh]

Viewing the cert in IE, I can see two CN's. Mozilla is picking the second one in
the list and causing the Cert Name mismatch dialog. Kai, are 2 CNs in a server
cert within spec?

Comment 2

[Terry Hayes]

RFC 2818 (an informational RFC) states:

If a subjectAltName extension of type dNSName is present, that MUST
   be used as the identity. Otherwise, the (most specific) Common Name
   field in the Subject field of the certificate MUST be used. Although
   the use of the Common Name is existing practice, it is deprecated and
   Certification Authorities are encouraged to use the dNSName instead.

This RFC says that one of the Common Name (CN) fields should be used to match 
the hostname.  There is no expectation that both fields will be checked.

However, the "most specific" means the last one given in the DER encoding of 
the name, which is the first one printed.  NSS seems to choose the first 
matching element in the DER encoding (the last printed). This conflicts with 
the recommendation in this RFC.  However, since CN is being phased out in 
favor of DNSName, and multiple CN fields really have no meaning, this behavior 
probably won't be changed.

NSS also implements a matching rule that can me used to allow a single CN 
entry to match multiple host names. in this case CN=two.(icl|fujitsu).sk would 
match both names given in the certificate.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Regarding NSS choosing the first, rather than the last CN element, this
seems like it might be easy to fix.  It's probably worth fixing, if so.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I'm changing this to product=NSS since it appears to be due to NSS cert 
handling.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Taking bug for NSS 3.10.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1 - take last Common name, not first.

I wonder if the code should do this for other attributes also.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 151920 [details] [diff] [review]
patch v1 - take last Common name, not first.

Wan-Teh, please review

Comment 8

[Wan-Teh Chang]

Comment on attachment 151920 [details] [diff] [review]
patch v1 - take last Common name, not first.

r=wtc.

>+/* RDNs are sorted from most general to most specific.
>+ * This code returns the first one found, the most general one found.
>+ */
> static char *
> CERT_GetNameElement(PRArenaPool *arena, CERTName *name, int wantedTag)

Might want to highlight "first" in the comment, as you
did to "last" in the other comment.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

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

I also made the change to the comment that Wan-Teh requested.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

This bug was fixed by changing the function CERT_GetCommonName from returning
the first (least specific) CN to instead return the last (most specific) CN 
in the name.  SSL needs the most specific.  

But it seems that when displaying a certificate's name, PSM had been using that 
CERT_GetCommonName to get the CN string to display, and it had been getting and 
displaying the most general (least specific) name.  

So the change made to fix this bug caused PSM's cert viewer to begin to display 
a different value than in had previously been displaying, to the alarm of some
folks who had always expected the display to contain the most general name.  
That is the subject of bug 305890.  

I think the solution is probably along these lines:
Change CERT_GetCommonName to work as it did before.
Create a new function, CERT_GetMostSpecificCommonName to do what SSL needs,
and change SSL (and possibly many other callers of GetCommonName) to call
this other new function instead.