martin-g commented on a change in pull request #462: URL: https://github.com/apache/wicket/pull/462#discussion_r594950194
########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AESCrypt.java ########## @@ -0,0 +1,132 @@ +/* + * 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.core.random.ISecureRandomSupplier; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.lang.Args; + +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.NoSuchPaddingException; +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 + */ +public class AESCrypt extends AbstractJceCrypt +{ + + private final SecretKey secretKey; + private final String algorithm; + private ISecureRandomSupplier randomSupplier; Review comment: final ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractJceCrypt.java ########## @@ -0,0 +1,105 @@ +/* + * 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.util.crypt.ICrypt; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.util.Base64; + + +/** + * 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; Review comment: Any reason this to be `public` ? Why not just use `StandardCharsets.UTF_8` ? If the encoding should be configurable then there should be a constructor parameter for it. ########## File path: wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractKeyInSessionCryptFactory.java ########## @@ -0,0 +1,89 @@ +/* + * 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 org.apache.wicket.util.io.IClusterable; + + +/** + * 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 IClusterable> + implements + ICryptFactory +{ + /** metadata-key used to store crypto-key in session metadata */ + private final MetaDataKey<T> KEY = new MetaDataKey<T>() + { + private static final long serialVersionUID = 1L; + }; + + public AbstractKeyInSessionCryptFactory() Review comment: No need of this constructor, it is implicit ########## File path: wicket-core/src/test/java/org/apache/wicket/core/util/crypt/AESCryptTest.java ########## @@ -0,0 +1,54 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.wicket.core.random.DefaultSecureRandomSupplier; +import org.apache.wicket.util.crypt.CipherUtils; +import org.junit.jupiter.api.Test; + +import java.security.GeneralSecurityException; + +import javax.crypto.SecretKey; + +public class AESCryptTest +{ + @Test + public void encrypDecrypt() throws GeneralSecurityException + { + DefaultSecureRandomSupplier randomSupplier = new DefaultSecureRandomSupplier(); + + SecretKey secretKey = CipherUtils.generatePBEKey( + "myWeakPassword", "PBKDF2WithHmacSHA1", "AES", + randomSupplier.getRandomBytes(16), 65536, 256); + + AbstractJceCrypt crypt = new AESCrypt(secretKey, randomSupplier); + + String input1 = "input1"; + String encrypted = crypt.encryptUrlSafe(input1); + + String input2 = "input2"; Review comment: In my previous review I have suggested to use a more fancy input, e.g. one with umlauts or cyrillic characters, to test UTF-8, not just ASCII ---------------------------------------------------------------- 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
