Re: [Pki-devel] [PATCH] 101 Generation of asymmetric keys in the DRM

On 8/4/2014 9:29 AM, Abhishek Koneru wrote: I've opened some tickets related to key management. Please take a look at them. The patch seems to be working just fine, so it's ACKed. Some comments below: 1. Not sure about the "b64" prefix in b64PublicKey and b64_public_key field names. We have some other fields that contain base-64 encoded values but they use regular field names. 2. Existing issue. The KeyGenerationRequest.getKeySize() swallows NumberFormatException and returns null. I think the method should let the exception be handled by the caller. It's a RuntimeException so it doesn't need to be declared. 3. In AsymKeyGenService.serviceRequest() the request ID doesn't really need to be converted into string. The string concatenation later will do that automatically. String id = request.getRequestId().toString(); 4. The following code in KeyRequestService might not be necessary because access to this service is already controlled by ACL, so owner is never null. if (owner == null) { throw new UnauthorizedException( "Key generation must be performed by an agent"); } In general we shouldn't hard-code authorization logic in the code unless it's something can't be expressed via ACL. 5. Some formatting issues: Formatting issue in KeyCLI.java: for(i=0;i<publicKey.length()/64;i++){ KeyRequestService.java: } else if (request instanceof AsymKeyGenerationRequest){ public Response generateAsymKey(AsymKeyGenerationRequest data){ KeyService.java: if(rec.getPublicKeyData() != null && getPublicKey){ AsymKeyGenerationRequest.java: public class AsymKeyGenerationRequest extends KeyGenerationRequest{ KeyGenerationRequest.java: public class KeyGenerationRequest extends ResourceMessage{ -- Endi S. Dewata