Enhance pbkdf2 password hashing

Fallback to PBKDF2WithHmacSHA1 added depricated doc Update security documentation for Password validation Update security documentation for Password validation Update security documentation for Password validation fix remove colon Removed redundant statement reinstated malformed check Add pbkdff2 sha256 updated Duplicate User for pbkdf2
prestodb · Dec 6, 2024 · 39b9a83 · 39b9a83
1 parent bd7afd5
commit 39b9a83
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 9 deletions.
diff --git a/presto-docs/src/main/sphinx/security/password-file.rst b/presto-docs/src/main/sphinx/security/password-file.rst
@@ -36,6 +36,42 @@ Property                             Description
                                      Defaults to ``1000``.
 ==================================== ==============================================
 
+Password Validation
+-------------------
+
+Password validation in Presto supports both `PBKDF2WithHmacSHA256` and `PBKDF2WithHmacSHA1` algorithms. 
+To ensure modern cryptographic standards, clients are encouraged to use `PBKDF2WithHmacSHA256`. 
+A fallback mechanism is available to maintain compatibility with legacy systems using `PBKDF2WithHmacSHA1`.
+
+Migration to `PBKDF2WithHmacSHA256` is strongly recommended to maintain security.
+
+API Method
+^^^^^^^^^^
+The following method uses the `PBKDF2WithHmacSHA256` validation mechanism and includes a fallback mechanism:
+
+.. code-block:: java
+
+    /**
+     * @Deprecated using PBKDF2WithHmacSHA1 is deprecated and clients should switch to PBKDF2WithHmacSHA256
+     */
+    public static boolean doesPBKDF2PasswordMatch(String inputPassword, String hashedPassword)
+    {
+        PBKDF2Password password = PBKDF2Password.fromString(hashedPassword);
+
+        // Validate using PBKDF2WithHmacSHA256
+        if (validatePBKDF2Password(inputPassword, password, "PBKDF2WithHmacSHA256")) {
+            return true;
+        }
+
+        // Fallback to PBKDF2WithHmacSHA1
+        LOG.warn("Using deprecated PBKDF2WithHmacSHA1 for password validation.");
+        return validatePBKDF2Password(inputPassword, password, "PBKDF2WithHmacSHA1");
+    }
+
+**Fallback Mechanism**
+
+If `PBKDF2WithHmacSHA256` fails for legacy reasons, the system gracefully falls back to `PBKDF2WithHmacSHA1` while logging a warning.
+
 Password Files
 --------------
 

diff --git a/...ssword-authenticators/src/main/java/com/facebook/presto/password/file/EncryptionUtil.java b/...ssword-authenticators/src/main/java/com/facebook/presto/password/file/EncryptionUtil.java
@@ -15,6 +15,7 @@
 
 import at.favre.lib.crypto.bcrypt.BCrypt;
 import at.favre.lib.crypto.bcrypt.IllegalBCryptFormatException;
+import com.facebook.airlift.log.Logger;
 import com.google.common.base.Splitter;
 
 import javax.crypto.SecretKeyFactory;
