[Pki-devel] Request for review: ECC support for pkisilent (CA, subCA, DRM, TKS, TPS, OCSP)

[Christina Fu]

Hi Ade,

Please find the following changes as discussed:
https://bugzilla.redhat.com/attachment.cgi?id=700248&action=diff&context=patch&collapsed=&headers=1&format=raw
https://bugzilla.redhat.com/attachment.cgi?id=700250&action=diff&context=patch&collapsed=&headers=1&format=raw

thanks!
Christina

On 02/19/2013 07:28 PM, Ade Lee wrote:
> Christina,
>
> The changes you have put in ComCrypto.java look great.  I did notice,
> however, that you do a similar token assignment around line 710, but
> still have the unnecessary cast, no check for null and wonky exception
> handling.  Please fix this instance too.
>
> Once that is done, we can ACK the patch.
>
> Ade
>
> On Tue, 2013-02-19 at 18:04 -0800, Christina Fu wrote:
>> Ade,
>>
>> Thank you for the review.
>>
>> Per our discussion.  The agreed changes can now be found at:
>> https://bugzilla.redhat.com/attachment.cgi?id=699762&action=diff&context=patch&collapsed=&headers=1&format=raw
>>
>> In addition, I have added changes to the admin cert profile to accept ECC:
>> https://bugzilla.redhat.com/attachment.cgi?id=699763&action=diff&context=patch&collapsed=&headers=1&format=raw
>>
>> Also, I have now put the ECC setup instruction at this location:
>> http://pki.fedoraproject.org/wiki/ECC_Setup_Instructions
>> So the silentEC_readme.txt now only contains link to it:
>> https://bugzilla.redhat.com/attachment.cgi?id=699764
>>
>> thanks!
>> Christina
>>
>> On 02/19/2013 12:04 PM, Ade Lee wrote:
>>> Comments:
>>>
>>> In general, the code changes look fine.  Just a couple of nitpicks ..
>>>
>>> 1. The variable save_p12 should be a boolean rather than a string.
>>>
>>> 2. In ComCrypto.java, you define setTokenName().  What about
>>> getTokenName()?
>>>
>>> 3. In ComCrypto.java,on line 448, you cast token to (PK11Token) and on
>>> 450, there is no cast.  Why the discrepancy?  Is the cast needed?
>>>
>>> 4. The exception handling in ComCrypto.java for lines 448/450 is pretty
>>> wonky.  It seems like token could return null.  Could we improve the
>>> exception handling in this case?
>>>
>>> Ade
>>>
>>> On Tue, 2The 013-02-19 at 11:26 -0800, Christina Fu wrote:
>>>> This is a request for code review for the following feature bug:
>>>> Bug 810967 - [RFE] ECC support for pkisilent
>>>>
>>>> the code changes can be found here:
>>>> https://bugzilla.redhat.com/attachment.cgi?id=699587&action=diff&context=patch&collapsed=&headers=1&format=raw
>>>>
>>>> The 5 new templates and one readme instruction file can be found the
>>>> the bug attachment.
>>>>
>>>> thanks!
>>>> Christina
>>>>
>>>>
>>>> _______________________________________________
>>>> Pki-devel mailing list
>>>> Pki-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pki-devel
>