papegaaij commented on a change in pull request #462: URL: https://github.com/apache/wicket/pull/462#discussion_r591810682
########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/CipherUtils.java ########## @@ -0,0 +1,107 @@ +/* + * 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 java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.KeySpec; + +import javax.crypto.Cipher; +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; + +/** + * Utility class meant to help building {@link Cipher}. + */ +public class CipherUtils +{ + /** + * Generate a new {@link SecretKey} based on the given algorithm and with the given length. + * + * @param algorithm + * the algorithm that will be used to build the key. + * @param keyLength + * the key length + * @return a new {@link SecretKey} + */ + public static SecretKey generateKey(String algorithm, int keyLength) + { + try + { + KeyGenerator keyGenerator = KeyGenerator.getInstance(algorithm); + keyGenerator.init(keyLength); Review comment: Pass in a SecureRandom here as well using the one from SecuritySettings.getRandomSupplier().getRandom(). That will make sure we use one reliable source of random and it's customizable by the user. ########## 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. Review comment: The ivSize is not needed. It is always equal to the blocksize of the cipher. So you can simply use cipher.getBlockSize() to get it. ########## 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: Better yet, remove this method entirely. Usage of this method should be replaced by ISecureRandomSupplier.getRandomBytes(length). The optimal length of the salt depends on the algorithm used. Some might require 8 bytes, but for example PBKDF2WithHmacSHA512 can use much longer salts and it's recommended to use a salt that's equal in length to your derived key. ########## File path: wicket-util/src/main/java/org/apache/wicket/util/crypt/CipherUtils.java ########## @@ -0,0 +1,107 @@ +/* + * 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 java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.KeySpec; + +import javax.crypto.Cipher; +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; + +/** + * Utility class meant to help building {@link Cipher}. + */ +public class CipherUtils +{ + /** + * Generate a new {@link SecretKey} based on the given algorithm and with the given length. + * + * @param algorithm + * the algorithm that will be used to build the key. + * @param keyLength + * the key length + * @return a new {@link SecretKey} + */ + public static SecretKey generateKey(String algorithm, int keyLength) + { + try + { + KeyGenerator keyGenerator = KeyGenerator.getInstance(algorithm); + keyGenerator.init(keyLength); + SecretKey key = keyGenerator.generateKey(); + return key; + } + catch (NoSuchAlgorithmException e) + { + throw new RuntimeException(e); + } + } + + /** + * + * + * @param password + * the password that will be used to build the key. + * @param pbeAlgorithm + * the password-based algorithm that will be used to build the key. + * @param keyAlgorithm + * the algorithm that will be used to build the key. + * @param salt + * salt for encryption. + * @param iterationCount + * iteration count. + * @param keyLength + * the key length. + * @return a new {@link SecretKey} + */ + public static SecretKey generatePBEKey(String password, String pbeAlgorithm, + String keyAlgorithm, byte[] salt, int iterationCount, int keyLength) + { + try + { + SecretKeyFactory factory = SecretKeyFactory.getInstance(pbeAlgorithm); + KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, iterationCount, keyLength); + SecretKey secret = new SecretKeySpec(factory.generateSecret(spec).getEncoded(), + keyAlgorithm); + return secret; + } + catch (NoSuchAlgorithmException | InvalidKeySpecException e) + { + throw new RuntimeException(e); + } + } + + /** + * Generates a random generated byte array of the given size. + * + * @param size + * the size of the array. + * @return the new byte array of the given size. + */ + public static byte[] randomByteArray(int size) + { + byte[] iv = new byte[size]; + new SecureRandom().nextBytes(iv); Review comment: Yes, creating a SecureRandom over and over will exhaust your systems entropy. This method already exists in ISecureRandomSupplier (SecuritySettings.getRandomSupplier), so it can be removed. ---------------------------------------------------------------- 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