@@ -33,6 +34,7 @@
 
 public final class EncryptionUtil
 {
+    private static final Logger LOG = Logger.get(EncryptionUtil.class);
     private static final int BCRYPT_MIN_COST = 8;
     private static final int PBKDF2_MIN_ITERATIONS = 1000;
 
@@ -58,14 +60,29 @@ public static boolean doesBCryptPasswordMatch(String inputPassword, String hashe
         return BCrypt.verifyer().verify(inputPassword.toCharArray(), hashedPassword).verified;
     }
 
+    /**
+     * @Deprecated using PBKDF2WithHmacSHA1 is deprecated and clients should switch to PBKDF2WithHmacSHA256
+     */
     public static boolean doesPBKDF2PasswordMatch(String inputPassword, String hashedPassword)
     {
         PBKDF2Password password = PBKDF2Password.fromString(hashedPassword);
 
+        // Validate using PBKDF2WithHmacSHA256
+        if (validatePBKDF2Password(inputPassword, password, "PBKDF2WithHmacSHA256")) {
+            return true;
+        }
+
+        // Fallback to PBKDF2WithHmacSHA1
+        LOG.warn("Using deprecated PBKDF2WithHmacSHA1 for password validation.");
+        return validatePBKDF2Password(inputPassword, password, "PBKDF2WithHmacSHA1");
+    }
+
+    private static boolean validatePBKDF2Password(String inputPassword, PBKDF2Password password, String algorithm)
+    {
         try {
             KeySpec spec = new PBEKeySpec(inputPassword.toCharArray(), password.salt(), password.iterations(), password.hash().length * 8);
-            SecretKeyFactory key = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
-            byte[] inputHash = key.generateSecret(spec).getEncoded();
+            SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(algorithm);
+            byte[] inputHash = keyFactory.generateSecret(spec).getEncoded();
 
             if (password.hash().length != inputHash.length) {
                 throw new HashedPasswordException("PBKDF2 password input is malformed");

diff --git a/...rd-authenticators/src/test/java/com/facebook/presto/password/file/TestEncryptionUtil.java b/...rd-authenticators/src/test/java/com/facebook/presto/password/file/TestEncryptionUtil.java
@@ -32,12 +32,19 @@ public void testHashingAlgorithmBCrypt()
     }
 
     @Test
-    public void testHashingAlgorithmPBKDF2()
+    public void testHashingAlgorithmPBKDF2SHA1()
     {
         String password = "1000:5b4240333032306164:f38d165fce8ce42f59d366139ef5d9e1ca1247f0e06e503ee1a611dd9ec40876bb5edb8409f5abe5504aab6628e70cfb3d3a18e99d70357d295002c3d0a308a0";
         assertEquals(getHashingAlgorithm(password), PBKDF2);
     }
 
+    @Test
+    public void testHashingAlgorithmPBKDF2SHA256()
+    {
+        String password = "1000:5b4240333032306164:acac1637d8219b50218fa2e1b82156dd73701f5fa6144a9178327226a1b3448bd1fc8e56c4a8a0ac582a4b02c5368a36663a03476e2e9be7c44680920c661c0f";
+        assertEquals(getHashingAlgorithm(password), PBKDF2);
+    }
+
     @Test
     public void testMinBCryptCost()
     {

diff --git a/...ord-authenticators/src/test/java/com/facebook/presto/password/file/TestPasswordStore.java b/...ord-authenticators/src/test/java/com/facebook/presto/password/file/TestPasswordStore.java
@@ -23,20 +23,25 @@
 public class TestPasswordStore
 {
     private static final String BCRYPT_PASSWORD = "$2y$10$BqTb8hScP5DfcpmHo5PeyugxHz5Ky/qf3wrpD7SNm8sWuA3VlGqsa";
-    private static final String PBKDF2_PASSWORD = "1000:5b4240333032306164:f38d165fce8ce42f59d366139ef5d9e1ca1247f0e06e503ee1a611dd9ec40876bb5edb8409f5abe5504aab6628e70cfb3d3a18e99d70357d295002c3d0a308a0";
+    private static final String PBKDF2_PASSWORD_SHA1 = "1000:5b4240333032306164:f38d165fce8ce42f59d366139ef5d9e1ca1247f0e06e503ee1a611dd9ec40876bb5edb8409f5abe5504aab6628e70cfb3d3a18e99d70357d295002c3d0a308a0";
+    private static final String PBKDF2_PASSWORD_SHA256 = "1000:5b4240333032306164:acac1637d8219b50218fa2e1b82156dd73701f5fa6144a9178327226a1b3448bd1fc8e56c4a8a0ac582a4b02c5368a36663a03476e2e9be7c44680920c661c0f";
 
     @Test
     public void testAuthenticate()
     {
-        PasswordStore store = createStore("userbcrypt:" + BCRYPT_PASSWORD, "userpbkdf2:" + PBKDF2_PASSWORD);
+        PasswordStore store = createStore("userbcrypt:" + BCRYPT_PASSWORD, "userpbkdf2sha1:" + PBKDF2_PASSWORD_SHA1, "userpbkdf2sha256:" + PBKDF2_PASSWORD_SHA256);
 
         assertTrue(store.authenticate("userbcrypt", "user123"));
         assertFalse(store.authenticate("userbcrypt", "user999"));
         assertFalse(store.authenticate("userbcrypt", "password"));
 
-        assertTrue(store.authenticate("userpbkdf2", "password"));
-        assertFalse(store.authenticate("userpbkdf2", "password999"));
-        assertFalse(store.authenticate("userpbkdf2", "user123"));
+        assertTrue(store.authenticate("userpbkdf2sha1", "password"));
+        assertFalse(store.authenticate("userpbkdf2sha1", "password999"));
+        assertFalse(store.authenticate("userpbkdf2sha1", "user123"));
+
+        assertTrue(store.authenticate("userpbkdf2sha256", "password"));
+        assertFalse(store.authenticate("userpbkdf2sha256", "password999"));
+        assertFalse(store.authenticate("userpbkdf2sha256", "user123"));
 
         assertFalse(store.authenticate("baduser", "user123"));
         assertFalse(store.authenticate("baduser", "password"));
@@ -54,9 +59,12 @@ public void testInvalidFile()
         assertThatThrownBy(() -> createStore("", "junk"))
                 .hasMessage("Error in password file line 2: Expected two parts for user and password");
 
-        assertThatThrownBy(() -> createStore("abc:" + BCRYPT_PASSWORD, "xyz:" + BCRYPT_PASSWORD, "abc:" + PBKDF2_PASSWORD))
+        assertThatThrownBy(() -> createStore("abc:" + BCRYPT_PASSWORD, "xyz:" + BCRYPT_PASSWORD, "abc:" + PBKDF2_PASSWORD_SHA1))
                 .hasMessage("Error in password file line 3: Duplicate user: abc");
 
+        assertThatThrownBy(() -> createStore("abc:" + BCRYPT_PASSWORD, "xyz:" + BCRYPT_PASSWORD, "xyz:" + PBKDF2_PASSWORD_SHA256))
+                .hasMessage("Error in password file line 3: Duplicate user: xyz");
+
         assertThatThrownBy(() -> createStore("x:x"))
                 .hasMessage("Error in password file line 1: Password hashing algorithm cannot be determined");