Change the hash logic for password, #AS-649

Auth/LdapAuth.php

@@ -43,9 +43,10 @@
  * There is no LDAP concept of a authentication token, and connecting to the LDAP
  * server for every token auth authentication would be very wasteful.
  *
- * So instead, when a user is synchronized, a token auth is generated using part of
- * the password hash stored in LDAP. We don't want to store the whole password hash
- * so attackers cannot get the true hash if they gain access to the MySQL DB.
+ * So instead, when a user is synchronized, Matomo stores a random placeholder in the
+ * password column and token auth authentication continues to use Matomo's normal token
+ * auth flow. The placeholder is intentionally unrelated to LDAP password attributes so
+ * database-only authentication cannot bypass LDAP-side checks.
  *
  * Once the token auth is generated, authenticating with it is done in the same way
  * as with {@link Piwik\Plugins\Login\Auth}. In fact, this class will create an

CHANGELOG.md

@@ -1,5 +1,8 @@
 # LoginLdap Changelog
 
+#### LoginLdap 5.2.4 - 2026-08-10
+- Added code to change the logic for random password generation
+
 #### LoginLdap 5.2.3 - 2026-08-03
 * Enabled password confirmation by default on the LoginLdap settings page
 * Added code to synchronize LDAP users using the resolved Matomo login instead of the supplied identifier

LdapInterop/UserMapper.php

