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)
Severity: normal → major
Priority: -- → P1
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)
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: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.