Updates password comparison to use length obfuscation alg.
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 @@
throw new NullPointerException("passwordToCheck can not be null");
}
- return passwordToCheck.equals(storedPassword);
+ return PasswordUtil.secureCompareFast(passwordToCheck, storedPassword);
}
}
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());
+ }
}
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 @@
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 @@
*/
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());
+ }
}
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);
+ }
+}
