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

[Christina Fu]

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