Re: Code Review Request for 6996769: support AEAD ciphers

2013-01-07 Thread Valerie (Yu-Ching) Peng
FYI, I have filed 8005830 Enhance GCM parameters generation Thanks, Valerie On 01/03/13 20:14, Michael StJohns wrote: Hi Valerie - Comments in line. At 09:17 PM 1/3/2013, Valerie (Yu-Ching) Peng wrote: Michael, I thought this some more. I think a cleaner way to do what you wanted may be

Re: Code Review Request for 6996769: support AEAD ciphers

2013-01-03 Thread Valerie (Yu-Ching) Peng
Michael, I thought this some more. I think a cleaner way to do what you wanted may be achieved by adding an AlgorithmParameterGenerator impl for GCM. I've read through your email multiple times but yet I can't think of a good way to fit all the functionality that you wanted into the

Re: Code Review Request for 6996769: support AEAD ciphers

2013-01-03 Thread Valerie (Yu-Ching) Peng
Max, Forgot to mention that the latest webrev has been updated: http://cr.openjdk.java.net/~valeriep/6996769/webrev.05/ I shall proceed with putbacks if you have no further comments tomorrow. Thanks, Valerie On 01/03/13 18:17, Valerie (Yu-Ching) Peng wrote: Michael, I thought this some

Re: Code Review Request for 6996769: support AEAD ciphers

2013-01-03 Thread Michael StJohns
At 09:17 PM 1/3/2013, Valerie (Yu-Ching) Peng wrote: Michael, I thought this some more. I think a cleaner way to do what you wanted may be achieved by adding an AlgorithmParameterGenerator impl for GCM. *sigh* I forgot to answer this. I think this would probably work too. But I agree that

Re: Code Review Request for 6996769: support AEAD ciphers

2012-12-17 Thread Michael StJohns
Hi Valerie - Comments in line. At 08:32 PM 12/14/2012, Valerie (Yu-Ching) Peng wrote: Mike, Thanks for the comments... With the current API, it is still possible to have the IV generated at the provider level by calling Cipher.init(...) without GCM parameters. This means that provider will

Re: Code Review Request for 6996769: support AEAD ciphers

2012-12-13 Thread Michael StJohns
Sorry for the late comment - You might want to consider section 9.1, first paragraph of SP800-38D which defines the GCM mode. Basically, for FIPS validated implementations, to prevent accidental reuse, the IV needs to be generated inside the cryptographic boundary using one of the defined

Re: Code Review Request for 6996769: support AEAD ciphers

2012-12-07 Thread Valerie (Yu-Ching) Peng
Max, The webrev has been updated so that different key + iv values have to be used for AES/GCM encryption. Latest version at: http://cr.openjdk.java.net/~valeriep/6996769/webrev.03/ Please review and send me comments. Thanks! Valerie On 11/07/12 21:50, Valerie (Yu-Ching) Peng wrote: Max,

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-07 Thread Weijun Wang
Hi Valerie Test4512704.java: Why not test AES/CBC/PKCS5Padding anymore? TestKATForGCM.java: Is there a URL for the test data? GaloisCounterMode, GCTR, and GHASH are good. Thanks Max On 11/06/2012 04:48 PM, Weijun Wang wrote: CipherCore.java: 79 * update() must buffer this

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-07 Thread Valerie (Yu-Ching) Peng
Hi, Max, Thanks for the prompt review! On 11/07/12 02:41, Weijun Wang wrote: Hi Valerie Test4512704.java: Why not test AES/CBC/PKCS5Padding anymore? This particular test is not really padding-relevant, so I just switching to test AES/CBC/NoPadding instead, since GCM mode requires

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-07 Thread Valerie (Yu-Ching) Peng
Hi, Max, Please find comments in line: On 11/06/12 00:48, Weijun Wang wrote: 380 AlgorithmParameters getParameters(String algName) The updated code does not return null anymore. Is there some other reason out of this patch? The init() method below seems to support null for all modes.

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-07 Thread Xuelei Fan
I have a concern about the performance. Not a big problem, but may be nice to consider it. Unlike CBC ciphers, GCM based ciphers need to update the IV for every encryption/decryption operation. As means that the Cipher.init() is required to call in every operation: Key key = // the key used

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-07 Thread Valerie (Yu-Ching) Peng
Max, Update: I removed the block (starting line 580 in CipherCore.java) for handling RC2 since it's never used. It turns out that the current impl in RC2Cipher always convert the AlgorithmParameters object to RC2ParameterSpec and only uses CipherCore.init(..., AlgorithmParameterSpec,...)

Re: Code Review Request for 6996769: support AEAD ciphers

2012-11-06 Thread Weijun Wang
CipherCore.java: 79 * update() must buffer this many bytes before before starting Dup before. 380 AlgorithmParameters getParameters(String algName) The updated code does not return null anymore. Is there some other reason out of this patch? The init() method below seems to support

Code Review Request for 6996769: support AEAD ciphers

2012-11-02 Thread Valerie (Yu-Ching) Peng
Brad or Max, Can either of you review my changes for the following RFE? 6996769: support AEAD ciphers This is the JCE part of changes for the EFP Support AEAD CipherSuites. The webrev is at: http://cr.openjdk.java.net/~valeriep/6996769/webrev.00/ I included IBM copyright in files where some