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

[Endi Sukma Dewata]

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{

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.

3. The PKIException previously has <Attributes>. Now that it uses 
<Properties> should we start implementing API versioning?

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.

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> {

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?

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));

I may have some more comments later.

-- 
Endi S. Dewata