[Pki-devel] [PATCH] 181-188 Adding Symmetric Key generation Service to DRM

[Ade Lee]

Final ack by Endi.

Pushed to master:

Exit status 0
To ssh://vakwetu@git.fedorahosted.org/git/pki.git
   78031e7..cf425df  master -> master

On Mon, 2014-02-03 at 22:55 -0500, Ade Lee wrote:
> New patch attached to address comments as below.
> 
> Please apply 189 on top of rest of patches.
> 
> On Thu, 2014-01-30 at 13:02 -0600, Endi Sukma Dewata wrote:
> > On 1/30/2014 9:48 AM, Ade Lee wrote:
> > > Hi,
> > >
> > > The attached patches add Symmetric Key generation service to the DRM and
> > > refactor the DRM REST interface.  Its worthwhile to look at each patch
> > > individually, but there will be many cases where I changed my mind on
> > > how to represent something - for instance, Request -> KeyRequest ->
> > > ResourceMessage.  So, the patches should be viewed as a whole.
> > >
> > > Summary of changes:
> > > 1) Added new REST service to generate symmetric keys.
> > > 2) Refactor API to use POST /keyrequests for all request types and using
> > > a generic RequestMessage object.
> > > 3) Refactor PKIException to use RequestMessage object.
> > > 4) Rename some objects in Key and KeyRequest resources.
> > >
> > > I tested all this using the DRMTest code.  I needed to comment out a
> > > couple of tests because they were causing problems (including a core
> > > dump on the client side), and I need to investigate why that happened.
> > > Those tests will be restored once I figure out whats going on.
> > >
> > > I'd like to get several eyes on this, please.
> > >
> > > Thanks,
> > > Ade
> > 
> > Some comments:
> > 
> > 1. Minor issue. Please put a space before the curly bracket:
> > 
> >      public static class Data extends ResourceMessage{
> > 
> Fixed.
> 
> > 2. I'm not sure if the ResourceMessage should have a Link attribute. The 
> > PKIException doesn't need it. Probably many other request/resource 
> > objects won't need it either.
> > 
> Removed Link
> > 3. The PKIException previously has <Attributes>. Now that it uses 
> > <Properties> should we start implementing API versioning?
> > 
> Revert to Attributes
> 
> > 4. This is actually an existing issue in the current code. The 
> > marshall/unmarshal code currently swallows the exception. We probably 
> > should have thrown the original exception, or if not possible we should 
> > wrap it with a RuntimeException.
> > 
> Deferring to later patch.
> 
> > 5. This is also an existing issue. The KeyDataInfo name is probably 
> > redundant. If it's an Info that means it doesn't have the Data, so the 
> > name probably should be KeyInfo. Similarly, the KeyDataInfoCollection 
> > probably can be renamed to KeyInfoCollection. The XmlRootElement should 
> > match too, but this probably requires versioning.
> > 
> > @XmlRootElement(name = "KeyDataInfos")
> > public class KeyDataInfoCollection extends DataCollection<KeyDataInfo> {
> > 
> Renamed classes.
> 
> > 6. The KEYGEN_ALGORITHMS is defined in SymKeyGenerationRequest but it's 
> > only used by the server only. Will the client need this too? Otherwise 
> > we should move it to the server. Maybe the client just needs the list of 
> > alg names instead of the actual objects?
> > 
> Fixed.
> 
> > 7. The DRMTest is using constants in PKIService. While this works in dev 
> > env, a real client will not be able to use the server class. We should 
> > move the constants and maybe provide a method in the client library to 
> > strip the header & trailer. Or maybe strip them on the server.
> > 
> > transportCert = transportCert.substring(PKIService.HEADER.length(),
> > transportCert.indexOf(PKIService.TRAILER));
> > 
> Defer to later patch.
> > I may have some more comments later.
> > 
> 
> Also added code to address Christina's comments as below:
> SymKeyGenService.java:
> * does IRequest.RESULT not need to be set in request for
> SymKeyGenService? and also
> mKRA.getRequestQueue().updateRequest(request); ?
> 
> Added code to SymKeyGenService etc.
> 
> * I am actually kind of surprised that the SymKeyGenService doesn't do
> both key gen and recovery in one shot.  That's what asym key
> server-side-key-generation does --- generating keys, archive, and then
> return the keys to the caller, all in one shot.
> If not needed now, we can at least put down as "ToDo" as an option, if
> the client supplies the kra-transport-wrapped session key.
> 
> Defer to later patch.
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel