This is an automated email from the ASF dual-hosted git repository.

svenmeier pushed a commit to branch WICKET-6864-crypt-enhancement
in repository https://gitbox.apache.org/repos/asf/wicket.git

commit 7bc47ec03cbbf273570ae72b535fa4663024737e
Author: Sven Meier <svenme...@apache.org>
AuthorDate: Sun Feb 7 11:58:34 2021 +0100

    WICKET-6864 updated crypt configuration
    
    to recommended standards
---
 .../strategy/DefaultAuthenticationStrategy.java    | 62 ++++++++++++++------
 .../util/crypt/KeyInSessionSunJceCryptFactory.java | 41 +++++++++----
 .../apache/wicket/settings/SecuritySettings.java   | 10 +++-
 .../core/request/mapper/CryptoMapperTest.java      | 26 ++-------
 .../wicket/examples/WicketExampleApplication.java  |  5 +-
 .../src/main/asciidoc/internals/pagestoring.adoc   |  8 +--
 .../util/crypt/CachingSunJceCryptFactory.java      |  2 +
 .../wicket/util/crypt/ClassCryptFactory.java       |  2 +
 .../org/apache/wicket/util/crypt/SunJceCrypt.java  | 67 +++++++++++++++++++---
 .../apache/wicket/util/crypt/SunJceCryptTest.java  |  2 +
 10 files changed, 159 insertions(+), 66 deletions(-)

diff --git 
a/wicket-core/src/main/java/org/apache/wicket/authentication/strategy/DefaultAuthenticationStrategy.java
 
b/wicket-core/src/main/java/org/apache/wicket/authentication/strategy/DefaultAuthenticationStrategy.java
index d51efbc..6567bdd 100644
--- 
a/wicket-core/src/main/java/org/apache/wicket/authentication/strategy/DefaultAuthenticationStrategy.java
+++ 
b/wicket-core/src/main/java/org/apache/wicket/authentication/strategy/DefaultAuthenticationStrategy.java
@@ -16,12 +16,14 @@
  */
 package org.apache.wicket.authentication.strategy;
 
-import org.apache.wicket.Application;
+import java.util.Random;
+import java.util.UUID;
+
 import org.apache.wicket.authentication.IAuthenticationStrategy;
 import org.apache.wicket.util.cookies.CookieDefaults;
 import org.apache.wicket.util.cookies.CookieUtils;
-import org.apache.wicket.util.crypt.CachingSunJceCryptFactory;
 import org.apache.wicket.util.crypt.ICrypt;
+import org.apache.wicket.util.crypt.SunJceCrypt;
 import org.apache.wicket.util.lang.Args;
 import org.apache.wicket.util.string.Strings;
 import org.slf4j.Logger;
@@ -30,6 +32,9 @@ import org.slf4j.LoggerFactory;
 /**
  * Wicket's default implementation of an authentication strategy. It'll 
concatenate username and
  * password, encrypt it and put it into one Cookie.
+ * <p>
+ * Note: To support automatic authentication across application restarts you 
have to use
+ * the constructor {@link 
DefaultAuthenticationStrategy#DefaultAuthenticationStrategy(String, String, 
byte[])}.
  * 
  * @author Juergen Donnerstag
  */
@@ -40,8 +45,11 @@ public class DefaultAuthenticationStrategy implements 
IAuthenticationStrategy
        /** The cookie name to store the username and password */
        protected final String cookieKey;
 
-       /** The key to use for encrypting/decrypting the cookie value  */
-       protected final String encryptionKey;
+       /**
+        * @deprecated no longer used TODO remove in Wicket 10
+        */
+       @Deprecated
+       protected final String encryptionKey = null;
 
        /** The separator used to concatenate the username and password */
        protected final String VALUE_SEPARATOR = "-sep-";
@@ -57,25 +65,50 @@ public class DefaultAuthenticationStrategy implements 
IAuthenticationStrategy
         * 
         * @param cookieKey
         *            The name of the cookie
