510367 - Fix the UTF8 characters in the nickname string for AC Raíz Certicamara S.A.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

Attachment: replace UTF8 characters in strings with \xNN\xNN

The nickname string in nssckbi for the newly added root cert for 
AC Raíz Certicamara S.A. has two UTF8 characters.  They got munged and are
invalid in the cvs repository.  They don't appear in Firefox, which never 
shows the nicknames, but they're a problem for any software that uses the
nicknames, because they're not valid UTF8.  

The attached patch corrects this.  I hope to sneak this into 3.12.4 RTM

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Checking in certdata.c;   new revision: 1.55; previous revision: 1.54
Checking in certdata.txt; new revision: 1.54; previous revision: 1.53

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

I wish we had a real UTF8 system on which to test this stuff.  
It would be nice to see the output of certutil on a system that really
handles UTF8 locale everywhere, and see that it displays correctly.  
I have no such system.  Does anyone on the NSS/NSPR team have one?  

I have verified that the hex values that certutil gets from the NSS libraries
in the nickname have the same binary (hex) values as are found in the cert's
subject name.  This is as sure as I can get that the values are correct, in 
the absence of a true way to display arbitrary UTF8 strings.  

I jumped the gun with this commit, I suppose.  Mea culpa.  
I think we can retag the two files listed above now.

Comment 3

[Robert Relyea]

Comment on attachment 394400 [details] [diff] [review]
replace UTF8 characters in strings with \xNN\xNN

r+ I've verified the UTF8 0xc3 0xad maps to 0x00ED which is 'i' with accent and 0xc3 0xa1 mapes to 0x00E1 which is a accent.

These match with the original cert request ing  Bug 486424 -  Add AC Raíz Certicámara S.A. root certificate to NSS

Comment 4

[Julien Pierre]

re: comment 2,

On solaris 10, I was able to set LANG=en_US.UTF-8 and then started an xterm.
After that, "certutil -d . -L -h all" showed all the root certs correctly, including the one in question here. However, I found no way to list the details of that cert with "certutil -d . -L -n <nickname>" . The cert cannot be found. I believe this fails because the nickname does not get re-encoded as multi-byte UTF-8 which would be needed to get a match in libnssckbi.so . However, that is a certutil problem, not NSS.

Comment 5

[Wan-Teh Chang]

Comment on attachment 394400 [details] [diff] [review]
replace UTF8 characters in strings with \xNN\xNN

It would be nice to also have an ASCII version of the name
in the comment.  Just like "Bodo Moeller", where "oe" stands
for the German 'o' with two dots.

Comment 6

[Wan-Teh Chang]

Comment on attachment 394400 [details] [diff] [review]
replace UTF8 characters in strings with \xNN\xNN

>-# Certificate "AC Ra+¡z Certic+ímara S.A."
>+# Certificate "AC Ra\xC3\xADz Certic\xC3\xA1mara S.A."

This can be

# Certificate "AC Raiz Certicamara S.A."

which is much more readable in a C source file.

>-# Trust for Certificate "AC Ra+¡z Certic+ímara S.A."
>+# Trust for Certificate "AC Ra\xC3\xADz Certic\xC3\xA1mara S.A."

This can be

# Trust for Certificate "AC Raiz Certicamara S.A."

Comment 7

[Wan-Teh Chang]

Attachment: Use ASCII version of nickname in comments for readability

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 394469 [details] [diff] [review]
Use ASCII version of nickname in comments for readability

Actually, something doesn't look quite right here, Wan-Teh,
Each occurrence of $ has become $ $
Maybe you should edit that before committing

>-  { (void *)"@(#) $RCSfile: certdata.c,v $ $Revision: 1.55 $ $Date: 2009/08/13 23:40:29 $""; @(#) $RCSfile: certdata.c,v $ $Revision: 1.55 $ $Date: 2009/08/13 23:40:29 $", (PRUint32)160 }
>+  { (void *)"@(#) $RCSfile: certdata.txt,v $ $Revision: 1.54 $ $Date: 2009/08/13 23:40:29 $""; @(#) $RCSfile: certdata.perl,v $ $Revision: 1.12 $ $Date: 2008/01/23 07:34:49 $", (PRUint32)160 }

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Wait, certdata.c -> certdata.txt?

Comment 10

[Wan-Teh Chang]

Yes, I was confused by that at first, too.  I believe that
the certdata.perl script records the revisions of the certdata.txt
and certdata.perl (itself) in the output certdata.c file.  This
was OK when we were generating certdata.c during the build.

I remember we checked certdata.c into CVS so we could support
Classic Mac OS, which didn't use makefiles.  Once checked in,
CVS substitutes the RCS $RCSFile keyword by the name of the file
(certdata.c).

Perhaps it's time to go back to generating certdata.c during
the build.

Comment 11

[Wan-Teh Chang]

Comment on attachment 394469 [details] [diff] [review]
Use ASCII version of nickname in comments for readability

I checked in this patch on the NSS trunk (3.12.5).
Since this is just a comment change, I didn't bump
the nssckbi version.

Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.56; previous revision: 1.55
done
Checking in certdata.txt;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.txt,v  <--  certdata.txt
new revision: 1.55; previous revision: 1.54
done

Comment 12

[Robert Relyea]

We check in certdata.c because we weren't confident all our build environments had the appropriate perl support (at the time windows either didn't have perl, or had an old version that didn't run the perl script).

Comment 13

[Wan-Teh Chang]

One solution is to use the "cvs admin -ko" command on certdata.c
to suppress RCS keyword expansion.  But I remember this will
apply to all revisions of that file, including the old releases.
So we will need to move certdata.c to a new file, such as
certdata2.c.  Would that be too confusing?  Note: this is why we have
security/nss/tests/ssl/sslreq.dat.  It used to be
security/nss/tests/ssl/sslreq.txt, and when we ran "cvs admin -kb"
to mark the file as binary, we also renamed it to avoid interfering
with old releases.

Comment 14

[Julien Pierre]

Wan-Teh,

Will cvs admin -ko still allow us to view diffs ? I know cvs admin -kb will not.
I think it may be confusing to do this on existing files.

I think the solution to have ASCII versions of nicknames in comments, and escaped versions of multi-byte UTF-8 nicknames (or any other strings) in the structures, should be OK going forward.

Comment 15

[Wan-Teh Chang]

Julien,

We're talking about the $RCSFile keywords inserted
into certdata.c by certdata.perl being replaced by
wrong file upon checkin.  See the diffs in comment 8.
The second line is the desired one, but it becomes
something like the first line upon checkin.

We're not talking about unescaped UTF8 character problem.