Updates password comparison to use length obfuscation alg. master
authorjohnnyv <johnnyv@apache.org>
Tue, 22 May 2018 13:55:33 +0000 (09:55 -0400)
committerjohnnyv <johnnyv@apache.org>
Tue, 22 May 2018 13:55:33 +0000 (09:55 -0400)
core/src/main/java/org/apache/ftpserver/usermanager/ClearTextPasswordEncryptor.java
core/src/main/java/org/apache/ftpserver/usermanager/Md5PasswordEncryptor.java
core/src/main/java/org/apache/ftpserver/usermanager/SaltedPasswordEncryptor.java
core/src/main/java/org/apache/ftpserver/util/PasswordUtil.java [new file with mode: 0644]

index e9dc49e..9792bff 100644 (file)
@@ -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);
     }
 
 }
index 3401eb2..a19d6f1 100644 (file)
 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());
+       }
 }
index 1cd1bf5..15d13ac 100644 (file)
@@ -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());
+       }
 
 }
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 (file)
index 0000000..62fdc75
--- /dev/null
@@ -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);
+       }
+}