+        *            
+        * @deprecated supply a crypt instead TODO remove in Wicket 10
         */
+       @Deprecated
        public DefaultAuthenticationStrategy(final String cookieKey)
        {
-               this(cookieKey, defaultEncryptionKey(cookieKey));
+               this(cookieKey, defaultEncryptionKey());
        }
 
-       private static String defaultEncryptionKey(String cookieKey)
+       private static String defaultEncryptionKey()
        {
-               if (Application.exists())
-               {
-                       return Application.get().getName();
-               }
-               return cookieKey;
+               return UUID.randomUUID().toString();
        }
 
+       /**
+        * @deprecated supply a crypt instead TODO remove in Wicket 10
+        */
+       @Deprecated
        public DefaultAuthenticationStrategy(final String cookieKey, final 
String encryptionKey)
        {
+               this(cookieKey, defaultCrypt(encryptionKey));
+       }
+
+       private static ICrypt defaultCrypt(String encryptionKey)
+       {
+               byte[] salt = SunJceCrypt.randomSalt();
+
+               SunJceCrypt crypt = new SunJceCrypt(salt, 1000);
+               crypt.setKey(encryptionKey);
+               return crypt;
+       }
+
+       /**
+        * Constructor
+        * 
+        * @param cookieKey
+        *            The name of the cookie
+        * @param crypt
+        *            the crypt
+        */
+       public DefaultAuthenticationStrategy(final String cookieKey, ICrypt 
crypt)
+       {
                this.cookieKey = Args.notEmpty(cookieKey, "cookieKey");
-               this.encryptionKey = Args.notEmpty(encryptionKey, 
"encryptionKey");
+               this.crypt = Args.notNull(crypt, "crypt");
        }
 
        /**
@@ -99,11 +132,6 @@ public class DefaultAuthenticationStrategy implements 
IAuthenticationStrategy
         */
        protected ICrypt getCrypt()
        {
-               if (crypt == null)
-               {
-                       CachingSunJceCryptFactory cryptFactory = new 
CachingSunJceCryptFactory(encryptionKey);
-                       crypt = cryptFactory.newCrypt();
-               }
                return crypt;
        }
 
diff --git 
a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java
 
b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java
index 556c1ff..381b83f 100644
--- 
a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java
+++ 
b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java
@@ -16,8 +16,10 @@
  */
 package org.apache.wicket.core.util.crypt;
 
+import java.io.Serializable;
 import java.security.Provider;
 import java.security.Security;
+import java.util.Random;
 import java.util.UUID;
 
 import org.apache.wicket.MetaDataKey;
@@ -38,8 +40,8 @@ import org.apache.wicket.util.lang.Args;
  */
 public class KeyInSessionSunJceCryptFactory implements ICryptFactory
 {
-       /** metadata-key used to store crypto-key in session metadata */
-       private static final MetaDataKey<String> KEY = new MetaDataKey<>()
+       /** metadata-key used to store crypt data in session metadata */
+       private static final MetaDataKey<CryptData> KEY = new MetaDataKey<>()
        {
                private static final long serialVersionUID = 1L;
        };
@@ -89,25 +91,44 @@ public class KeyInSessionSunJceCryptFactory implements 
ICryptFactory
                session.bind();
 
                // retrieve or generate encryption key from session
-               String key = session.getMetaData(KEY);
-               if (key == null)
+               CryptData data = session.getMetaData(KEY);
+               if (data == null)
                {
+                       // generate new salt
+                       byte[] salt = SunJceCrypt.randomSalt();
+                       
                        // generate new key
-                       key = session.getId() + "." + 
UUID.randomUUID().toString();
-                       session.setMetaData(KEY, key);
+                       String key = session.getId() + "." + 
UUID.randomUUID().toString();
+                       
+                       data = new CryptData(key, salt);
+                       session.setMetaData(KEY, data);
                }
 
-               // build the crypt based on session key
-               ICrypt crypt = createCrypt();
-               crypt.setKey(key);
+               // build the crypt based on session key and salt
+               SunJceCrypt crypt = new SunJceCrypt(cryptMethod, data.salt, 
1000);
+               crypt.setKey(data.key);
+               
                return crypt;
        }
 
        /**
         * @return the {@link org.apache.wicket.util.crypt.ICrypt} to use
+        * 
+        * @deprecated this method is no longer called
         */
        protected ICrypt createCrypt()
        {
-               return new SunJceCrypt(cryptMethod);
+               return null;
+       }
+       
+       private static final class CryptData implements Serializable {
+               final String key;
+               
+               final byte[] salt;
+               
+               CryptData(String key, byte[] salt) {
+                       this.key = key;
+                       this.salt = salt;
+               }
        }
 }
