Re: [Pki-devel] [PATCH] 86-3 Addressed comments for patch 86-2

Please review the patch 86-3 which addresses comments given for patch 86-2. Submitting both patches 86-2 and 86-3. Apply 86-3 on top of 86-2. Thanks, Abhishek On Wed, 2014-03-12 at 13:54 -0400, Abhishek Koneru wrote: > Please ignore the previous patch. Removed some unnecessary code. > > --Abhishek > > ----- Original Message ----- > From: "Abhishek Koneru" <akoneru(a)redhat.com&gt; > To: alee(a)redhat.com > Cc: "pki-devel" <pki-devel(a)redhat.com&gt; > Sent: Wednesday, March 12, 2014 12:11:27 PM > Subject: Re: [Pki-devel] [PATCH] 86 Update KeyClient and DRMTest on the Java side similar to the python side > > Please review the patch which addresses the comments below. > > CryptoOperationStore(): > > On the python side, we created an abstract class CryptoUtil to > > encapsulate the crypto operations we wanted to provide. The idea here > > was that the client might want to use some crypto library other than > > JSS. In the main program for the client then - in the python case, its > > drmtest.py, the client would elect to instantiate the relevant subclass > > of CryptoUtil. > > > > The way you have it now, the KeyClient is hardcoded to use NSS. This is > > not what we want. Instead do this: > > > > Create a new abstract class - CryptoProvider. This will contain methods > > that we will call in KeyClient. Note that the CryptoProvider cannot > > contain static methods. Then create a NSSCryptoUtil class that > > implements the required methods, does init() etc. > > > > DRMTest should instantiate and pass the NSSCryptoUtil class into the > > KeyClient constructor. > > > Created classes CryptoProvider and NSSCryptoProvider in > base/common/src/com/netscape/certsrv/util. An NSSCrypto provider object > is passed in PKIClient object that stores information about > configuration and connection. Since PKIClient is passed to KeyClient it > can access the CryptoProvider object from there. The reason for adding > CryptoProvider in PKIClient is that it can be used in future for all > kinds of crypto operations to be done in any client. > > KeyClient: > > 1. Extra S in param_id description in getKeyInfo() - instead of KeyId > > object - something more descriptive like - key id for secret. This > > applies in many cases. > > 2. Many comments do not specify what the return is. > > 3. In generateKey(), you do not use transWrappedSession key - you should > > at least pass that parameter if it is provided. > > 4. archiveKey() - typo in comment clinetKeyId > > There is a sentence fragment -- To be implemented ... ? > > 5. archiveKey() - there are Java classes to deal with OIDs - and in > > fact, this particular OID is defined within JSS. You should use those > > classes, rather than : > > String algorithmOID = "{1 2 840 113549 3 7}"; > > 6. > > > > KeyRequestResponse > > 1. Needs a XMLAccessType(None) > > Addressed all the comments except for 5. Did not find a way to get the > OID from the algorithm name in the JSS packages. Added it as a static > final variable in KeyRequestResource. > > > > I may have further comments when the patch is resubmitted. > > > > Ade > > > > On Tue, 2014-03-04 at 20:13 -0500, Abhishek Koneru wrote: > > > Please review the attached patch which replicates the new python client's KeyClient class > > > on the Java side. > > > > > > --Abhishek. > > > _______________________________________________ > > > Pki-devel mailing list > > > Pki-devel(a)redhat.com > > > https://www.redhat.com/mailman/listinfo/pki-devel > > > > > > > > --Abhishek > > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel