The default bug view has changed. See this FAQ.

CERT_AsciiToName fails when AVAs in an RDN are separated by '+'

RESOLVED FIXED in 3.12.3

Status

NSS
Libraries
P1
major
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: SUN_MUST_HAVE)

Attachments

(2 attachments)

Created attachment 359711 [details] [diff] [review]
Patch v1 for NSS Trunk

When an RDN has multiple AVAs, they are separated in the ASCII string form
by '+' rather than by ',' or ';'.  CERT_NameToAscii generates that form 
of separator correctly, but CERT_AsciiToName fails when it encounters a '+'
separator.  This means that commands like certutil, which take a DN in ASCII
form on the command line, fail if the DN uses '+' to separate any AVAs.
CERT_AsciiToName is presently unable to create RDNs with multiple AVAs.
There's an existing function for adding AVAs to RDNs, and it is unused.
It's amazing that we've not run into this problem before now.

The attached patch seems to fix it.  Julien, please review it.
Sun CR 6799382.
Attachment #359711 - Flags: review?(julien.pierre.boogz)
(Assignee)

Updated

8 years ago
Severity: normal → major
Priority: -- → P1
Whiteboard: SUN_MUST_HAVE

Comment 1

8 years ago
Comment on attachment 359711 [details] [diff] [review]
Patch v1 for NSS Trunk

I think this patch is OK, but in ParseRFC1485Name, there should probably be a comment about the

if (bp[-1] != '+')

test. It appears to be reading undefined memory. It doesn't really, because bp has been incremented due to the successful call to CERT_ParseRFC1485AVA . 
It would be more elegant to add an argument to that function to signify that the RDN is done when + is encountered, so the caller doesn't have to parse backwards. That's a stylistic matter so I won't block the patch for that, as long as a comment is added.
Attachment #359711 - Flags: review?(julien.pierre.boogz) → review+
Checking in alg1485.c; new revision: 1.29; previous revision: 1.28

I will add a comment, as Julien suggested.
Created attachment 360777 [details] [diff] [review]
supplemental patch

This patch adds an explicit definition for the behavior of the AVA parsing
function, and makes the function behave according to that definition.
It also makes the function static.  It is basically a clean up.  
Julien, please review.
Attachment #360777 - Flags: review?(julien.pierre.boogz)

Updated

8 years ago
Attachment #360777 - Flags: review?(julien.pierre.boogz) → review+
Checking in certdb/alg1485.c; new revision: 1.30; previous revision: 1.29
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.