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