@@ -125,19 +125,29 @@ public function getExpectedLdapUsername($login)
     }
 
     /**
-     * The password we store for a mapped user isn't used to authenticate, it's just
-     * data used to generate a user's token auth.
+     * Returns the value to store in the Matomo user's password column.
+     *
+     * When LoginLdap is configured to always authenticate against LDAP, this value must never be
+     * derived from LDAP password data. Otherwise fallback database authentication would let users
+     * bypass LDAP-side checks such as ldap_user_filter, required_member_of and directory account
+     * state.
+     *
+     * When LDAP is only used for synchronization, the stored database password remains part of the
+     * supported authentication flow, so the legacy behavior is preserved.
      */
     private function getPiwikPasswordForLdapUser($ldapUser, $user)
     {
+        $useLdapForAuthentication = Config::getUseLdapForAuthentication();
         $ldapPassword = $this->getLdapUserField($ldapUser, $this->ldapUserPasswordField);
 
-        if (!empty($user['password']) && !Config::getShouldSynchronizeUsersAfterLogin()) {
-            // do not generate new passwords for users that are already synchronized
-            return $user['password'];
-        } elseif (!empty($ldapPassword)) {
-            return $this->hashLdapPassword($ldapPassword);
-        } else {
+        if (!$useLdapForAuthentication) {
+            if (!empty($user['password']) && !Config::getShouldSynchronizeUsersAfterLogin()) {
+                // do not generate new passwords for users that are already synchronized
+                return $user['password'];
+            } elseif (!empty($ldapPassword)) {
+                return $this->hashLdapPassword($ldapPassword);
+            }
+
             $this->logger->debug(
                 "UserMapper::{func}: Could not find LDAP password for user '{user}', generating random one.",
                 array(
@@ -148,6 +158,16 @@ private function getPiwikPasswordForLdapUser($ldapUser, $user)
 
             return $this->generateRandomPassword();
         }
+
+        $this->logger->debug(
+            "UserMapper::{func}: generating random placeholder password for user '{user}'.",
+            array(
+                'func' => __FUNCTION__,
+                'user' => @$ldapUser[$this->ldapUserIdField]
+            )
+        );
+
+        return $this->generateRandomPassword();
     }
 
     /**
@@ -157,7 +177,7 @@ private function getPiwikPasswordForLdapUser($ldapUser, $user)
      */
     public function generateRandomPassword()
     {
-        return $this->hashLdapPassword(uniqid());
+        return $this->hashLdapPassword(random_bytes(32));
     }
 
     private function getEmailAddressForLdapUser($ldapUser, $login)
@@ -290,8 +310,8 @@ public function setAppendUserEmailSuffixToUsername($appendUserEmailSuffixToUsern
     }
 
     /**
-     * Hashes the LDAP password so no part the real LDAP password (or the hash stored in
-     * LDAP) will be stored in Piwik's DB.
+     * Hashes a value into the MD5 shaped hash that UsersManager::checkPasswordHash() expects,
+     * since the value is handed to the UsersManager API as an already hashed password.
      */
     protected function hashLdapPassword(
         #[\SensitiveParameter]

LoginLdap.php

@@ -94,6 +94,7 @@ public function getClientSideTranslationKeys(&$keys)
         $keys[] = 'LoginLdap_UserIdField';
         $keys[] = 'LoginLdap_UserIdFieldDescription';
         $keys[] = 'LoginLdap_PasswordField';
+        $keys[] = 'LoginLdap_PasswordFieldLegacy';
         $keys[] = 'LoginLdap_MailField';
         $keys[] = 'LoginLdap_MailFieldDescription';
         $keys[] = 'LoginLdap_UsernameSuffix';
@@ -140,6 +141,8 @@ public function getClientSideTranslationKeys(&$keys)
         $keys[] = 'LoginLdap_MemberOfDescription2';
         $keys[] = 'LoginLdap_PasswordFieldDescription';
         $keys[] = 'LoginLdap_PasswordFieldDescription2';
+        $keys[] = 'LoginLdap_PasswordFieldLdapAuthDescription';
+        $keys[] = 'LoginLdap_PasswordFieldLdapAuthDescription2';
         $keys[] = 'LoginLdap_LoadUserCommandDesc';
         $keys[] = 'LoginLdap_ReadMoreAboutAccessSynchronization';
         $keys[] = 'LoginLdap_ThisMatomoInstanceNameDescription';

README.md

@@ -58,8 +58,8 @@ Each strategy has advantages and disadvantages. What you should use depends on y
 This strategy is more secure than the one below, but it requires connecting to the LDAP server on each login attempt.
 
 With this strategy, every time a user logs in, LoginLdap will connect to LDAP to authenticate. On successful login, the user can
-be synchronised, but the user's password is never stored in Matomo's DB, just in the LDAP server. Additionally, the token auth is generated using
-a hash of a hash of the password, or is generated randomly.
+be synchronised, but the user's LDAP password is never stored in Matomo's DB. Instead, Matomo stores a random placeholder value for
+LDAP-authenticated users so database-only authentication cannot bypass LDAP-side checks.
 
 This means that if the Matomo DB is ever compromised, your LDAP users' passwords will still be safe.
 
@@ -238,8 +238,8 @@ If you set the **User Access Attribute Server & Site List Separator** option to
 
 **User passwords**
 
-For added security, LoginLdap's default configuration will not store user passwords or a hash of a user password within Matomo's DB. So if the Matomo DB is compromised
-for whatever reason, user passwords will not be compromised.
+For added security, LoginLdap's default configuration will not store user passwords or a value derived from an LDAP password attribute within Matomo's DB. LDAP-authenticated
+users receive a random placeholder instead. So if the Matomo DB is compromised for whatever reason, the LDAP password material is not exposed there.
 
 **Token Auths**
 

lang/en.json

@@ -74,6 +74,9 @@
         "PasswordField": "User Password Field",
         "PasswordFieldDescription": "Name of the LDAP attribute containing a user's password, e.g. 'userPassword' or 'unicodePwd'.",
         "PasswordFieldDescription2": "If 'Always Use LDAP for Authentication' is enabled and 'Generate Random token_auth For New Users' is disabled, the value of this field in LDAP must be a user's hashed or encrypted password.",
+        "PasswordFieldLegacy": "User Password Field (legacy in LDAP auth mode)",
+        "PasswordFieldLdapAuthDescription": "When 'Always Use LDAP for Authentication' is enabled, LoginLdap no longer uses this field to generate the Matomo password stored for synchronized users.",
+        "PasswordFieldLdapAuthDescription2": "Instead, LoginLdap stores a random placeholder so database-only authentication cannot bypass LDAP-side checks. This field is still used when LDAP is only used for synchronization.",
         "ReadMoreAboutAccessSynchronization": "To learn more about user access synchronization, %1$sread our docs%2$s.",
         "ExpectedLdapAttributes": "Expected LDAP attributes",
         "ExpectedLdapAttributesPrelude": "With this configuration, LoginLdap will expect attributes in LDAP that look like",

plugin.json

@@ -1,6 +1,6 @@
 {
     "name": "LoginLdap",
-    "version": "5.2.3",
+    "version": "5.2.4",
     "description": "LDAP authentication and synchronization for Matomo.",
     "theme": false,
     "keywords": ["ldap", "login", "authentication", "active", "directory", "kerberos", "sso"],

tests/Integration/AuthenticationTest.php

@@ -15,6 +15,7 @@
 use Piwik\Config;
 use Piwik\Container\StaticContainer;
 use Piwik\Plugins\LoginLdap\Auth\LdapAuth;
+use Piwik\Plugins\LoginLdap\LdapInterop\UserMapper;
 use Piwik\Plugins\UsersManager\API as UsersManagerAPI;
 use Piwik\Tests\Framework\Fixture;
 
@@ -231,6 +232,36 @@ public function test_LdapAuth_DoesNotAuthenticate_WhenAnonymousLoginProvided()
         $this->assertEquals(0, $authResult->getCode());
     }
 
+    public function test_LdapAuth_RotatesStoredPlaceholderAfterSuccessfulLogin_WhenSynchronizeUsersAfterLoginDisabled()
+    {
+        Config::getInstance()->LoginLdap['synchronize_users_after_login'] = 0;
+
+        UsersManagerAPI::getInstance()->addUser(
+            self::TEST_LOGIN,
+            'averywrongpassword',
+            'billionairephilanthropistplayboy@starkindustries.com'
+        );
+
+        $userMapper = new UserMapper();
+        $userMapper->markUserAsLdapUser(self::TEST_LOGIN);
+
+        $ldapAuth = LdapAuth::makeConfigured();
+        $ldapAuth->setLogin(self::TEST_LOGIN);
+        $ldapAuth->setPassword(self::TEST_PASS);
+        $authResult = $ldapAuth->authenticate();
+
+        $this->assertEquals(AuthResult::SUCCESS, $authResult->getCode());
+
+        $user = $this->getUser(self::TEST_LOGIN);
+        $this->assertPasswordIsRandomPlaceholder($user['password']);
+
+        $normalAuth = new \Piwik\Plugins\Login\Auth();
+        $normalAuth->setLogin(self::TEST_LOGIN);
+        $normalAuth->setPassword('averywrongpassword');
+
+        $this->assertEquals(AuthResult::FAILURE, $normalAuth->authenticate()->getCode());
+    }
+
     private function getNonLdapUserTokenAuth()
     {
         return $this->nonLdapUserAppPassword;

tests/Integration/LdapIntegrationTest.php

@@ -157,8 +157,7 @@ protected function assertStarkSynchronized($expectedDomain = 'starkindustries.co
     {
         $user = $this->getUser(self::TEST_LOGIN);
         $this->assertNotEmpty($user);
-        $passwordHelper = new Password();
-        $this->assertTrue($passwordHelper->verify(md5(self::TEST_PASS_LDAP), $user['password']));
+        $this->assertPasswordIsRandomPlaceholder($user['password']);
         unset($user['password']);
         $this->assertEquals(array(
             'login' => self::TEST_LOGIN,
@@ -168,6 +167,20 @@ protected function assertStarkSynchronized($expectedDomain = 'starkindustries.co
         $this->assertTrue($userMapper->isUserLdapUser(self::TEST_LOGIN));
     }
 
+    /**
+     * A synchronized LDAP user's Matomo password column holds an unguessable placeholder. It must
+     * not be derivable from the LDAP userPassword attribute: Matomo treats the column as a real
+     * password hash, so a derivable value would let anyone holding that attribute log in as the
+     * user, without the LDAP side checks (ldap_user_filter, required_member_of, account state)
+     * ever running.
+     */
+    protected function assertPasswordIsRandomPlaceholder($storedPasswordHash)
+    {
+        $passwordHelper = new Password();
+        $this->assertFalse($passwordHelper->verify(md5(self::TEST_PASS_LDAP), $storedPasswordHash));
+        $this->assertFalse($passwordHelper->verify(md5(self::TEST_PASS), $storedPasswordHash));
+    }
+
     protected function assertRomanovSynchronized($expectedDomain)
     {
         $user = $this->getUser('blackwidow');

tests/Integration/LdapUserSynchronizationTest.php

@@ -264,20 +264,20 @@ public function test_SuperUserAccessSynchronized_WhenLdapAccessInfoPresent_AndIn
 
     public function test_RandomPasswordGenerated()
     {
-        $passwordManager = new Password();
-
         $this->authenticateViaLdap();
 
         $user = $this->getUser(self::TEST_LOGIN);
 
-        $this->assertTrue($passwordManager->verify(md5(self::TEST_PASS_LDAP), $user['password']));
+        $this->assertPasswordIsRandomPlaceholder($user['password']);
 
-        // test that password doesn't change after re-synchronizing
+        // the placeholder is regenerated when re-synchronizing. This does not invalidate existing
+        // sessions, since UserSynchronizer resets ts_password_modified afterwards.
         $this->authenticateViaLdap();
 
         $userAgain = $this->getUser(self::TEST_LOGIN);
 
-        $this->assertTrue($passwordManager->verify(md5(self::TEST_PASS_LDAP), $userAgain['password']));
+        $this->assertPasswordIsRandomPlaceholder($userAgain['password']);
+        $this->assertNotEquals($user['password'], $userAgain['password']);
     }
 
     public function test_CorrectExistingUserUpdated_WhenUserEmailSuffixUsed()

tests/Integration/PasswordConfirmationTest.php

@@ -167,18 +167,17 @@ public function testSaveLdapConfigRejectsWrongPasswordForLdapUser()
     }
 
     /**
-     * A synchronized LDAP user's Matomo password column holds md5() of the LDAP password
-     * *hash* (see UserMapper::getPiwikPasswordForLdapUser), never the plaintext LDAP
-     * password, so confirming with TEST_PASS can only succeed by binding to LDAP.
+     * A synchronized LDAP user's Matomo password column holds an unguessable placeholder (see
+     * UserMapper::getPiwikPasswordForLdapUser), neither the plaintext LDAP password nor anything
+     * derived from the LDAP password attribute, so confirming with TEST_PASS can only succeed by
+     * binding to LDAP.
      */
     public function testLdapUserPasswordConfirmationIsCheckedAgainstLdapNotTheMatomoDatabase()
     {
         $ldapAuth = $this->useRealLdapUser();
 
         $user = $this->getUser(self::TEST_LOGIN);
-        $passwordHelper = new \Piwik\Auth\Password();
-        $this->assertFalse($passwordHelper->verify(md5(self::TEST_PASS), $user['password']));
-        $this->assertTrue($passwordHelper->verify(md5(self::TEST_PASS_LDAP), $user['password']));
+        $this->assertPasswordIsRandomPlaceholder($user['password']);
 
         StaticContainer::getContainer()->set('Piwik\Auth', new \Piwik\Plugins\Login\Auth());
         $this->assertFalse($this->passwordVerifier->isPasswordCorrect(self::TEST_LOGIN, self::TEST_PASS));