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

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(a)redhat.com&gt; >> To: pki-devel(a)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(a)redhat.com >> https://www.redhat.com/mailman/listinfo/pki-devel >> >> >> _______________________________________________ >> Pki-devel mailing list >> Pki-devel(a)redhat.com >> https://www.redhat.com/mailman/listinfo/pki-devel _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel