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

[Abhishek Koneru]

ACKe'd by Ade with some comments on IRC.
Addressed the comments and pushed to master.

--Abhishek
On Tue, 2014-03-18 at 12:02 -0400, Abhishek Koneru wrote:
> 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 at redhat.com>
> > To: alee at redhat.com
> > Cc: "pki-devel" <pki-devel at redhat.com>
> > 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 at redhat.com
> > > > https://www.redhat.com/mailman/listinfo/pki-devel
> > > 
> > > 
> > > 
> > 
> > --Abhishek
> > 
> > _______________________________________________
> > Pki-devel mailing list
> > Pki-devel at redhat.com
> > https://www.redhat.com/mailman/listinfo/pki-devel
> > _______________________________________________
> > Pki-devel mailing list
> > Pki-devel at redhat.com
> > https://www.redhat.com/mailman/listinfo/pki-devel
>