7:13 p.m.
Please review the attached patch which replicates the new python client's KeyClient
class
on the Java side.
--Abhishek.
12:33 p.m.
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.
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)
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
11:11 a.m.
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
12:54 p.m.
New subject: [PATCH] 86-2 Addressed comments for patch 86
Please ignore the previous patch. Removed some unnecessary code.
--Abhishek
----- Original Message -----
From: "Abhishek Koneru" <akoneru(a)redhat.com>
To: alee(a)redhat.com
Cc: "pki-devel" <pki-devel(a)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(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
12:03 p.m.
New subject: [PATCH] 86-2 Addressed comments for patch 86
In KeyClient:
1. setTransportCert() should have a javadoc explaining exactly what is
in this variable (the base64 cert sans header and footer?)
2. Do we have something specifying the valid status values to be set in
modifyKeyStatus()? If so, we should probably add a check for that in
modifyKeyStatus()
3. retrieveKey() can be simplified. Unlike python, where we cannot
overload methods, we can do so on the Java side. Separate into
retrieveKey(keyId) and retrieveKey(keyId, transWrappedSessionKey)
This will simplify the documentation too.
4. A similar simplification can be done for retrieveKeyByPassphrase()
5. Fix algorithm OID.
6. NSSCryptoProvider:
* In initialize(), remove TODO comment.
* generateSessionKey(algorithm?). The purpose of this method is
to return a symmetric key that is valid for wrapping/ unwrapping
the DRM. Right now, we support only one algorithm -- 168 bit
DES3 keys. So this method should not include an algorithm as
a parameter. The whole point of this method is that the user
need not know what the algorithm is.
* generateSymmetricKey() has no length parameters.
This is not generic enough - some algorithms require length
parameters. See the python code.
* the methods should have a check for whether the object is
initialized and if not - initialize. This will prevent
null pointer exceptions in case someone fails to initialize first.
* getKeyGenAlgorithm() and getEncryptionAlgorithm() should not
return defaults if the argument passed in is wrong. They should
throw an exception. You return the default if the value passed in
is null.
7. There was code added to the Python side to set up an NSS database
for the user if one did not already exist, and import the
transport cert. Much of that is pretty useful, but has not been
added to the Java side.
8. Please confirm that all the new functions in KeyClient are
exercised in the DRMTest.
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>
To: alee(a)redhat.com
Cc: "pki-devel" <pki-devel(a)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(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
11:02 a.m.
New subject: [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>
To: alee(a)redhat.com
Cc: "pki-devel" <pki-devel(a)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(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
8:42 a.m.
New subject: [PATCH] 86-3 Addressed comments for patch 86-2
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(a)redhat.com>
> To: alee(a)redhat.com
> Cc: "pki-devel" <pki-devel(a)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(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
3712
days inactive
3727
days old
6 comments
2 participants
participants (2)
-
Abhishek Koneru -
Ade Lee