Closed Bug 476126 Opened 15 years ago Closed 15 years ago

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

Categories

(NSS :: Libraries, defect, P1)

Tracking

(Not tracked)

RESOLVED FIXED
3.12.3

People

(Reporter: nelson, Assigned: nelson)

Details

(Whiteboard: SUN_MUST_HAVE)

Attachments

(2 files)

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)
Severity: normal → major
Priority: -- → P1
Whiteboard: SUN_MUST_HAVE
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.
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)
Attachment #360777 - Flags: review?(julien.pierre.boogz) → review+
Checking in certdb/alg1485.c; new revision: 1.30; previous revision: 1.29
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.