martin-g commented on a change in pull request #462: URL: https://github.com/apache/wicket/pull/462#discussion_r591241937
########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractKeyInSessionCryptFactory.java ########## @@ -0,0 +1,91 @@ +/* + * 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.core.util.crypt; + +import org.apache.wicket.MetaDataKey; +import org.apache.wicket.Session; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.crypt.ICryptFactory; + +import java.io.Serializable; + + +/** + * Base class to implement crypt factories that store crypt into user session. Note that the use of + * this crypt factory will result in an immediate creation of a http session. + * + * @author andrea del bene + * + * @param <T> + * the type for the secret key. + */ +public abstract class AbstractKeyInSessionCryptFactory<T extends Serializable> + implements + ICryptFactory +{ + /** metadata-key used to store crypto-key in session metadata */ + private static final MetaDataKey<Serializable> KEY = new MetaDataKey<Serializable>() + { + private static final long serialVersionUID = 1L; + }; + + public AbstractKeyInSessionCryptFactory() + { + super(); + } + + /** + * Creates a new crypt for the current user session. If no user session is available, a new one + * is created. + * + * @return + */ + @Override + @SuppressWarnings("unchecked") + public ICrypt newCrypt() + { + Session session = Session.get(); + session.bind(); + + // retrieve or generate encryption key from session + Serializable key = session.getMetaData(KEY); Review comment: s/Serializable/T/ ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractKeyInSessionCryptFactory.java ########## @@ -0,0 +1,91 @@ +/* + * 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.core.util.crypt; + +import org.apache.wicket.MetaDataKey; +import org.apache.wicket.Session; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.crypt.ICryptFactory; + +import java.io.Serializable; + + +/** + * Base class to implement crypt factories that store crypt into user session. Note that the use of + * this crypt factory will result in an immediate creation of a http session. + * + * @author andrea del bene + * + * @param <T> + * the type for the secret key. + */ +public abstract class AbstractKeyInSessionCryptFactory<T extends Serializable> + implements + ICryptFactory +{ + /** metadata-key used to store crypto-key in session metadata */ + private static final MetaDataKey<Serializable> KEY = new MetaDataKey<Serializable>() Review comment: MetaDataKey<T> ########## File path: wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java ########## @@ -49,8 +49,11 @@ public class SecuritySettings { /** - * encryption key used by default crypt factory + * encryption key is no longer used + * + * @deprecated */ + @Deprecated Review comment: forRemoval ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AESCrypt.java ########## @@ -0,0 +1,112 @@ +/* + * 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.util.crypt; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.SecretKey; +import javax.crypto.spec.IvParameterSpec; + +/** + * AES based {@link ICrypt} encrypt and decrypt strings such as passwords or URL segments. + * Based on http://stackoverflow.com/a/992413 + * + * @see ICrypt + */ +class AESCrypt extends AbstractJceCrypt +{ + + private final SecretKey secretKey; + private final String algorithm; + private final int ivSize; + + + /** + * Constructor + * + * @param secretKey + * The {@link SecretKey} to use to initialize the {@link Cipher}. + */ + public AESCrypt(SecretKey secretKey) Review comment: The class is package-private. Should it be `public` or the constructor should be `package-private` too ? ########## File path: wicket-util/src/test/java/org/apache/wicket/util/crypt/AESCryptTest.java ########## @@ -0,0 +1,55 @@ +/* + * 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.util.crypt; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +import java.security.GeneralSecurityException; + +import javax.crypto.SecretKey; + +public class AESCryptTest +{ + @Test + public void encrypDecrypt() throws GeneralSecurityException + { + boolean unlimitedStrengthJurisdictionPolicyInstalled = SunJceCryptTest + .isUnlimitedStrengthJurisdictionPolicyInstalled(); + Assumptions.assumeTrue(unlimitedStrengthJurisdictionPolicyInstalled); + + SecretKey secretKey = CipherUtils.generatePBEKey( + "myWeakPassword", "PBKDF2WithHmacSHA1", "AES", + CipherUtils.randomByteArray(16), 65536, 256); + + AbstractJceCrypt crypt = new AESCrypt(secretKey); + + String input1 = "input1"; + String encrypted = crypt.encryptUrlSafe(input1); + + String input2 = "input2"; Review comment: let's use some UTF-8 characters here ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AESCrypt.java ########## @@ -0,0 +1,112 @@ +/* + * 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.util.crypt; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.SecretKey; +import javax.crypto.spec.IvParameterSpec; + +/** + * AES based {@link ICrypt} encrypt and decrypt strings such as passwords or URL segments. + * Based on http://stackoverflow.com/a/992413 + * + * @see ICrypt + */ +class AESCrypt extends AbstractJceCrypt +{ + + private final SecretKey secretKey; + private final String algorithm; + private final int ivSize; + + + /** + * Constructor + * + * @param secretKey + * The {@link SecretKey} to use to initialize the {@link Cipher}. + */ + public AESCrypt(SecretKey secretKey) + { + this(secretKey, "AES/CBC/PKCS5Padding", 16); + } + + /** + * Constructor + * + * @param secretKey + * The {@link SecretKey} to use to initialize the {@link Cipher}. + * @param algorithm + * The cipher algorithm to use, for example "AES/CBC/PKCS5Padding". + * @param ivSize + * The size of the Initialization Vector to use with the cipher. + */ + private AESCrypt(SecretKey secretKey, String algorithm, int ivSize) + { + this.secretKey = secretKey; Review comment: I think it would be good if we use `Args.non***` checks here. ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java ########## @@ -82,32 +76,53 @@ public KeyInSessionSunJceCryptFactory(String cryptMethod) } } - @Override - public ICrypt newCrypt() - { - Session session = Session.get(); - session.bind(); - - // retrieve or generate encryption key from session - String key = session.getMetaData(KEY); - if (key == null) - { - // generate new key - key = session.getId() + "." + UUID.randomUUID().toString(); - session.setMetaData(KEY, key); - } - - // build the crypt based on session key - ICrypt crypt = createCrypt(); - crypt.setKey(key); - return crypt; - } - /** * @return the {@link org.apache.wicket.util.crypt.ICrypt} to use + * + * @deprecated this method is no longer called TODO remove in Wicket 10 */ + @Deprecated(forRemoval = true) protected ICrypt createCrypt() { - return new SunJceCrypt(cryptMethod); + return null; + } + + @Override + protected CryptData generateKey(Session session) + { + // generate new salt + byte[] salt = SunJceCrypt.randomSalt(); + + // generate new key + String key = session.getId() + "." + UUID.randomUUID().toString(); + + return new CryptData(key, salt); + } + + @Override + protected ICrypt createCrypt(CryptData keyParams) + { + SunJceCrypt crypt = new SunJceCrypt(cryptMethod, keyParams.salt, 1000); + crypt.setKey(keyParams.key); + + return crypt; + } + + static final class CryptData implements Serializable + { + /** Review comment: Empty javadoc. Remove it ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractKeyInSessionCryptFactory.java ########## @@ -0,0 +1,91 @@ +/* + * 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.core.util.crypt; + +import org.apache.wicket.MetaDataKey; +import org.apache.wicket.Session; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.crypt.ICryptFactory; + +import java.io.Serializable; + + +/** + * Base class to implement crypt factories that store crypt into user session. Note that the use of + * this crypt factory will result in an immediate creation of a http session. + * + * @author andrea del bene + * + * @param <T> + * the type for the secret key. + */ +public abstract class AbstractKeyInSessionCryptFactory<T extends Serializable> Review comment: s/Serializable/IClusterable/ ?! ########## File path: wicket-examples/src/main/java/org/apache/wicket/examples/WicketExampleApplication.java ########## @@ -57,8 +55,7 @@ protected void init() // 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()); Review comment: NoCrypt::new ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java ########## @@ -82,32 +76,53 @@ public KeyInSessionSunJceCryptFactory(String cryptMethod) } } - @Override - public ICrypt newCrypt() - { - Session session = Session.get(); - session.bind(); - - // retrieve or generate encryption key from session - String key = session.getMetaData(KEY); - if (key == null) - { - // generate new key - key = session.getId() + "." + UUID.randomUUID().toString(); - session.setMetaData(KEY, key); - } - - // build the crypt based on session key - ICrypt crypt = createCrypt(); - crypt.setKey(key); - return crypt; - } - /** * @return the {@link org.apache.wicket.util.crypt.ICrypt} to use + * + * @deprecated this method is no longer called TODO remove in Wicket 10 */ + @Deprecated(forRemoval = true) protected ICrypt createCrypt() { - return new SunJceCrypt(cryptMethod); + return null; + } + + @Override + protected CryptData generateKey(Session session) + { + // generate new salt + byte[] salt = SunJceCrypt.randomSalt(); + + // generate new key + String key = session.getId() + "." + UUID.randomUUID().toString(); + + return new CryptData(key, salt); + } + + @Override + protected ICrypt createCrypt(CryptData keyParams) + { + SunJceCrypt crypt = new SunJceCrypt(cryptMethod, keyParams.salt, 1000); + crypt.setKey(keyParams.key); + + return crypt; + } + + static final class CryptData implements Serializable Review comment: IClusterable ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractKeyInSessionCryptFactory.java ########## @@ -0,0 +1,91 @@ +/* + * 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.core.util.crypt; + +import org.apache.wicket.MetaDataKey; +import org.apache.wicket.Session; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.crypt.ICryptFactory; + +import java.io.Serializable; + + +/** + * Base class to implement crypt factories that store crypt into user session. Note that the use of + * this crypt factory will result in an immediate creation of a http session. + * + * @author andrea del bene + * + * @param <T> + * the type for the secret key. + */ +public abstract class AbstractKeyInSessionCryptFactory<T extends Serializable> + implements + ICryptFactory +{ + /** metadata-key used to store crypto-key in session metadata */ + private static final MetaDataKey<Serializable> KEY = new MetaDataKey<Serializable>() + { + private static final long serialVersionUID = 1L; + }; + + public AbstractKeyInSessionCryptFactory() + { + super(); + } + + /** + * Creates a new crypt for the current user session. If no user session is available, a new one + * is created. + * + * @return + */ + @Override + @SuppressWarnings("unchecked") + public ICrypt newCrypt() + { + Session session = Session.get(); + session.bind(); + + // retrieve or generate encryption key from session + Serializable key = session.getMetaData(KEY); + if (key == null) + { + // generate new key + key = generateKey(session); + session.setMetaData(KEY, key); + } + + // build the crypt based on session key + ICrypt crypt = createCrypt((T)key); Review comment: if the above changes are applied then there is no need to cast here and to annotate with `@SuppressWarnings("unchecked")` ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) Review comment: What is `transformation`? This name is not self-explanatory (at least to me) ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); + crypter.init(opMode, secretKey, params); + return crypter; + } catch (InvalidKeyException | InvalidAlgorithmParameterException + | NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new RuntimeException(e); + } + } + + /** + * Decrypts a string into a string. + * + * @param text + * text to decrypt + * @return the decrypted text + */ + @Override + public final String decryptUrlSafe(final String text) + { + try + { + byte[] decoded = java.util.Base64.getUrlDecoder().decode(text); + return new String(decryptByteArray(decoded), CHARACTER_ENCODING); + } + catch (Exception ex) + { + log.debug("Error decoding text: " + text, ex); + return null; + } + } + + /** + * Encrypt a string into a string using URL safe Base64 encoding. + * + * @param plainText + * text to encrypt + * @return encrypted string + */ + @Override + public final String encryptUrlSafe(final String plainText) + { + byte[] encrypted = encryptStringToByteArray(plainText); Review comment: indentation ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); + crypter.init(opMode, secretKey, params); + return crypter; + } catch (InvalidKeyException | InvalidAlgorithmParameterException + | NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new RuntimeException(e); + } + } + + /** + * Decrypts a string into a string. + * + * @param text + * text to decrypt + * @return the decrypted text + */ + @Override + public final String decryptUrlSafe(final String text) + { + try + { + byte[] decoded = java.util.Base64.getUrlDecoder().decode(text); + return new String(decryptByteArray(decoded), CHARACTER_ENCODING); + } + catch (Exception ex) + { + log.debug("Error decoding text: " + text, ex); Review comment: Use slf4j's {} instead of String concatenation ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/KeyInSessionSunJceCryptFactory.java ########## @@ -82,32 +76,53 @@ public KeyInSessionSunJceCryptFactory(String cryptMethod) } } - @Override - public ICrypt newCrypt() - { - Session session = Session.get(); - session.bind(); - - // retrieve or generate encryption key from session - String key = session.getMetaData(KEY); - if (key == null) - { - // generate new key - key = session.getId() + "." + UUID.randomUUID().toString(); - session.setMetaData(KEY, key); - } - - // build the crypt based on session key - ICrypt crypt = createCrypt(); - crypt.setKey(key); - return crypt; - } - /** * @return the {@link org.apache.wicket.util.crypt.ICrypt} to use + * + * @deprecated this method is no longer called TODO remove in Wicket 10 */ + @Deprecated(forRemoval = true) protected ICrypt createCrypt() { - return new SunJceCrypt(cryptMethod); + return null; + } + + @Override + protected CryptData generateKey(Session session) + { + // generate new salt + byte[] salt = SunJceCrypt.randomSalt(); + + // generate new key + String key = session.getId() + "." + UUID.randomUUID().toString(); + + return new CryptData(key, salt); + } + + @Override + protected ICrypt createCrypt(CryptData keyParams) + { + SunJceCrypt crypt = new SunJceCrypt(cryptMethod, keyParams.salt, 1000); + crypt.setKey(keyParams.key); + + return crypt; + } + + static final class CryptData implements Serializable + { + /** + * + */ + private static final long serialVersionUID = 1L; + + final String key; Review comment: the indentation is wrong ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); + crypter.init(opMode, secretKey, params); + return crypter; + } catch (InvalidKeyException | InvalidAlgorithmParameterException + | NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new RuntimeException(e); + } + } + + /** + * Decrypts a string into a string. + * + * @param text + * text to decrypt + * @return the decrypted text + */ + @Override + public final String decryptUrlSafe(final String text) + { + try + { + byte[] decoded = java.util.Base64.getUrlDecoder().decode(text); + return new String(decryptByteArray(decoded), CHARACTER_ENCODING); + } + catch (Exception ex) + { + log.debug("Error decoding text: " + text, ex); + return null; + } + } + + /** + * Encrypt a string into a string using URL safe Base64 encoding. + * + * @param plainText + * text to encrypt + * @return encrypted string + */ + @Override + public final String encryptUrlSafe(final String plainText) + { + byte[] encrypted = encryptStringToByteArray(plainText); + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + byte[] encoded = encoder.encode(encrypted); + return new String(encoded, CHARACTER_ENCODING); + } + + /** + * Decrypts an encrypted byte array. + * + * @param encrypted + * byte array to decrypt + * @return the decrypted text + */ + abstract protected byte[] decryptByteArray(final byte[] encrypted); + + + /** + * Encrypts the given text into a byte array. + * + * @param plainText + * text to encrypt + * @return the string encrypted + * @throws GeneralSecurityException + */ + abstract protected byte[] encryptStringToByteArray(final String plainText); + + + @Override + public void setKey(String key) { Review comment: `final` ?! ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); + crypter.init(opMode, secretKey, params); + return crypter; + } catch (InvalidKeyException | InvalidAlgorithmParameterException + | NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new RuntimeException(e); + } + } + + /** + * Decrypts a string into a string. + * + * @param text + * text to decrypt + * @return the decrypted text + */ + @Override + public final String decryptUrlSafe(final String text) + { + try + { + byte[] decoded = java.util.Base64.getUrlDecoder().decode(text); + return new String(decryptByteArray(decoded), CHARACTER_ENCODING); + } + catch (Exception ex) + { + log.debug("Error decoding text: " + text, ex); + return null; + } + } + + /** + * Encrypt a string into a string using URL safe Base64 encoding. + * + * @param plainText + * text to encrypt + * @return encrypted string + */ + @Override + public final String encryptUrlSafe(final String plainText) + { + byte[] encrypted = encryptStringToByteArray(plainText); + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + byte[] encoded = encoder.encode(encrypted); + return new String(encoded, CHARACTER_ENCODING); + } + + /** + * Decrypts an encrypted byte array. + * + * @param encrypted + * byte array to decrypt + * @return the decrypted text + */ + abstract protected byte[] decryptByteArray(final byte[] encrypted); + + + /** + * Encrypts the given text into a byte array. + * + * @param plainText + * text to encrypt + * @return the string encrypted + * @throws GeneralSecurityException + */ + abstract protected byte[] encryptStringToByteArray(final String plainText); + + + @Override + public void setKey(String key) { + throw new UnsupportedOperationException(); Review comment: A message why it is not supported would be good ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); Review comment: s/crypter/cipher/ ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,122 @@ +/* + * 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.util.crypt; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.spec.AlgorithmParameterSpec; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; + + +/** + * Base class for JCE based ICrypt implementations. + * + */ +public abstract class AbstractJceCrypt implements ICrypt +{ + /** Encoding used to convert java String from and to byte[] */ + public static final Charset CHARACTER_ENCODING = StandardCharsets.UTF_8; + + /** Log. */ + private static final Logger log = LoggerFactory.getLogger(AbstractJceCrypt.class); + + protected Cipher buildCipher(int opMode, SecretKey secretKey, String transformation, AlgorithmParameterSpec params) + { + try + { + Cipher crypter = Cipher.getInstance(transformation); + crypter.init(opMode, secretKey, params); + return crypter; + } catch (InvalidKeyException | InvalidAlgorithmParameterException + | NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new RuntimeException(e); + } + } + + /** + * Decrypts a string into a string. + * + * @param text + * text to decrypt + * @return the decrypted text + */ + @Override + public final String decryptUrlSafe(final String text) + { + try + { + byte[] decoded = java.util.Base64.getUrlDecoder().decode(text); + return new String(decryptByteArray(decoded), CHARACTER_ENCODING); + } + catch (Exception ex) + { + log.debug("Error decoding text: " + text, ex); + return null; + } + } + + /** + * Encrypt a string into a string using URL safe Base64 encoding. + * + * @param plainText + * text to encrypt + * @return encrypted string + */ + @Override + public final String encryptUrlSafe(final String plainText) + { + byte[] encrypted = encryptStringToByteArray(plainText); + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + byte[] encoded = encoder.encode(encrypted); + return new String(encoded, CHARACTER_ENCODING); + } + + /** + * Decrypts an encrypted byte array. + * + * @param encrypted + * byte array to decrypt + * @return the decrypted text + */ + abstract protected byte[] decryptByteArray(final byte[] encrypted); + + + /** + * Encrypts the given text into a byte array. + * + * @param plainText + * text to encrypt + * @return the string encrypted + * @throws GeneralSecurityException + */ + abstract protected byte[] encryptStringToByteArray(final String plainText); Review comment: Do we need these long method names ? The return type and the argument type already give this info. Just `encrypt` is enough to me ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java ########## @@ -153,4 +194,18 @@ protected KeySpec createKeySpec() { return new PBEKeySpec(getKey().toCharArray()); } + + /** + * Create a random salt to be used for this crypt. + * + * @return salt, always 8 bytes long + */ + public static byte[] randomSalt() + { + // must be 8 bytes - for anything else PBES1Core throws + // InvalidAlgorithmParameterException: Salt must be 8 bytes long + byte[] salt = new byte[8]; + new Random().nextBytes(salt); Review comment: Should we use CipherUtils.randomByteArray(8) here ? It would use SecureRandom ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
