Code review request for JEP-121

Vincent Ryan vincent.x.ryan at oracle.com
Thu Oct 4 10:50:30 UTC 2012


I've made further modifications including adding support to PBEParameterSpec
for an AlgorithmParameterSpec object. This is used to convey parameters
(in addition to salt and iteration count) to the underlying cipher.

For example, AES requires an initialization vector so PBE algorithms that use
AES need a mechanism to supply an IV parameter.

The latest webrev is at:
   http://cr.openjdk.java.net/~vinnie/6383200/webrev.04/



On 4 Sep 2012, at 18:04, Vincent Ryan wrote:

> Thanks Valerie.
> 
> I'd addressed your comments except the first one.
> 
> Since RC4 is a stream cipher and RC2 is a block cipher they are handled
> slightly differently. It would be possible to re-factor this code to
> simplify it but I'd like to leave that for later as I'm under pressure
> to meet the next promotion date.
> 
> The latest webrev is at:
>  http://cr.openjdk.java.net/~vinnie/6383200/webrev.03/
> 
> 
> 
> On 08/31/12 03:05 AM, Valerie (Yu-Ching) Peng wrote:
>> Vinnie,
>> 
>> <PKCS12PBECipherCore.java>
>> 1. Is it possible to replace the CipherCore object w/ CipherSpi object
>> so to maximize the code re-use? The new code uses CipherSpi object for
>> RC4 and CipherCore for RC2. Perhaps by using CipherSpi for both RC4 and
>> RC2, we can have less code which would be easier to maintain...
>> 
>> <PBEKeyFactory>
>> 1. line 57, change the initial set size to 17 from 4?
>> 
>> <PBES2Core.java>
>> 1. the impls of the two following engineInit() methods are inconsistent,
>> i.e.
>> engineInit(int, Key, AlgorithmParameterSpec, SecureRandom) - expects
>> IvParameterSpec
>> engineInit(int, Key, AlgorithmParameters, SecureRandom) - expects
>> objects created from PBEParameterSpec
>> 2. The impl of engineGetParameters() currently returns objects created
>> from PBEParameterSpec. It should return whatever is expected in the
>> engineInit(...) calls, I'd think.
>> 
>> Will send you the rest of comments later,
>> Valerie
>> 
>> On 08/29/12 08:20, Vincent Ryan wrote:
>>> On 06/ 1/12 07:18 PM, Vincent Ryan wrote:
>>>> Hello Valerie,
>>>> 
>>>> Could you please review these changes for JEP-121:
>>>> http://cr.openjdk.java.net/~vinnie/6383200/webrev.00/
>>>> 
>>>> Thanks.
>>>> 
>>> 
>>> The latest webrev is now available at:
>>> 
>>> http://cr.openjdk.java.net/~vinnie/6383200/webrev.02/
>>> 
>>> I've incorporated review comments and made some fixes
>>> to the implementation of AES-based PBE algorithms.
>>> 
>>> Thanks.
>> 
> 




More information about the security-dev mailing list