Re: [Pki-devel] [PATCH] 0010..0013 DNP3/IECUserRoles extension support

Hi Fraser, My apology for getting back to you this late due to Dogtag release. (I think there may be a major issue there, so you might want to jump to the "hmmm" part first) General: * It would help if in the review request email, you could put a link to the spec you are coding against. I had to search around and every place I looked it requires me to sign in or purchase. IECUserRolesExtension.java * It would help if you could put the relevant ASN1 in the extension code IECUserRolesExtension.java * the getName() method returns the OID string instead of the conventional name of the class * by convention, other existing extension classes use the JAVA class Boolean instead of the native boolean for criticality. Please try to stick to it. * hmmm... Shouldn't this extension be a "SEQUENCE of" "UserRoleInfo"? This code seems to implement only the "UserRoleInfo" part. This would be a major problem. You might want to take a look of how SubjectAlternativeNameExtension.java is done where it is a "SEQUENCE of" GeneralName See: http://tools.ietf.org/html/rfc5280#section-4.2.1.6 scroll down a bit to see the ASN1 definition. Search in our code for the following: - SubjectAlternativeNameExtension.java - GeneralNames - GeneralName Again, since I don't have the spec that you code against so I might be wrong, please supply the ASN1 spec to this extension before I continue. I think I will stop here and let you work on / respond to the above first as it seems like a deal breaker if I was right. regards, Christina On 08/18/2014 12:03 AM, Fraser Tweedale wrote: