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

On Mon, Sep 08, 2014 at 04:00:16PM -0700, Christina Fu wrote: > 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 Above issues have been addressed; new patches attached. Fraser