[Pki-devel] [pki-devel][PATCH] 0064-Port-symkey-JNI-to-Java-classes.patch

[Christina Fu]

Just realized that I missed this comment for conditional ack:

It was already communicated to Jack. Please file a  ticket for this.
derivedKey = encryptDes3.derive();
  it throws exception when fail (in nethsm it seems to be the case now), 
and then default to encryption.
  It'd be better to provide a config param to turn on and off derive 
v.s. encryption in case we know for sure a certain hsm does not handle 
such thing, then the process won't waste the consistent exceptions.

Once again, great job on such daunting task!!
thanks,
Christina

On 05/18/2016 06:31 PM, Christina Fu wrote:
> This is the re-review of the patches that addressed my original comments.
> Overall I think it's good for this round.  I only have a few comments 
> and most have already been communicated to jack.
>
> Conditional ACK upon completion of the following, and of course, 
> tested to work:
>
> * Please open the new tickets for tasks you wish to push for later. 
> Feel free to combine things in same area into one ticket if it makes 
> sense. Please list the ticket(s) at commit response.
> * Please write a wrapper function computeKekKey() to call the 
> computeSessionKey_SCP01() with null null, so for people who read the 
> code it's clear that it's actually getting the kek key handle rather 
> than a session key.
> * wrapSessionKey() now takes a wrapping key, and if it's null, it 
> takes a global transportKey.  Please put this in a top block method 
> comment to make it clear what this method does
> * you defined cryptogram types (per my earlier comment), but you did 
> not replace the 0 and 1 in the calling method.
> * the top of method comment convention is usually using /* ...*/ 
> instead of a whole bunch of //'s
>
>
> thanks!
> Christina
>
>
> On 05/17/2016 06:44 PM, John Magne wrote:
>> Enclosed revised patches:
>>
>> Thanks to cfu for careful review.
>>
>> Also enclosed responses to comments ,for convenience.
>>
>>
>>
>>
>> ----- Original Message -----
>>> From: "Christina Fu" <cfu at redhat.com>
>>> To: pki-devel at redhat.com
>>> Sent: Friday, May 13, 2016 11:34:17 AM
>>> Subject: Re: [Pki-devel] [pki-devel][PATCH] 
>>> 0064-Port-symkey-JNI-to-Java-classes.patch
>>>
>>> Hi,
>>> First of all, I have to say that Jack did a wonderful job on such 
>>> daunting
>>> task. The sheer amount of code and complexity does make the review more
>>> challenging, but I dug through them with my teeth and claws 
>>> regardless ;-).
>>>
>>> We discussed and think we should postpone the checkin to next 
>>> release so we
>>> can make sure it gets the kind of attention in details that it 
>>> deserves.
>>>
>>> For the first round of reviews, I sent him two separate sets of review
>>> comments last week. One for JSS, and one for the rest.
>>> The JSS patch was not attached to his original email request for 
>>> review. It
>>> is attached to the following ticket:
>>> https://fedorahosted.org/pki/ticket/801
>>>
>>> You can find my review comments attached to this email.
>>>
>>> thanks,
>>> Christina
>>>
>>> On 04/15/2016 07:03 PM, John Magne wrote:
>>>
>>>
>>>
>>> Subject: [PATCH] Port symkey JNI to Java classes. Ticket #801 : Merge
>>>   pki-symkey into jss
>>>
>>> What is supported:
>>>
>>> 1. Everything that is needed to support Secure Channel Protocol 01.
>>> 2. Supports the nist sp800 kdf and the original kdf.
>>> 3. Supports key unwrapping used by TPS which was formerly in the 
>>> symkey JNI.
>>>
>>> Requires:
>>>
>>> 1. A new JSS that supports more advanced symkey operations such as key
>>> derivation, more advanced key
>>> unwrapping , and a way to list and identify a given symmetric key by 
>>> name.
>>> Version of new Jss will be forthcoming.
>>>
>>> Still to do:
>>>
>>> 1. Port over the 2 or 3 SCP02 routines from Symkey to use this code.
>>> 2. The original symkey will remain in place until we can port over
>>> everything.
>>> 3. SCP03 support can be added later.
>>>
>>>
>>> _______________________________________________
>>> Pki-devel mailing list Pki-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pki-devel
>>>
>>>
>>> _______________________________________________
>>> Pki-devel mailing list
>>> Pki-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pki-devel
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel