[Pki-devel] [PATCH] 191 - Add strength and algorithm to KeyData and KeyInfo

[Endi Sukma Dewata]

On 2/5/2014 12:27 PM, Ade Lee wrote:
>      Add strength and algorithm to KeyData and KeyInfo classes
>
>      Make sure these are updated so that clients can get this information
>      when accessing a symmetric key.  Also allow a default for generation
>      requests (but not for archival requests).
>
> Please review.
>
> Thanks,
> Ade

ACK. Feel free to push after addressing these minor issues:

1. Exception messages are meant to be read by human, so it would be 
better to use a user-friendly name (e.g. client ID) instead of variable 
name (e.g. clientId).

     throw new BadRequestException(
         "Invalid key generation request. Missing clientId");

2. The following alg/size validation should be done even for the default 
alg/size to make sure we're not hard-coding invalid values (or if it 
becomes invalid in the future).

     KeyGenAlgorithm alg =
         KeyRequestService.KEYGEN_ALGORITHMS.get(algName);
     if (alg == null) {
         throw new BadRequestException("Invalid Algorithm");
     }

     if (!alg.isValidStrength(size)) {
         throw new BadRequestException(
             "Invalid key size for this algorithm");
     }

3. If the keySize in SymKeyGenerationRequest is optional we should use 
Integer rather than int. This way we can detect and report missing 
keySize properly using null. Right now the above code seems to be 
generating "Invalid key size for this algorithm" on missing keySize 
since the value will be converted into 0.

4. It would be better to include the offending values in the error 
message: "Key size {keySize} is invalid for {algorithm} algorithm".

5. The terms "strength" and "size" aren't used consistently. Any preference?

-- 
Endi S. Dewata