From d740224b664273d17d5d292d1ae85d7a19c2d0dd Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Tue, 24 Jun 2014 09:45:41 +0000
Subject: [PATCH] Forward port fix for OPENDJ-1497: pwdHistory stops updating when multiple storage schemes are used
---
opendj-sdk/opendj3-server-dev/src/server/org/opends/server/core/PasswordPolicyState.java | 73 +++++++++++++-
opendj-sdk/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/core/PasswordPolicyTestCase.java | 202 ++++++++++++++++++++++++++++++++++------
2 files changed, 240 insertions(+), 35 deletions(-)
diff --git a/opendj-sdk/opendj3-server-dev/src/server/org/opends/server/core/PasswordPolicyState.java b/opendj-sdk/opendj3-server-dev/src/server/org/opends/server/core/PasswordPolicyState.java
index cdaf31f..f50f4f1 100644
--- a/opendj-sdk/opendj3-server-dev/src/server/org/opends/server/core/PasswordPolicyState.java
+++ b/opendj-sdk/opendj3-server-dev/src/server/org/opends/server/core/PasswordPolicyState.java
@@ -255,6 +255,30 @@
}
+ /**
+ * Get the password storage scheme used by a given password value.
+ *
+ * @param v The encoded password value to check.
+ *
+ * @return The scheme used by the password.
+ *
+ * @throws DirectoryException If the password could not be decoded.
+ */
+ private PasswordStorageScheme<?> getPasswordStorageScheme(ByteString v)
+ throws DirectoryException
+ {
+ if (passwordPolicy.isAuthPasswordSyntax())
+ {
+ StringBuilder[] pwComps = AuthPasswordSyntax.decodeAuthPassword(v.toString());
+ return DirectoryServer.getAuthPasswordStorageScheme(pwComps[0].toString());
+ }
+ else
+ {
+ String[] pwComps = UserPasswordSyntax.decodeUserPassword(v.toString());
+ return DirectoryServer.getPasswordStorageScheme(pwComps[0]);
+ }
+ }
+
/**
* {@inheritDoc}
@@ -2528,7 +2552,14 @@
}
-
+ /**
+ * Get the broken-down components of the given password value.
+ *
+ * @param usesAuthPasswordSyntax true if the value is an authPassword.
+ * @param v The encoded password value to break down.
+ *
+ * @return An array of components.
+ */
private StringBuilder[] getPwComponents(boolean usesAuthPasswordSyntax,
ByteString v) throws DirectoryException
{
@@ -2863,7 +2894,6 @@
return false;
}
-
// Check to see if the provided password is equal to any of the current
// passwords. If so, then we'll consider it to be in the history.
if (passwordMatches(password))
@@ -3151,8 +3181,10 @@
/**
- * Updates the password history information for this user by adding all
- * current passwords to it.
+ * Updates the password history information for this user by adding one of
+ * the passwords to it. It will choose the first password encoded using a
+ * secure storage scheme, and will fall back to a password encoded using an
+ * insecure storage scheme if necessary.
*/
public void updatePasswordHistory()
{
@@ -3162,9 +3194,40 @@
{
for (Attribute a : attrList)
{
+ boolean usesAuthPasswordSyntax = passwordPolicy.isAuthPasswordSyntax();
+ ByteString insecurePassword = null;
for (ByteString v : a)
{
- addPasswordToHistory(v.toString());
+ try
+ {
+ PasswordStorageScheme<?> scheme = getPasswordStorageScheme(v);
+
+ if (scheme.isStorageSchemeSecure())
+ {
+ addPasswordToHistory(v.toString());
+ insecurePassword = null;
+ // no need to check any more values for this attribute
+ break;
+ }
+ else if (insecurePassword == null)
+ {
+ insecurePassword = v;
+ }
+ }
+ catch (DirectoryException e)
+ {
+ if (logger.isTraceEnabled())
+ {
+ logger.trace("Encoded password " + v.toString() +
+ " cannot be decoded and cannot be added to history.");
+ }
+ }
+ }
+ // If we get here we haven't found a password encoded securely, so we
+ // have to use one of the other values.
+ if (insecurePassword != null)
+ {
+ addPasswordToHistory(insecurePassword.toString());
}
}
}
diff --git a/opendj-sdk/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/core/PasswordPolicyTestCase.java b/opendj-sdk/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/core/PasswordPolicyTestCase.java
index 3bb7061..ecf8aae 100644
--- a/opendj-sdk/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/core/PasswordPolicyTestCase.java
+++ b/opendj-sdk/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/core/PasswordPolicyTestCase.java
@@ -35,20 +35,20 @@
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import org.forgerock.opendj.config.server.ConfigException;
+import org.forgerock.opendj.ldap.ByteString;
import org.opends.server.TestCaseUtils;
import org.opends.server.admin.server.AdminTestCaseUtils;
import org.opends.server.admin.std.meta.PasswordPolicyCfgDefn;
import org.opends.server.admin.std.server.PasswordPolicyCfg;
import org.opends.server.api.PasswordStorageScheme;
import org.opends.server.config.ConfigEntry;
-import org.forgerock.opendj.config.server.ConfigException;
+import org.opends.server.schema.UserPasswordSyntax;
import org.opends.server.tools.LDAPModify;
-import org.opends.server.types.AttributeType;
-import org.opends.server.types.DN;
-import org.opends.server.types.Entry;
-import org.opends.server.types.InitializationException;
+import org.opends.server.types.*;
import org.opends.server.util.TimeThread;
+import static org.assertj.core.api.Assertions.*;
import static org.testng.Assert.*;
@@ -4277,6 +4277,170 @@
/**
+ * Sleep until we can be sure that the time thread has been updated.
+ *
+ * Otherwise, password changes can all appear to be in the same
+ * millisecond and really weird things start to happen because of the way
+ * that we handle conflicts in password history timestamps. In short, if
+ * the history is already at the maximum count and all the previous
+ * changes occurred in the same millisecond as the new change, then it's
+ * possible for a new change to come with a timestamp that is before the
+ * timestamps of all the existing values.
+ *
+ * @throws InterruptedException
+ */
+ private void advanceTimeThread() throws InterruptedException
+ {
+ long timeThreadCurrentTime = TimeThread.getTime();
+ while (timeThreadCurrentTime == TimeThread.getTime())
+ {
+ Thread.sleep(10);
+ }
+ }
+
+
+
+ /**
+ * Check there are 3 securely stored passwords in the entry's password history.
+ *
+ * @param dn The entry to check.
+ *
+ * @return The password history.
+ */
+ private Attribute checkPasswordHistory(String dn)
+ throws DirectoryException
+ {
+ Entry entry = DirectoryServer.getEntry(DN.valueOf(dn));
+ assertNotNull(entry);
+ AttributeType pwdHistory = DirectoryServer.getAttributeType("pwdhistory");
+ assertNotNull(pwdHistory);
+ Attribute historyAttr = entry.getExactAttribute(pwdHistory, null);
+ assertNotNull(historyAttr);
+ assertThat(historyAttr).hasSize(3);
+
+ for (ByteString v : historyAttr)
+ {
+ String[] history = v.toString().split("#");
+ assertEquals(history.length, 3);
+ String[] pwComps = UserPasswordSyntax.decodeUserPassword(history[2]);
+ PasswordStorageScheme<?> scheme = DirectoryServer.getPasswordStorageScheme(pwComps[0]);
+ assertTrue(scheme.isStorageSchemeSecure());
+ }
+
+ return historyAttr;
+ }
+
+
+
+ /**
+ * Use multiple storage schemes in the policy. Verify that history only
+ * contains one secure version of each password, is truncated correctly,
+ * and continues to change after reaching maximum.
+ *
+ * @throws Exception If an unexpected problem occurs.
+ */
+ @Test()
+ public void testHistoryWithMultipleSchemes()
+ throws Exception
+ {
+ TestCaseUtils.initializeTestBackend(true);
+ TestCaseUtils.dsconfig(
+ "set-password-policy-prop",
+ "--policy-name", "Default Password Policy",
+ "--set", "password-history-count:3",
+ "--set", "default-password-storage-scheme:Base64",
+ "--set", "default-password-storage-scheme:Salted SHA-256");
+
+ TestCaseUtils.addEntry(
+ "dn: uid=test.user,o=test",
+ "objectClass: top",
+ "objectClass: person",
+ "objectClass: organizationalPerson",
+ "objectClass: inetOrgPerson",
+ "uid: test.user",
+ "givenName: Test",
+ "sn: User",
+ "cn: Test User",
+ "userPassword: originalPassword",
+ "ds-privilege-name: bypass-acl");
+
+ try
+ {
+ // Change the password three times.
+ String newPWsPath = TestCaseUtils.createTempFile(
+ "dn: uid=test.user,o=test",
+ "changetype: modify",
+ "replace: userPassword",
+ "userPassword: newPassword1",
+ "",
+ "dn: uid=test.user,o=test",
+ "changetype: modify",
+ "replace: userPassword",
+ "userPassword: newPassword2",
+ "",
+ "dn: uid=test.user,o=test",
+ "changetype: modify",
+ "replace: userPassword",
+ "userPassword: newPassword3");
+
+ String[] args = new String[]
+ {
+ "-h", "127.0.0.1",
+ "-p", String.valueOf(TestCaseUtils.getServerLdapPort()),
+ "-D", "uid=test.user,o=test",
+ "-w", "originalPassword",
+ "-f", newPWsPath
+ };
+
+ assertEquals(LDAPModify.mainModify(args, false, System.out, System.err),
+ 0);
+
+
+ advanceTimeThread();
+
+
+ Attribute historyAfterThreeMods = checkPasswordHistory("uid=test.user,o=test");
+
+ newPWsPath = TestCaseUtils.createTempFile(
+ "dn: uid=test.user,o=test",
+ "changetype: modify",
+ "replace: userPassword",
+ "userPassword: newPassword4");
+
+ args = new String[]
+ {
+ "-h", "127.0.0.1",
+ "-p", String.valueOf(TestCaseUtils.getServerLdapPort()),
+ "-D", "uid=test.user,o=test",
+ "-w", "newPassword3",
+ "-f", newPWsPath
+ };
+
+ assertEquals(LDAPModify.mainModify(args, false, System.out, System.err),
+ 0);
+
+
+ advanceTimeThread();
+
+
+ Attribute historyAfterFourMods = checkPasswordHistory("uid=test.user,o=test");
+
+ // check history is now different (OPENDJ-1497)
+ assertFalse(historyAfterThreeMods.equals(historyAfterFourMods));
+
+ }
+ finally
+ {
+ TestCaseUtils.dsconfig(
+ "set-password-policy-prop",
+ "--policy-name", "Default Password Policy",
+ "--set", "default-password-storage-scheme:Salted SHA-1",
+ "--set", "password-history-count:0");
+ }
+ }
+
+
+ /**
* Tests the Directory Server's password history maintenance capabilities
* using only the password history count configuration option.
*
@@ -4362,19 +4526,7 @@
0);
- // Sleep until we can be sure that the time thread has been updated.
- // Otherwise, the password changes can all appear to be in the same
- // millisecond and really weird things start to happen because of the way
- // that we handle conflicts in password history timestamps. In short, if
- // the history is already at the maximum count and all the previous
- // changes occurred in the same millisecond as the new change, then it's
- // possible for a new change to come with a timestamp that is before the
- // timestamps of all the existing values.
- long timeThreadCurrentTime = TimeThread.getTime();
- while (timeThreadCurrentTime == TimeThread.getTime())
- {
- Thread.sleep(10);
- }
+ advanceTimeThread();
// Make sure that we still can't use the original password.
@@ -4417,12 +4569,7 @@
0);
- // Sleep again to ensure that the time thread is updated.
- timeThreadCurrentTime = TimeThread.getTime();
- while (timeThreadCurrentTime == TimeThread.getTime())
- {
- Thread.sleep(10);
- }
+ advanceTimeThread();
// Make sure that we can't use the second new password.
@@ -4445,12 +4592,7 @@
0);
- // Sleep again to ensure that the time thread is updated.
- timeThreadCurrentTime = TimeThread.getTime();
- while (timeThreadCurrentTime == TimeThread.getTime())
- {
- Thread.sleep(10);
- }
+ advanceTimeThread();
// Reduce the password history count from 3 to 2 and verify that we can
--
Gitblit v1.10.0