[Pki-devel] [PATCH] PKCS10Client enhancement

[Ade Lee]

On Thu, 2013-08-15 at 16:29 -0700, Andrew Wnuk wrote:
> On 08/15/2013 09:47 AM, Ade Lee wrote:
> > Couple of questions:
> >
> > 1. I am assuming that ":" is not a valid  character in each of the
> > subject name fields, right?
> ":" is not on the list of special characters required to be escaped, so 
> it is a valid character as names are validated. You can see name 
> validation in the code.
> 
> Keep in mind that this tool modification is added only to test one very 
> unique incompatibility of some cryptographic libraries which are not 
> processing subject names in their proper canonical form, which can only 
> be seen in one corner case of cross signing.
> 
> There are few options to introduce encoding types:
> 1. Introduce encoding types as a separate set parameters.
> 2. Introduce distinguished name preprocessing, collect encoding types 
> from preprocessor and distribute them to component name encoders.
> 3. Use above syntax only if enabled by flag
> 4. Use above syntax only if prefix includes allowed encoding typesfor 
> specific name component
> 
> Option (1) seems inconvenient and error prone
> Option (2) seems a bit expensive for its purpose
> Option (3) seems reasonable but new enabling flag seems a bit inconvenient
> I believe I selected option (4) but attached the wrong file.
> I selected option (4) base on expense and the purpose of this tool.

I am OK with using option (4) given the limited scope of this
tool/usage.  However, given that ":" is a valid character, the code as
you currently have it could break on strings that contain a ":".

For example, "cn=aaaa:bbb, ou=ccc, o=dddd" will be encoded as "ou=ccc,
o=dddd".  Why?  Because the cn=aaaa:bbb will be parsed as one that is
supposed to contain an encoding.  When the supposed encoding ("aaaa")
does not match what you expect, it is silently ignored and dropped.

Right?  Or am I missing something?
 
> > 2. Is the format "cn=UTF8String:aa,ou=BMPString:bb,o=cc" a standard way
> > of specifying this type of thing?  If not, is there a standard way of
> > specifying subject names with multiple encodings? (and why would anyone
> > do that?)
> There is no standard way to include encoding for attribute values in 
> distinguished names.
> The main reason here is that distinguished names are defined by LDAP 
> standards providing strict encoding rules and strict name processing rules.
> LDAP is not exposed to our case of cryptographic libraries incorrectly 
> processing subject names.
> > 3. Right now, if someone puts in an incorrect encoding, the element will
> > silently fail to be added to the subject name.  Is this what we want?
> > Do we rather want to throw an exception and notify the user?
> This will go away with option 4.

> > 4. Similarly, in addNameElement, we catch the exception and just print
> > out an error message.  Do we want to rather bubble up the exception and
> > abort?
> I think that providing error message is more appropriate here.
> >
OK

> > 5. Does it make sense to add logic to addNameElement() to default to
> > PrintableString in the case that n=0, and then just eliminate the calls
> > to the old functions?
> >
> > For example, instead of :
> >
> >                 if (split[0].equals("O")) {
> >                      if (n > 0) {
> >                          ret = addNameElement (ret, Name.organizationName, n, split[1]);
> >                      } else {
> >                          ret.addOrganizationName(split[1]);
> >                      }
> >                      //                System.out.println("O found : " + split[1]);
> >                      continue;
> >                 }
> >
> > have:
> >                 if (split[0].equals("O")) {
> >                      ret = addNameElement (ret, Name.organizationName, n, split[1]);
> >                      // System.out.println("O found : " + split[1]);
> >                      continue;
> >                 }
> >
> > and remove the function addOrganizationName() etc.  In fact, it probably
> > make sense to just compute n in the addNameElement() function, rather
> > than passing it.
> All "name" functions are provided by JSS, which includes OID to name 
> mapping and default encoding
> To do the above, you would have to duplicate OID to name mapping and 
> default encoding selection outside JSS, which is an error prone 
> duplication of functionality.
> 

Well, you're not really duplicating anything in that you are passing the
OID mapping provided by JSS, and the default is provided by the RFC,
right?

Still, you'll need to change your code to handle the case I mention
above.  In pseudo-code, I guess you could do something like this:

valid_encodings = {arraylist of encodings}
encoding = substring(value)

if (split[0].equals("O")) {
    if (encoding in valid_encodings) {
        ret = addNameElement (ret, Name.organizationName, encoding, value);
    } else {
        ret.addOrganizationName(value)
    }
}
 
> >
> > 6.  Please add some comments above the addNameElement() function to
> > describe what is going on. (maybe referencing the ticket number).
> Sure.
> >
> > 7.  In the diff at least, it looks like the System.out.println()
> > statements are a little offset.  I know they are not in your code
> > changes, but please fix them while we are there.
> I thought those lines are purposely formatted this way, since all files 
> were reformatted before.

The reformatting was done over all the code - its very mostly right, but
there will be some bad cases every so often, particularly in commented
out code.  We should just fix them as we find them.

> >
> > Thanks,
> > Ade
> >   
> > On Wed, 2013-08-14 at 17:08 -0700, Andrew Wnuk wrote:
> >> This patch provides enhancement to PKCS10Client allowing to control
> >> encoding for components of the subject name.
> >>
> >> Ticket #677
> >>
> >> More details included in comment #2 of ticket #677.
> >> Testing procedure included in comment #4 of ticket #677.
> >>
> >>
> >> _______________________________________________
> >> Pki-devel mailing list
> >> Pki-devel at redhat.com
> >> https://www.redhat.com/mailman/listinfo/pki-devel
>