diff --git 
a/wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java 
b/wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java
index a618ee2..e05a061 100644
--- a/wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java
+++ b/wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java
@@ -49,8 +49,11 @@ import org.apache.wicket.util.lang.Args;
 public class SecuritySettings
 {
        /**
-        * encryption key used by default crypt factory
+        * encryption key is no longer used
+        * 
+        * @deprecated
         */
+       @Deprecated
        public static final String DEFAULT_ENCRYPTION_KEY = "WiCkEt-FRAMEwork";
 
        /** The authorization strategy. */
@@ -121,8 +124,8 @@ public class SecuritySettings
        }
 
        /**
-        * Note: Prints a warning to stderr if no factory was set and {@link 
#DEFAULT_ENCRYPTION_KEY} is
-        * used instead.
+        * Returns the {@link ICryptFactory}. If no factory is set, a {@link 
KeyInSessionSunJceCryptFactory}
+        * is used.
         * 
         * @return crypt factory used to generate crypt objects
         */
@@ -270,6 +273,7 @@ public class SecuritySettings
         *
         * @return Returns the authentication strategy.
         */
+       @SuppressWarnings("deprecation")
        public IAuthenticationStrategy getAuthenticationStrategy()
        {
                if (authenticationStrategy == null)
diff --git 
a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java
 
b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java
index 17d2273..3a89b26 100644
--- 
a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java
+++ 
b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java
@@ -24,8 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.util.function.Supplier;
-
 import org.apache.wicket.MockPage;
 import 
org.apache.wicket.core.request.handler.BookmarkableListenerRequestHandler;
 import org.apache.wicket.core.request.handler.ListenerRequestHandler;
@@ -50,10 +48,8 @@ import 
org.apache.wicket.request.mapper.info.PageComponentInfo;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
 import org.apache.wicket.request.resource.PackageResourceReference;
 import org.apache.wicket.request.resource.UrlResourceReference;
-import org.apache.wicket.settings.SecuritySettings;
-import org.apache.wicket.util.crypt.CachingSunJceCryptFactory;
 import org.apache.wicket.util.crypt.ICrypt;
-import org.apache.wicket.util.crypt.ICryptFactory;
+import org.apache.wicket.util.crypt.SunJceCrypt;
 import org.apache.wicket.util.string.StringValue;
 import org.apache.wicket.util.string.Strings;
 import org.apache.wicket.util.tester.WicketTester;
@@ -100,23 +96,11 @@ class CryptoMapperTest extends AbstractMapperTest
                WebApplication application = tester.getApplication();
                application.mountPage(MOUNTED_URL, Page1.class);
 
-               /**
-                * Use explicit crypt provider to prevent crypt warning output, 
see
-                * SecuritySettings#getCryptFactory()
-                */
-               Supplier<ICrypt> cryptProvider = new Supplier<ICrypt>()
-               {
-                       private ICryptFactory cryptFactory = new 
CachingSunJceCryptFactory(
-                               SecuritySettings.DEFAULT_ENCRYPTION_KEY);
-
-                       @Override
-                       public ICrypt get()
-                       {
-                               return cryptFactory.newCrypt();
-                       }
-               };
+               ICrypt crypt = new SunJceCrypt(new byte[]{ (byte)0x15, 
(byte)0x8c, (byte)0xa3, (byte)0x4a,
+                       (byte)0x66, (byte)0x51, (byte)0x2a, (byte)0xbc }, 17);
+               crypt.setKey("WiCkEt-FRAMEwork");
 
-               mapper = new CryptoMapper(application.getRootRequestMapper(), 
cryptProvider);
+               mapper = new CryptoMapper(application.getRootRequestMapper(), 
() -> crypt);
        }
 
        /**
diff --git 
a/wicket-examples/src/main/java/org/apache/wicket/examples/WicketExampleApplication.java
 
b/wicket-examples/src/main/java/org/apache/wicket/examples/WicketExampleApplication.java
index aa083c3..9430b48 100644
--- 
a/wicket-examples/src/main/java/org/apache/wicket/examples/WicketExampleApplication.java
+++ 
b/wicket-examples/src/main/java/org/apache/wicket/examples/WicketExampleApplication.java
@@ -22,8 +22,6 @@ import org.apache.wicket.request.cycle.IRequestCycleListener;
 import org.apache.wicket.request.cycle.RequestCycle;
 import org.apache.wicket.request.http.WebResponse;
 import org.apache.wicket.resource.CssUrlReplacer;
-import org.apache.wicket.settings.SecuritySettings;
-import org.apache.wicket.util.crypt.ClassCryptFactory;
 import org.apache.wicket.util.crypt.NoCrypt;
 
 
@@ -57,8 +55,7 @@ public abstract class WicketExampleApplication extends 
WebApplication
                // has the java security classes required by Crypt installed
                // and we want them to be able to run the examples out of the
                // box.
-               getSecuritySettings().setCryptFactory(
-                       new ClassCryptFactory(NoCrypt.class, 
SecuritySettings.DEFAULT_ENCRYPTION_KEY));
+               getSecuritySettings().setCryptFactory(() -> new NoCrypt());
 
                getDebugSettings().setDevelopmentUtilitiesEnabled(true);
                
diff --git a/wicket-user-guide/src/main/asciidoc/internals/pagestoring.adoc 
b/wicket-user-guide/src/main/asciidoc/internals/pagestoring.adoc
index 273a874..4bb85b9 100644
--- a/wicket-user-guide/src/main/asciidoc/internals/pagestoring.adoc
+++ b/wicket-user-guide/src/main/asciidoc/internals/pagestoring.adoc
@@ -9,12 +9,12 @@ image::../img/page-storage.png[]
 === IPageManager
 
 _org.apache.wicket.page.IPageManager_'s task is to manage which pages have 
been used in a request and store their last state in the backing stores, namely 
_IPageStore_.
-The default implementation _org.apache.wicket.page.PageStoreManager_ collects 
all stateful pages which have been used in the request cycle (more than one 
page can be used in a single request if for example _setResponsePage()_ or 
_RestartResponseException_ is used).
-At the end of the request all collected page instances are being stored in the 
first level cache - http session. They are stored in http session attribute 
named "_wicket:persistentPageManagerData-APPLICATION_NAME_" and passed to the 
underlying _IPageStore_.
-When the next http request comes _IPageProvider_ will ask for page with 
specific id and _PageStoreManager_ will look first in the http session and if 
no match is found then it will delegate to the IPageStore. At the end of the 
second request the http session based cache is being overwritten completely 
with the newly used page instances.
+The default implementation _org.apache.wicket.page.PageManager_ uses a chaing 
of page stores to collect all stateful pages which have been used in the 
request cycle (more than one page can be used in a single request if for 
example _setResponsePage()_ or _RestartResponseException_ is used).
+At the end of the request all collected page instances are being stored in the 
first level cache - http session. They are stored as metadata in the http 
session and passed to an underlying _IPageStore_.
+When the next http request is handled, _IPageProvider_ will ask for page with 
specific id and _PageManager_ will look first in the http session and if no 
match is found then it will delegate to any further IPageStore. At the end of 
the second request the http session based cache is being overwritten completely 
with the newly used page instances.
 
 To setup another _IPageManager_ implementation use 
_org.apache.wicket.Application.setPageManagerProvider(IPageManagerProvider)_.
-The custom _IPageManager_ implementation may or may not use 
_IPageStore/IDataStore_.
+The custom _IPageManager_ implementation may use a custom chain of 
_IPageStore_s as needed.
 
 === IPageStore
 
diff --git 
a/wicket-util/src/main/java/org/apache/wicket/util/crypt/CachingSunJceCryptFactory.java
 
b/wicket-util/src/main/java/org/apache/wicket/util/crypt/CachingSunJceCryptFactory.java
index 65db9e4..8edf89e 100644
--- 
a/wicket-util/src/main/java/org/apache/wicket/util/crypt/CachingSunJceCryptFactory.java
+++ 
b/wicket-util/src/main/java/org/apache/wicket/util/crypt/CachingSunJceCryptFactory.java
@@ -21,6 +21,8 @@ package org.apache.wicket.util.crypt;
  * all further invocations of {@link #newCrypt()}.
  * 
  * @author Igor Vaynberg (ivaynberg)
+ * 
+ * @deprecated use a lambda expression instead TODO remove in Wicket 10
  */
 public class CachingSunJceCryptFactory extends CryptFactoryCachingDecorator
 {
diff --git 
a/wicket-util/src/main/java/org/apache/wicket/util/crypt/ClassCryptFactory.java 
b/wicket-util/src/main/java/org/apache/wicket/util/crypt/ClassCryptFactory.java
index 5216c3b..e74d758 100644
--- 
a/wicket-util/src/main/java/org/apache/wicket/util/crypt/ClassCryptFactory.java
+++ 
b/wicket-util/src/main/java/org/apache/wicket/util/crypt/ClassCryptFactory.java
@@ -28,6 +28,8 @@ import org.slf4j.LoggerFactory;
  * must implement {@link ICrypt}.
  * 
  * @author Igor Vaynberg (ivaynberg)
+ * 
+ * @deprecated use a lambda expression instead TODO remove in Wicket 10
  */
 public class ClassCryptFactory implements ICryptFactory
 {
diff --git 
a/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java 
b/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java
index b702851..32bd77d 100644
--- a/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java
+++ b/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java
@@ -21,6 +21,7 @@ import java.security.NoSuchAlgorithmException;
 import java.security.spec.AlgorithmParameterSpec;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.KeySpec;
+import java.util.Random;
 
 import javax.crypto.Cipher;
 import javax.crypto.SecretKey;
@@ -44,22 +45,31 @@ public class SunJceCrypt extends AbstractCrypt
        /**
         * Iteration count used in combination with the salt to create the 
encryption key.
         */
-       private final static int COUNT = 17;
+       private final static int DEFAULT_ITERATION_COUNT = 17;
 
        /** Name of the default encryption method */
        public static final String DEFAULT_CRYPT_METHOD = "PBEWithMD5AndDES";
 
-       /** Salt */
+       /**
+        * Default salt.
+        * 
+        * @deprecated TODO remove in Wicket 10
+        */
+       @Deprecated
        public final static byte[] SALT = { (byte)0x15, (byte)0x8c, (byte)0xa3, 
(byte)0x4a,
                        (byte)0x66, (byte)0x51, (byte)0x2a, (byte)0xbc };
 
-       private static final PBEParameterSpec PARAMETER_SPEC = new 
PBEParameterSpec(SALT, COUNT);
-
        /** The name of encryption method (cipher) */
        private final String cryptMethod;
-
+       
+       private final int iterationCount;
+       
+       private final byte[] salt;
+ 
        /**
         * Constructor
+        * 
+        * @deprecated
         */
        public SunJceCrypt()
        {
@@ -67,16 +77,45 @@ public class SunJceCrypt extends AbstractCrypt
        }
 
        /**
+        * Constructor.
+        * 
+        * @param salt
+        *              salt for encryption
+        * @param iterationCount
+        *                              iteration count
+        */
+       public SunJceCrypt(byte[] salt, int iterationCount)
+       {
+               this(DEFAULT_CRYPT_METHOD, salt, iterationCount);
+       }
+
+       /**
+        * Constructor
+        *
+        * @deprecated
+        */
+       public SunJceCrypt(String cryptMethod)
+       {
+               this(cryptMethod, SALT, DEFAULT_ITERATION_COUNT);
+       }
+
+       /**
         * Constructor that uses a custom encryption method (cipher).
         * You may need to override {@link #createKeySpec()} and/or
         * {@link #createParameterSpec()} for the custom cipher.
         *
         * @param cryptMethod
         *              the name of encryption method (the cipher)
+        * @param salt
+        *              salt for encryption
+        * @param iterationCount
+        *                              iteration count
         */
-       public SunJceCrypt(String cryptMethod)
+       public SunJceCrypt(String cryptMethod, byte[] salt, int iterationCount)
        {
                this.cryptMethod = Args.notNull(cryptMethod, "Crypt method");
+               this.salt = Args.notNull(salt, "salt");
+               this.iterationCount = iterationCount;
        }
 
        /**
@@ -138,12 +177,13 @@ public class SunJceCrypt extends AbstractCrypt
                return keyFactory.generateSecret(spec);
        }
 
+       
        /**
         * @return the parameter spec to be used for the configured crypt method
         */
        protected AlgorithmParameterSpec createParameterSpec()
        {
-               return PARAMETER_SPEC;
+               return new PBEParameterSpec(salt, iterationCount);
        }
 
        /**
@@ -153,4 +193,17 @@ public class SunJceCrypt extends AbstractCrypt
        {
                return new PBEKeySpec(getKey().toCharArray());
        }
+
+       /**
+        * Create a random salt.
+        * 
+        * @return salt
+        */
+       public static byte[] randomSalt()
+       {
+               // only 8 bytes long supported
+               byte[] salt = new byte[8];
+               new Random().nextBytes(salt);
+               return salt;
+       }
 }
diff --git 
a/wicket-util/src/test/java/org/apache/wicket/util/crypt/SunJceCryptTest.java 
b/wicket-util/src/test/java/org/apache/wicket/util/crypt/SunJceCryptTest.java
index fdd3164..bed4e8f 100644
--- 
a/wicket-util/src/test/java/org/apache/wicket/util/crypt/SunJceCryptTest.java
+++ 
b/wicket-util/src/test/java/org/apache/wicket/util/crypt/SunJceCryptTest.java
@@ -34,6 +34,7 @@ public class SunJceCryptTest
        @Test
        public void defaultEncryption() throws GeneralSecurityException
        {
+               @SuppressWarnings("deprecation")
                SunJceCrypt crypt = new SunJceCrypt();
                String input = "input";
                byte[] encrypted = crypt.crypt(input.getBytes(), 
Cipher.ENCRYPT_MODE);
@@ -51,6 +52,7 @@ public class SunJceCryptTest
                boolean unlimitedStrengthJurisdictionPolicyInstalled = 
isUnlimitedStrengthJurisdictionPolicyInstalled();
                
Assumptions.assumeTrue(unlimitedStrengthJurisdictionPolicyInstalled);
 
+               @SuppressWarnings("deprecation")
                SunJceCrypt crypt = new SunJceCrypt("PBEWithMD5AndTripleDES");
                String input = "input";
                byte[] encrypted = crypt.crypt(input.getBytes(), 
Cipher.ENCRYPT_MODE);

Reply via email to