Repository: mina-ftpserver Updated Branches: refs/heads/master 49788dfc4 -> 497559d0e
Updates password comparison to use length obfuscation alg. Project: http://git-wip-us.apache.org/repos/asf/mina-ftpserver/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-ftpserver/commit/497559d0 Tree: http://git-wip-us.apache.org/repos/asf/mina-ftpserver/tree/497559d0 Diff: http://git-wip-us.apache.org/repos/asf/mina-ftpserver/diff/497559d0 Branch: refs/heads/master Commit: 497559d0e00ee427d0f7953b2d7665333d6782f5 Parents: 49788df Author: johnnyv <john...@apache.org> Authored: Tue May 22 09:55:33 2018 -0400 Committer: johnnyv <john...@apache.org> Committed: Tue May 22 09:55:33 2018 -0400 ---------------------------------------------------------------------- .../usermanager/ClearTextPasswordEncryptor.java | 5 +- .../usermanager/Md5PasswordEncryptor.java | 43 ++++---- .../usermanager/SaltedPasswordEncryptor.java | 86 ++++++++-------- .../org/apache/ftpserver/util/PasswordUtil.java | 103 +++++++++++++++++++ 4 files changed, 170 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-ftpserver/blob/497559d0/core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java b/core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java index e9dc49e..9792bff 100644 --- a/core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java +++ b/core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java @@ -19,8 +19,7 @@ package org.apache.ftpserver.usermanager; - - +import org.apache.ftpserver.util.PasswordUtil; /** * Password encryptor that does no encryption, that is, keps the @@ -48,7 +47,7 @@ public class ClearTextPasswordEncryptor implements PasswordEncryptor { throw new NullPointerException("passwordToCheck can not be null"); } - return passwordToCheck.equals(storedPassword); + return PasswordUtil.secureCompareFast(passwordToCheck, storedPassword); } } http://git-wip-us.apache.org/repos/asf/mina-ftpserver/blob/497559d0/core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java b/core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java index 3401eb2..a19d6f1 100644 --- a/core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java +++ b/core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java @@ -20,35 +20,34 @@ package org.apache.ftpserver.usermanager; import org.apache.ftpserver.util.EncryptUtils; - +import org.apache.ftpserver.util.PasswordUtil; /** - * Password encryptor that hashes the password using MD5. Please note that this form - * of encryption is sensitive to lookup attacks. + * Password encryptor that hashes the password using MD5. Please note that this + * form of encryption is sensitive to lookup attacks. * * @author <a href="http://mina.apache.org";>Apache MINA Project</a> */ public class Md5PasswordEncryptor implements PasswordEncryptor { - /** - * Hashes the password using MD5 - */ - public String encrypt(String password) { - return EncryptUtils.encryptMD5(password); - } + /** + * Hashes the password using MD5 + */ + public String encrypt(String password) { + return EncryptUtils.encryptMD5(password); + } - /** - * {@inheritDoc} - */ - public boolean matches(String passwordToCheck, String storedPassword) { - if(storedPassword == null) { - throw new NullPointerException("storedPassword can not be null"); - } - if(passwordToCheck == null) { - throw new NullPointerException("passwordToCheck can not be null"); - } - - return encrypt(passwordToCheck).equalsIgnoreCase(storedPassword); - } + /** + * {@inheritDoc} + */ + public boolean matches(String passwordToCheck, String storedPassword) { + if (storedPassword == null) { + throw new NullPointerException("storedPassword can not be null"); + } + if (passwordToCheck == null) { + throw new NullPointerException("passwordToCheck can not be null"); + } + return PasswordUtil.secureCompareFast(encrypt(passwordToCheck).toLowerCase(), storedPassword.toLowerCase()); + } } http://git-wip-us.apache.org/repos/asf/mina-ftpserver/blob/497559d0/core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java b/core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java index 1cd1bf5..15d13ac 100644 --- a/core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java +++ b/core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java @@ -22,11 +22,12 @@ package org.apache.ftpserver.usermanager; import java.security.SecureRandom; import org.apache.ftpserver.util.EncryptUtils; +import org.apache.ftpserver.util.PasswordUtil; /** - * Password encryptor that hashes a salt together with the password using MD5. - * Using a salt protects against birthday attacks. - * The hashing is also made in iterations, making lookup attacks much harder. + * Password encryptor that hashes a salt together with the password using MD5. + * Using a salt protects against birthday attacks. The hashing is also made in + * iterations, making lookup attacks much harder. * * The algorithm is based on the principles described in * http://www.jasypt.org/howtoencryptuserpasswords.html @@ -35,50 +36,51 @@ import org.apache.ftpserver.util.EncryptUtils; */ public class SaltedPasswordEncryptor implements PasswordEncryptor { - private SecureRandom rnd = new SecureRandom(); + private SecureRandom rnd = new SecureRandom(); - private static final int MAX_SEED = 99999999; - private static final int HASH_ITERATIONS = 1000; + private static final int MAX_SEED = 99999999; + private static final int HASH_ITERATIONS = 1000; - private String encrypt(String password, String salt) { - String hash = salt + password; - for (int i = 0; i < HASH_ITERATIONS; i++) { - hash = EncryptUtils.encryptMD5(hash); - } - return salt + ":" + hash; - } + private String encrypt(String password, String salt) { + String hash = salt + password; + for (int i = 0; i < HASH_ITERATIONS; i++) { + hash = EncryptUtils.encryptMD5(hash); + } + return salt + ":" + hash; + } - /** - * Encrypts the password using a salt concatenated with the password - * and a series of MD5 steps. - */ - public String encrypt(String password) { - String seed = Integer.toString(rnd.nextInt(MAX_SEED)); + /** + * Encrypts the password using a salt concatenated with the password and a + * series of MD5 steps. + */ + public String encrypt(String password) { + String seed = Integer.toString(rnd.nextInt(MAX_SEED)); - return encrypt(password, seed); - } + return encrypt(password, seed); + } - /** - * {@inheritDoc} - */ - public boolean matches(String passwordToCheck, String storedPassword) { - if (storedPassword == null) { - throw new NullPointerException("storedPassword can not be null"); - } - if (passwordToCheck == null) { - throw new NullPointerException("passwordToCheck can not be null"); - } + /** + * {@inheritDoc} + */ + public boolean matches(String passwordToCheck, String storedPassword) { + if (storedPassword == null) { + throw new NullPointerException("storedPassword can not be null"); + } + if (passwordToCheck == null) { + throw new NullPointerException("passwordToCheck can not be null"); + } - // look for divider for hash - int divider = storedPassword.indexOf(':'); - - if(divider < 1) { - throw new IllegalArgumentException("stored password does not contain salt"); - } - - String storedSalt = storedPassword.substring(0, divider); - - return encrypt(passwordToCheck, storedSalt).equalsIgnoreCase(storedPassword); - } + // look for divider for hash + int divider = storedPassword.indexOf(':'); + + if (divider < 1) { + throw new IllegalArgumentException("stored password does not contain salt"); + } + + String storedSalt = storedPassword.substring(0, divider); + + return PasswordUtil.secureCompareFast(encrypt(passwordToCheck, storedSalt).toLowerCase(), + storedPassword.toLowerCase()); + } } http://git-wip-us.apache.org/repos/asf/mina-ftpserver/blob/497559d0/core/src/main/java/org/apache/ftpserver/util/PasswordUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/ftpserver/util/PasswordUtil.java b/core/src/main/java/org/apache/ftpserver/util/PasswordUtil.java new file mode 100644 index 0000000..62fdc75 --- /dev/null +++ b/core/src/main/java/org/apache/ftpserver/util/PasswordUtil.java @@ -0,0 +1,103 @@ +/* + * 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.ftpserver.util; + +public class PasswordUtil { + /** + * Securely compares two strings up to a maximum number of characters in a way + * that obscures the password length from timing attacks + * + * @param input + * user input + * @param password + * correct password + * @param loops + * number of characters to compare; must be larger than password + * length; 1024 is a good number + * + * @throws IllegalArgumentException + * when the limit is less than the password length + * + * @return true if the passwords match + */ + public static boolean secureCompare(String input, String password, int loops) { + if (loops < password.length()) { + throw new IllegalArgumentException("loops must be equal or greater than the password length"); + } + /* + * Set the default result based on the string lengths; if the lengths do not + * match then we know that this comparison should always fail. + */ + int result = (input.length() ^ password.length()); + /* + * Cycle through all of the characters up to the limit value + * + * Important to note that this loop may return a false positive comparison if + * the target string is a repeating set of characters in direct multiples of the + * input string. This design fallacy is corrected by the original length + * comparison above. The use of modulo this way is intended to prevent compiler + * and memory optimizations for retrieving the same char position in sequence. + */ + for (int i = 0; i < loops; i++) { + result |= (input.charAt(i % input.length()) ^ password.charAt(i % password.length())); + } + + return (result == 0); + } + + /** + * Securely compares two strings forcing the number of loops equal to password length + * thereby obscuring the password length based on user input + * + * @param input + * user input + * @param password + * correct password + * @throws IllegalArgumentException + * when the limit is less than the password length + * + * @return true if the passwords match + */ + public static boolean secureCompareFast(String input, String password) { + /* + * the number of compare loops + */ + int loops = password.length(); + /* + * Set the default result based on the string lengths; if the lengths do not + * match then we know that this comparison should always fail. + */ + int result = (input.length() ^ password.length()); + /* + * Cycle through all of the characters up to the limit value + * + * Important to note that this loop may return a false positive comparison if + * the target string is a repeating set of characters in direct multiples of the + * input string. This design fallacy is corrected by the original length + * comparison above. The use of modulo this way is intended to prevent compiler + * and memory optimizations for retrieving the same char position in sequence. + */ + for (int i = 0; i < loops; i++) { + result |= (input.charAt(i % input.length()) ^ password.charAt(i % password.length())); + } + + return (result == 0); + } +}
