Last Comment Bug 210584 - CERT_AsciiToName doesn't accept all valid values
: CERT_AsciiToName doesn't accept all valid values
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.8
: All All
: P3 normal (vote)
: 3.12
Assigned To: Nelson Bolyard (seldom reads bugmail)
: 292369 (view as bug list)
Depends on: 124923 161326
Blocks: 210709
  Show dependency treegraph
Reported: 2003-06-24 22:39 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2007-08-27 23:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch v1, accept OID=#hex ATAV strings. (8.65 KB, patch)
2007-08-01 18:28 PDT, Nelson Bolyard (seldom reads bugmail)
no flags Details | Diff | Splinter Review
patch v2 (tested) (9.06 KB, patch)
2007-08-04 15:56 PDT, Nelson Bolyard (seldom reads bugmail)
alvolkov.bgs: superreview+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2003-06-24 22:39:37 PDT
Some time ago, bug 39494 was fixed.  NSS's function CERT_NameToAscii was 
enhanced to translate unknown/unrecognized attribute types to decimal strings
that represented the OID values, and unrecognized attribute values were 
translated into hex strings (IIRC).  

But the function CERT_AsciiToName was not enhanced to be able to translate
the ASCII strings produced by  CERT_NameToAscii back into DER DNs.  

Consequently, CERT_AsciiToName now returns errors in response to some strings
returned by CERT_NameToAscii.  This should be fixed.  Ideally, the two functions
should be each other's inverses.
Comment 1 Nelson Bolyard (seldom reads bugmail) 2003-06-24 22:50:41 PDT
CERT_FindCertByNameString depends on this function.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2003-06-25 16:43:21 PDT
There are more problems with this function than I realized when I first filed it.
The problems include;

a) This function doesn't understand attribute types of the form 
where <dotted-decimal-oid> is an oid represented as decimal numbers separated
by periods,  and 

b) This function doesn't understand attribute values that are hex strings 
preceeded by #, and 

c) This function doesn't understand backslash escaping, which include 
escaping of hexadecimal pairs and escaping of special characters such as 
commas, and

d) This function doesn't reconstruct "multi-valued" RDNs, whose attributes
are separated by "+" characters.  

In short, this function needs to accept all input strings that are valid 
according to any of RFCs 2253, 1779 and 1485.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2003-07-03 14:02:10 PDT
For this function to work properly, our ASN.1 encoder needs to properly 
encode SET OF, which it does not do now.  Otherwise, the encoded AVAs in 
a multi-valued RDN may (probably will) be encoded in the wrong order, 
producing a non-matching DER DN.  I will file a separate bug.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2003-07-08 21:11:30 PDT
Comment 3 above was attached to the wrong bug.  It was meant for bug 210709.
Bug 211655 has now been created, and blocks bug 210709.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2004-01-28 23:36:57 PST
Changing dependency.  This bug now depends on bug 124923 instead of bug 196360.
Bug 196360 will become a dup of bug 124923.
Comment 6 Nelson Bolyard (seldom reads bugmail) 2005-05-02 09:35:20 PDT
*** Bug 292369 has been marked as a duplicate of this bug. ***
Comment 7 Nelson Bolyard (seldom reads bugmail) 2007-07-11 16:55:59 PDT
All the prerequisite dependencies of this bug are now resolved.  
Neil, please take this.
Comment 8 Steve Parkinson 2007-07-19 15:26:49 PDT
I think you also have to worry about changes to SetupAVAValue() in secname.c.
The switch statement will need to have a new case for 'undefined' type, since we don't know the type of the value. Perhaps you can used '0', since that's the type corresponding to the type for an unknown OID: SEC_OID_UNKNOWN in the name2kinds table in alg1485.c 

Comment 9 Nelson Bolyard (seldom reads bugmail) 2007-07-20 12:58:02 PDT
In reply to comment 8:

I think the relevant case is not "unknown" but rather "pre-encoded",
for use with values that are hexadecimal representations of the 
already-DER-encoded values.  

Is that what you had in mind, Steve?
Comment 10 Steve Parkinson 2007-07-20 13:25:06 PDT
So, we will define '0' in the valueType field of struct NameToKind as meaning:
 - "the associated value is the ASCII string of the form "#ab1234...", or
 - "the byte array resulting from decoding the ASCII string of the form "#ab1234"

Where is the hex encoding scheme specified? Its only mentioned in passing RFC 1485. I am most curious about what structure is to be encoded. Is it the entire "SET OF AttributeValue" field?

Attribute       ::=     SEQUENCE {
      type              AttributeType,
      values    SET OF AttributeValue }
Comment 11 Nelson Bolyard (seldom reads bugmail) 2007-07-20 15:50:56 PDT     was superseded by and     which were superseded by and     which were superseded by and and     (Basically 4510-4519)

You asked: "Where is the hex encoding scheme specified?"
See RFC 4514 section 2.4 and the example in section 4
The hex attribute value is the entire BER/DER encoding of the AttributeValue.
Comment 12 Nelson Bolyard (seldom reads bugmail) 2007-07-20 16:11:25 PDT
Note that this bug was blocked until recently by bug 161326, which reported
the absence of a function to turn ASCII strings of the form OID.n.n.n...
into DER-encoded OIDs.  But bug 161326 is now resolved fixed, and the 
new function SEC_StringToOID() is ready to be used by CERT_AsciiToName.
Comment 13 Nelson Bolyard (seldom reads bugmail) 2007-08-01 18:28:30 PDT
Created attachment 274868 [details] [diff] [review]
patch v1, accept OID=#hex ATAV strings.

Steve, can you test this patch and tell me if it solves the problem you're
having in libPKIX?
Comment 14 Nelson Bolyard (seldom reads bugmail) 2007-08-04 15:56:37 PDT
Created attachment 275277 [details] [diff] [review]
patch v2 (tested)

This patch is tested better than the last one.
I had to fix a bug in certutil to test it. 
Will file a separate bug about that.
Comment 15 Alexei Volkov 2007-08-10 13:58:43 PDT
Comment on attachment 275277 [details] [diff] [review]
patch v2 (tested)

look good. Seems to be much more robust parsing code.
Comment 16 Nelson Bolyard (seldom reads bugmail) 2007-08-27 23:42:53 PDT
Checking in alg1485.c;   new revision: 1.27; previous revision: 1.26
Checking in certi.h;     new revision: 1.19; previous revision: 1.18
Checking in secname.c;   new revision: 1.21; previous revision: 1.20

Note You need to log in before you can comment on or make changes to this bug.