[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-13 Thread klopfdreh 
Github user klopfdreh commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r241308791
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/DefaultPageManagerProvider.java ---
@@ -167,6 +168,10 @@ protected IPageStore newPersistentStore()
Bytes maxSizePerSession = storeSettings.getMaxSizePerSession();
File fileStoreFolder = storeSettings.getFileStoreFolder();
 
-   return new DiskPageStore(application.getName(), 
fileStoreFolder, maxSizePerSession, getSerializer());
+   if (storeSettings.isEncrypted()) {
--- End diff --

Well the delegation could be done by a default implementation (for example 
as long as you return a store to delegate to). But I think you are right - the 
store configuration is rather static and because of this the stack is more 
verbose and not worth the effort to make it that configurable.


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-11 Thread svenmeier 
Github user svenmeier commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r240615707
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java 
---
@@ -1,100 +1,76 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.page;
-
-import org.apache.wicket.util.lang.Args;
-
-/**
- * Decorator for {@link IPageManager}
- * 
- * @author igor
- */
-public class PageManagerDecorator implements IPageManager
-{
-   private final IPageManager delegate;
-
-   /**
-* Constructor
-* 
-* @param delegate
-*/
-   public PageManagerDecorator(IPageManager delegate)
-   {
-   Args.notNull(delegate, "delegate");
-   this.delegate = delegate;
-   }
-
-   @Override
-   public IPageManagerContext getContext()
-   {
-   return delegate.getContext();
-   }
-
-   @Override
-   public IManageablePage getPage(int id)
-   {
-   return delegate.getPage(id);
-   }
-
-   @Override
-   public void removePage(final IManageablePage page) {
-   delegate.removePage(page);
-   }
-
-   @Override
-   public void touchPage(IManageablePage page)
-   {
-   delegate.touchPage(page);
-   }
-
-   @Override
-   public void untouchPage(IManageablePage page)
-   {
-   delegate.untouchPage(page);
-   }
-
-   @Override
-   public boolean supportsVersioning()
-   {
-   return delegate.supportsVersioning();
-   }
-
-   @Override
-   public void commitRequest()
-   {
-   delegate.commitRequest();
-   }
-
-   @Override
-   public void newSessionCreated()
-   {
-   delegate.newSessionCreated();
-   }
-
-   @Override
-   public void clear()
-   {
-   delegate.clear();
-   }
-
-   @Override
-   public void destroy()
-   {
-   delegate.destroy();
-   }
-
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.wicket.page.IManageablePage;
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * An {@link IPageStore} that delegates to another storage.
+ */
+public abstract class DelegatingPageStore implements IPageStore
+{
+   private final IPageStore delegate;
+   
+   protected DelegatingPageStore(IPageStore delegate) {
+   this.delegate = Args.notNull(delegate, "delegate");
+   }
+
+   public IPageStore getDelegate()
+   {
+   return delegate;
+   }
+   
+   /**
+* Versioning is supported depending on the delegate.
+*/
+   @Override
+   public boolean supportsVersioning()
+   {
+   return delegate.supportsVersioning();
+   }
+   
+   @Override
+   public void addPage(IPageContext context, IManageablePage page) {
--- End diff --

You're right, I'll do a reformat once we're ready to move this 
implemenation to master.


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-11 Thread svenmeier 
Github user svenmeier commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r240615436
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/pageStore/disk/NestedFolders.java 
---
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore.disk;
+
+import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.wicket.util.file.Files;
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * Helps creating nested folders.
+ * 
+ * @author svenmeier
+ */
+public class NestedFolders
+{
+   private final File base;
+   
+   /**
+* Create folders in the given base folder.
+* 
+* @param base base has to be a folder
+*/
+   public NestedFolders(File base) {
+   this.base = Args.notNull(base, "base");
+   }
+   
+   public File getBase()
+   {
+   return base;
+   }
+   
+   /**
+* Get a nested folder for the given name.
+* 
+* @param name name 
+* @param create
+* @return
+*/
+   public File get(String name, final boolean create) {
+   name = name.replace('*', '_');
+   name = name.replace('/', '_');
--- End diff --

I've moved that code from the old DiskDataStore implementation into this 
utility class. I think you're right, backslash should probably be escaped too.


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-11 Thread svenmeier 
Github user svenmeier commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r240608325
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/DefaultPageManagerProvider.java ---
@@ -167,6 +168,10 @@ protected IPageStore newPersistentStore()
Bytes maxSizePerSession = storeSettings.getMaxSizePerSession();
File fileStoreFolder = storeSettings.getFileStoreFolder();
 
-   return new DiskPageStore(application.getName(), 
fileStoreFolder, maxSizePerSession, getSerializer());
+   if (storeSettings.isEncrypted()) {
--- End diff --

This is a question of CoR vs a collection, like 
https://stackoverflow.com/questions/1055383/what-are-the-advantages-of-chain-of-responsibility-vs-lists-of-classes

I prefer the former, since it gives more power to each store, e.g. 
GroupingPageStore has to control delegation to the next store.
I don't think configurability is a huge issue, since page stores are pretty 
static once set up.


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-09 Thread bitstorm 
Github user bitstorm commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r240041611
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/pageStore/DelegatingPageStore.java 
---
@@ -1,100 +1,76 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.page;
-
-import org.apache.wicket.util.lang.Args;
-
-/**
- * Decorator for {@link IPageManager}
- * 
- * @author igor
- */
-public class PageManagerDecorator implements IPageManager
-{
-   private final IPageManager delegate;
-
-   /**
-* Constructor
-* 
-* @param delegate
-*/
-   public PageManagerDecorator(IPageManager delegate)
-   {
-   Args.notNull(delegate, "delegate");
-   this.delegate = delegate;
-   }
-
-   @Override
-   public IPageManagerContext getContext()
-   {
-   return delegate.getContext();
-   }
-
-   @Override
-   public IManageablePage getPage(int id)
-   {
-   return delegate.getPage(id);
-   }
-
-   @Override
-   public void removePage(final IManageablePage page) {
-   delegate.removePage(page);
-   }
-
-   @Override
-   public void touchPage(IManageablePage page)
-   {
-   delegate.touchPage(page);
-   }
-
-   @Override
-   public void untouchPage(IManageablePage page)
-   {
-   delegate.untouchPage(page);
-   }
-
-   @Override
-   public boolean supportsVersioning()
-   {
-   return delegate.supportsVersioning();
-   }
-
-   @Override
-   public void commitRequest()
-   {
-   delegate.commitRequest();
-   }
-
-   @Override
-   public void newSessionCreated()
-   {
-   delegate.newSessionCreated();
-   }
-
-   @Override
-   public void clear()
-   {
-   delegate.clear();
-   }
-
-   @Override
-   public void destroy()
-   {
-   delegate.destroy();
-   }
-
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.wicket.page.IManageablePage;
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * An {@link IPageStore} that delegates to another storage.
+ */
+public abstract class DelegatingPageStore implements IPageStore
+{
+   private final IPageStore delegate;
+   
+   protected DelegatingPageStore(IPageStore delegate) {
+   this.delegate = Args.notNull(delegate, "delegate");
+   }
+
+   public IPageStore getDelegate()
+   {
+   return delegate;
+   }
+   
+   /**
+* Versioning is supported depending on the delegate.
+*/
+   @Override
+   public boolean supportsVersioning()
+   {
+   return delegate.supportsVersioning();
+   }
+   
+   @Override
+   public void addPage(IPageContext context, IManageablePage page) {
--- End diff --

Just a minor issue with curly brackets indentation which should be on a new 
line.


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-07 Thread klopfdreh 
Github user klopfdreh commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r239994141
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/pageStore/disk/NestedFolders.java 
---
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore.disk;
+
+import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.wicket.util.file.Files;
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * Helps creating nested folders.
+ * 
+ * @author svenmeier
+ */
+public class NestedFolders
+{
+   private final File base;
+   
+   /**
+* Create folders in the given base folder.
+* 
+* @param base base has to be a folder
+*/
+   public NestedFolders(File base) {
+   this.base = Args.notNull(base, "base");
+   }
+   
+   public File getBase()
+   {
+   return base;
+   }
+   
+   /**
+* Get a nested folder for the given name.
+* 
+* @param name name 
+* @param create
+* @return
+*/
+   public File get(String name, final boolean create) {
+   name = name.replace('*', '_');
+   name = name.replace('/', '_');
--- End diff --

What about \ in windows?


---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-12-07 Thread klopfdreh 
Github user klopfdreh commented on a diff in the pull request:

https://github.com/apache/wicket/pull/283#discussion_r239994086
  
--- Diff: 
wicket-core/src/main/java/org/apache/wicket/DefaultPageManagerProvider.java ---
@@ -167,6 +168,10 @@ protected IPageStore newPersistentStore()
Bytes maxSizePerSession = storeSettings.getMaxSizePerSession();
File fileStoreFolder = storeSettings.getFileStoreFolder();
 
-   return new DiskPageStore(application.getName(), 
fileStoreFolder, maxSizePerSession, getSerializer());
+   if (storeSettings.isEncrypted()) {
--- End diff --

Maybe it would be nice to not make the chain configurable by a field in 
general but to have something more spring like. (Like in case of spring Filters)

They implemented it with a List in which you can add filter at various 
places and they delegate to each other. You can add new filters in between like 
filters.add(0,new MyFilter(...));

WDYT?


---

Re: [GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-10 Thread Andrea Del Bene 
Loosing code is always good, but doing it right it's harder than loosing
weight :-D

On Tue, Jul 10, 2018 at 11:08 AM, Tobias Soloschenko <
tobiassolosche...@googlemail.com.invalid> wrote:

>
> > Am 10.07.2018 um 09:54 schrieb Emond Papegaaij <
> emond.papega...@topicus.nl>:
> >
> >> 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
> > :)
> >
>
> :-O
>
> >>> 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
> >
>
> kind regards
>
> Tobias




-- 
Andrea Del Bene.
Apache Wicket committer.

Re: [GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-10 Thread Tobias Soloschenko 


> Am 10.07.2018 um 09:54 schrieb Emond Papegaaij :
> 
>> 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 
> :)
> 

:-O

>>> 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
> 

kind regards

Tobias

Re: [GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-10 Thread Emond Papegaaij 
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

Re: [GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-09 Thread Sven Meier 

Hi Edmund,

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.


> 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.


Have fun
Sven


generated key:

Am 09.07.2018 um 17:36 schrieb Emond Papegaaij:

Hi Sven,

Thanks for the work you have put into this. However, this is an enormous
amount of code to review so this will take some time. I'll try to get this
done this week, but I can't promise anything.

For now, I can at least comment on CryptingPageStore.java. The ciphers used by
this store are both obsolete and not applicable to this use case. It currently
uses a Password Based Encryption (PBE) and a very insecure one (single DES). I
would recommend changing the code to using AES/CBC/PKCS5Padding with a
generated key:

SecureRandom rnd = SecureRandom.getInstance("SHA1PRNG", "SUN");
KeyGenerator keyGen = KeyGenerator.getInstance("AES");
keyGen.init(256, rnd);
SecretKey key = keyGen.generateKey();

This key is serializable and can be stored in the session directly. Make sure
that rnd is a SecureRandom with a strong algorithm, and very importantly: it
must be reused. Due to application specific demands on this SecureRandom (such
as automatic reseeding), I think users must be able to provide their own. To
encrypt a page, use:

Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.ENCRYPT_MODE, key, rnd);
AlgorithmParameters params = cipher.getParameters();
byte[] iv = params.getParameterSpec(IvParameterSpec.class).getIV();
byte[] ciphertext = cipher.doFinal(plainpagebytes);
byte[] result = Bytes.concat(iv, ciphertext);

Use the same SecureRandom for the cipher.init. This will be used to generate
the IV.  To decrypt the page, use:

byte[] iv = new byte[16];
byte[] ciphertext = new byte[cipherInput.length - 16];
System.arraycopy(cipherInput, 0, iv, 0, iv.length);
System.arraycopy(cipherInput, 16, ciphertext, 0, ciphertext.length);

Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv));
byte[] plainpagebytes = cipher.doFinal(ciphertext);

This should give a much stronger and faster algorithm than using
PBEWithMD5AndDES.

Best regards,
Emond

On maandag 9 juli 2018 16:45:43 CEST svenmeier wrote:

GitHub user svenmeier opened a pull request:

 https://github.com/apache/wicket/pull/283

 Wicket-6563 page store implementation

 Basically I propose to
 *unify IPageStore with IDateStore
 *allow all IPageStore implementations to use Request/Session data (see
DiskPageStore, GroupingPageStore and CryptingPageStore) as needed *cut down
PageStoreManager to a very simple manager with a chain of stores, each
offering different solutions.

 I appreciate your feedback.

You can merge this pull request into a Git repository by running:

 $ git pull https://github.com/apache/wicket WICKET-6563

Alternatively you can review and apply these changes as the patch at:

 https://github.com/apache/wicket/pull/283.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

 This closes #283


commit bcf76f517310ac5d27a2092595c3a925c3973067
Author: Sven Meier 
Date:   2018-06-25T15:19:41Z

 WICKET-6563 new IPageStore implementation

commit a3604f7c359f9bbd2df03d57831c3f0d838ef521
Author: Sven Meier 
Date:   2018-07-03T18:18:10Z

 WICKET-6563 allow passing of SerializedPage

 between page stores

commit ad1f9b88ce8412e2b8eec5de72073b9688deb3bb
Author: Sven Meier 
Date:   2018-07-03T21:52:10Z

 WICKET-6563 javadoc and test

 for SerializingPageStore; keep page type in serializedpage

commit 2f70db06d3aa7f1ee07c96e1287507fe0021f18e
Author: Sven Meier 
Date:   2018-07-05T18:09:49Z

 WICKET-6563 crypt page store

commit 7e9c7e5166fda6692163c9551dc56d3177acfe6d
Author: Sven Meier 
Date:   2018-07-07T18:37:54Z

 WICKET-6563 IPageContext set synchronization

 prevent multiple threads from settings data into IPageContext
 concurrently

commit fd7b26bac6f72d5ebc647f490263c41f169faec1
Author: Sven Meier 
Date:   2018-07-09T08:54:57Z

 WICKET-6563 improved test




---

Re: [GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-09 Thread Emond Papegaaij 
Hi Sven,

Thanks for the work you have put into this. However, this is an enormous 
amount of code to review so this will take some time. I'll try to get this 
done this week, but I can't promise anything.

For now, I can at least comment on CryptingPageStore.java. The ciphers used by 
this store are both obsolete and not applicable to this use case. It currently 
uses a Password Based Encryption (PBE) and a very insecure one (single DES). I 
would recommend changing the code to using AES/CBC/PKCS5Padding with a 
generated key:

SecureRandom rnd = SecureRandom.getInstance("SHA1PRNG", "SUN");
KeyGenerator keyGen = KeyGenerator.getInstance("AES");
keyGen.init(256, rnd);
SecretKey key = keyGen.generateKey();

This key is serializable and can be stored in the session directly. Make sure 
that rnd is a SecureRandom with a strong algorithm, and very importantly: it 
must be reused. Due to application specific demands on this SecureRandom (such 
as automatic reseeding), I think users must be able to provide their own. To 
encrypt a page, use:

Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.ENCRYPT_MODE, key, rnd);
AlgorithmParameters params = cipher.getParameters();
byte[] iv = params.getParameterSpec(IvParameterSpec.class).getIV();
byte[] ciphertext = cipher.doFinal(plainpagebytes);
byte[] result = Bytes.concat(iv, ciphertext);

Use the same SecureRandom for the cipher.init. This will be used to generate 
the IV.  To decrypt the page, use:

byte[] iv = new byte[16];
byte[] ciphertext = new byte[cipherInput.length - 16];
System.arraycopy(cipherInput, 0, iv, 0, iv.length);
System.arraycopy(cipherInput, 16, ciphertext, 0, ciphertext.length);

Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv));
byte[] plainpagebytes = cipher.doFinal(ciphertext);

This should give a much stronger and faster algorithm than using 
PBEWithMD5AndDES.

Best regards,
Emond

On maandag 9 juli 2018 16:45:43 CEST svenmeier wrote:
> GitHub user svenmeier opened a pull request:
> 
> https://github.com/apache/wicket/pull/283
> 
> Wicket-6563 page store implementation
> 
> Basically I propose to
> *unify IPageStore with IDateStore
> *allow all IPageStore implementations to use Request/Session data (see
> DiskPageStore, GroupingPageStore and CryptingPageStore) as needed *cut down
> PageStoreManager to a very simple manager with a chain of stores, each
> offering different solutions.
> 
> I appreciate your feedback.
> 
> You can merge this pull request into a Git repository by running:
> 
> $ git pull https://github.com/apache/wicket WICKET-6563
> 
> Alternatively you can review and apply these changes as the patch at:
> 
> https://github.com/apache/wicket/pull/283.patch
> 
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
> 
> This closes #283
> 
> 
> commit bcf76f517310ac5d27a2092595c3a925c3973067
> Author: Sven Meier 
> Date:   2018-06-25T15:19:41Z
> 
> WICKET-6563 new IPageStore implementation
> 
> commit a3604f7c359f9bbd2df03d57831c3f0d838ef521
> Author: Sven Meier 
> Date:   2018-07-03T18:18:10Z
> 
> WICKET-6563 allow passing of SerializedPage
> 
> between page stores
> 
> commit ad1f9b88ce8412e2b8eec5de72073b9688deb3bb
> Author: Sven Meier 
> Date:   2018-07-03T21:52:10Z
> 
> WICKET-6563 javadoc and test
> 
> for SerializingPageStore; keep page type in serializedpage
> 
> commit 2f70db06d3aa7f1ee07c96e1287507fe0021f18e
> Author: Sven Meier 
> Date:   2018-07-05T18:09:49Z
> 
> WICKET-6563 crypt page store
> 
> commit 7e9c7e5166fda6692163c9551dc56d3177acfe6d
> Author: Sven Meier 
> Date:   2018-07-07T18:37:54Z
> 
> WICKET-6563 IPageContext set synchronization
> 
> prevent multiple threads from settings data into IPageContext
> concurrently
> 
> commit fd7b26bac6f72d5ebc647f490263c41f169faec1
> Author: Sven Meier 
> Date:   2018-07-09T08:54:57Z
> 
> WICKET-6563 improved test
> 
> 
> 
> 
> ---

[GitHub] wicket pull request #283: Wicket-6563 page store implementation

2018-07-09 Thread svenmeier 
GitHub user svenmeier opened a pull request:

https://github.com/apache/wicket/pull/283

Wicket-6563 page store implementation

Basically I propose to
*unify IPageStore with IDateStore
*allow all IPageStore implementations to use Request/Session data (see 
DiskPageStore, GroupingPageStore and CryptingPageStore) as needed
*cut down PageStoreManager to a very simple manager with a chain of stores, 
each offering different solutions.

I appreciate your feedback.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/wicket WICKET-6563

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/wicket/pull/283.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #283


commit bcf76f517310ac5d27a2092595c3a925c3973067
Author: Sven Meier 
Date:   2018-06-25T15:19:41Z

WICKET-6563 new IPageStore implementation

commit a3604f7c359f9bbd2df03d57831c3f0d838ef521
Author: Sven Meier 
Date:   2018-07-03T18:18:10Z

WICKET-6563 allow passing of SerializedPage

between page stores

commit ad1f9b88ce8412e2b8eec5de72073b9688deb3bb
Author: Sven Meier 
Date:   2018-07-03T21:52:10Z

WICKET-6563 javadoc and test

for SerializingPageStore; keep page type in serializedpage

commit 2f70db06d3aa7f1ee07c96e1287507fe0021f18e
Author: Sven Meier 
Date:   2018-07-05T18:09:49Z

WICKET-6563 crypt page store

commit 7e9c7e5166fda6692163c9551dc56d3177acfe6d
Author: Sven Meier 
Date:   2018-07-07T18:37:54Z

WICKET-6563 IPageContext set synchronization

prevent multiple threads from settings data into IPageContext
concurrently

commit fd7b26bac6f72d5ebc647f490263c41f169faec1
Author: Sven Meier 
Date:   2018-07-09T08:54:57Z

WICKET-6563 improved test




---