On maandag 9 juli 2018 21:21:42 CEST Sven Meier wrote:
> Hi Emond,
> 
> many thanks for your first feedback.
> 
>  > this is an enormous amount of code to review so this will take some time
> 
> Actually its mostly the old code squeezed into new classes. But we can
> take all the time we want to work on it.

Well, you did lose over 1100 lines of code somewhere. I guess it was all trash 
:)
 
>  > I would recommend changing the code to using AES/CBC/PKCS5Padding with
> 
> Sure, my first try was just using what popped up first in a Google
> search :P.
> It was just a nice opportunity to show how the new store chain is
> capable of adding encryption without much hassle.
> 
> I've changed the crypt implementation now, I hope it performs better
> that way.

This new implementation is much better. However, I would keep the SecureRandom 
out of the user session. Constructing a SecureRandom is very expensive wrt 
system entropy. Constructing one for every session will deplete your system 
entropy in minutes on systems with heavy traffic. I would change the ICrypter 
interface to take the SecureRandom as parameter to encrypt and add a init 
method (which takes SecureRandom as well) to generate the key. That way you 
can store the SecureRandom in CryptingPageStore.

Btw, this will also close WICKET-6559

Emond


Reply via email to