From 0d2456fd9e2678f6db075b224528727b741ff205 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Fri, 14 Sep 2018 11:24:35 -0400
Subject: [PATCH] Ticket 49950 - PassSync not setting pwdLastSet attribute in
Active Directory after Pw update from LDAP sync for normal user
Bug Description:
If a user's password was reset by an "Admin" or directory manager, the
password policy requires a user must change their password after it's
been "reset", and the user then resets their password in DS, this
information was not sent to AD. Then if the user logged in AD after
resetting their password in DS they still get forced to change their
password again in AD.
Fix Description:
When sending a password update to AD, and AD is enforcing password must
be reset, check if the user's did reset thier password. If so, set the
correct "pwdLastSet" value to prevent AD from forcing that user to
change their password again.
But this only works going from DS to AD. The information needed to make
it work from AD -> DS is not available to passSync, and if it was available
it could not be correctly sent to DS anyway (not without a major redesign).
Side Note:
Also moved iand consolidated the function "fetch_attr" to util.c. It
was reused and redefined in many plugins. So I added the definition
to slapi-plugin.h and removed the duplicate definitions.
https://pagure.io/389-ds-base/issue/49950
Reviewed by: tbordaz(Thanks!)
(cherry picked from commit d9437be2e60fdbd6a5f1364f5887e1a3c89cda68)
(cherry picked from commit ac500d378aa22d5e818b110074ac9cd3e421e38d)
---
ldap/servers/plugins/automember/automember.c | 20 -----
ldap/servers/plugins/linkedattrs/fixup_task.c | 20 -----
ldap/servers/plugins/memberof/memberof.c | 17 ----
.../plugins/posix-winsync/posix-group-task.c | 18 +---
.../replication/repl5_replica_config.c | 13 ---
.../replication/windows_protocol_util.c | 90 ++++++++++++++-----
.../plugins/schema_reload/schema_reload.c | 17 ----
ldap/servers/plugins/syntaxes/validate_task.c | 20 -----
ldap/servers/slapd/slapi-plugin.h | 2 +
ldap/servers/slapd/task.c | 17 ----
ldap/servers/slapd/test-plugins/sampletask.c | 16 ----
ldap/servers/slapd/util.c | 17 ++++
12 files changed, 89 insertions(+), 178 deletions(-)
diff --git a/ldap/servers/plugins/automember/automember.c b/ldap/servers/plugins/automember/automember.c
index c91aa4e8e..d982d49a3 100644
--- a/ldap/servers/plugins/automember/automember.c
+++ b/ldap/servers/plugins/automember/automember.c
@@ -74,7 +74,6 @@ static void automember_free_regex_rule(struct automemberRegexRule *rule);
static int automember_parse_grouping_attr(char *value, char **grouping_attr, char **grouping_value);
static int automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc *ldif_fd);
static int automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char *grouping_attr, char *grouping_value, PRFileDesc *ldif_fd);
-const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
/*
* task functions
@@ -1927,25 +1926,6 @@ typedef struct _task_data
int scope;
} task_data;
-/*
- * extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Value *val = NULL;
- Slapi_Attr *attr;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0) {
- return default_val;
- }
- slapi_attr_first_value(attr, &val);
-
- return slapi_value_get_string(val);
-}
-
static void
automember_task_destructor(Slapi_Task *task)
{
diff --git a/ldap/servers/plugins/linkedattrs/fixup_task.c b/ldap/servers/plugins/linkedattrs/fixup_task.c
index 900ee1135..4929714b4 100644
--- a/ldap/servers/plugins/linkedattrs/fixup_task.c
+++ b/ldap/servers/plugins/linkedattrs/fixup_task.c
@@ -22,7 +22,6 @@ static void linked_attrs_fixup_task_thread(void *arg);
static void linked_attrs_fixup_links(struct configEntry *config);
static int linked_attrs_remove_backlinks_callback(Slapi_Entry *e, void *callback_data);
static int linked_attrs_add_backlinks_callback(Slapi_Entry *e, void *callback_data);
-static const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
/*
* Function Implementations
@@ -459,22 +458,3 @@ done:
return rc;
}
-
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0) {
- return default_val;
- }
-
- slapi_attr_first_value(attr, &val);
-
- return slapi_value_get_string(val);
-}
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index 87313ff19..26236dc68 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -142,7 +142,6 @@ static int memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *con
static int memberof_modop_one_replace_r(Slapi_PBlock *pb, MemberOfConfig *config, int mod_op, Slapi_DN *group_sdn, Slapi_DN *op_this_sdn, Slapi_DN *replace_with_sdn, Slapi_DN *op_to_sdn, memberofstringll *stack);
static int memberof_task_add(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter, int *returncode, char *returntext, void *arg);
static void memberof_task_destructor(Slapi_Task *task);
-static const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
static void memberof_fixup_task_thread(void *arg);
static int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *td);
static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);
@@ -2871,22 +2870,6 @@ done:
"memberof_fixup_task_thread - refcount decremented.\n");
}
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
- slapi_attr_first_value(attr, &val);
- return slapi_value_get_string(val);
-}
-
int
memberof_task_add(Slapi_PBlock *pb,
Slapi_Entry *e,
diff --git a/ldap/servers/plugins/posix-winsync/posix-group-task.c b/ldap/servers/plugins/posix-winsync/posix-group-task.c
index b4c507595..d8b6addd4 100644
--- a/ldap/servers/plugins/posix-winsync/posix-group-task.c
+++ b/ldap/servers/plugins/posix-winsync/posix-group-task.c
@@ -42,22 +42,6 @@ posix_group_fixup_task_thread(void *arg);
static int
posix_group_fix_memberuid_callback(Slapi_Entry *e, void *callback_data);
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
- slapi_attr_first_value(attr, &val);
- return slapi_value_get_string(val);
-}
-
/* e configEntry */
int
posix_group_task_add(Slapi_PBlock *pb __attribute__((unused)),
@@ -82,7 +66,7 @@ posix_group_task_add(Slapi_PBlock *pb __attribute__((unused)),
/* get arg(s) */
/* default: set replication basedn */
- if ((dn = fetch_attr(e, "basedn", slapi_sdn_get_dn(posix_winsync_config_get_suffix()))) == NULL) {
+ if ((dn = fetch_attr(e, "basedn", (char *)slapi_sdn_get_dn(posix_winsync_config_get_suffix()))) == NULL) {
*returncode = LDAP_OBJECT_CLASS_VIOLATION;
rv = SLAPI_DSE_CALLBACK_ERROR;
goto out;
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
index ea430d9a4..84e02639b 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -1353,19 +1353,6 @@ replica_execute_cleanruv_task(Object *r, ReplicaId rid, char *returntext __attri
return LDAP_SUCCESS;
}
-const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
-
- slapi_attr_first_value(attr, &val);
- return slapi_value_get_string(val);
-}
-
static int
replica_cleanall_ruv_task(Slapi_PBlock *pb __attribute__((unused)),
Slapi_Entry *e,
diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index f350b6d34..f6898d018 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -720,39 +720,79 @@ send_password_modify(Slapi_DN *sdn,
} else {
Slapi_Attr *attr = NULL;
int force_reset_pw = 0;
+ int pwd_already_reset = 0;
+ int ds_must_change = config_get_pw_must_change();
+
/*
- * If AD entry has password must change flag is set,
- * we keep the flag (pwdLastSet == 0).
- * msdn.microsoft.com: Windows Dev Centor - Desktop
- * To force a user to change their password at next logon,
- * set the pwdLastSet attribute to zero (0).
- */
+ * If AD entry has password must change flag is set,
+ * we keep the flag (pwdLastSet == 0).
+ * msdn.microsoft.com: Windows Dev Centor - Desktop
+ * To force a user to change their password at next logon,
+ * set the pwdLastSet attribute to zero (0).
+ */
if (remote_entry &&
(0 == slapi_entry_attr_find(remote_entry, "pwdLastSet", &attr)) &&
- attr) {
+ attr)
+ {
Slapi_Value *v = NULL;
int i = 0;
+
for (i = slapi_attr_first_value(attr, &v);
v && (i != -1);
- i = slapi_attr_next_value(attr, i, &v)) {
+ i = slapi_attr_next_value(attr, i, &v))
+ {
const char *s = slapi_value_get_string(v);
if (NULL == s) {
continue;
}
if (0 == strcmp(s, "0")) {
- slapi_log_err(SLAPI_LOG_REPL, windows_repl_plugin_name,
- "%s: AD entry %s set \"user must change password at next logon\". ",
- agmt_get_long_name(prp->agmt), slapi_entry_get_dn(remote_entry));
force_reset_pw = 1;
+ if (ds_must_change) {
+ /*
+ * DS already enforces "password must be changed after reset".
+ * Do an internal search and check the passwordExpirationtime
+ * to see if is it actually needs to be reset. If it doesn't,
+ * then set pwdLastSet to -1
+ */
+ char *expiration_val;
+ int rc = 0;
+ Slapi_DN *local_sdn = NULL;
+
+ rc = map_entry_dn_inbound(remote_entry, &local_sdn, prp->agmt);
+ if ((0 == rc) && local_sdn) {
+ Slapi_Entry *local_entry = NULL;
+ /* Get the local entry if it exists */
+ rc = windows_get_local_entry(local_sdn, &local_entry);
+ if ((0 == rc) && local_entry) {
+ expiration_val = (char *)fetch_attr(local_entry, "passwordExpirationtime", NULL);
+ if (expiration_val && parse_genTime(expiration_val) != NO_TIME){
+ /* The user did reset their password */
+ slapi_log_err(SLAPI_LOG_REPL, windows_repl_plugin_name,
+ "send_password_modify - entry (%s) password was reset by user send that info to AD\n",
+ slapi_sdn_get_dn(local_sdn));
+ pwd_already_reset = 1;
+ force_reset_pw = 0;
+ }
+ slapi_entry_free(local_entry);
+ }
+ }
+ slapi_sdn_free(&local_sdn);
+ } else {
+ slapi_log_err(SLAPI_LOG_REPL, windows_repl_plugin_name,
+ "%s: AD entry %s set \"user must change password at next logon\n",
+ agmt_get_long_name(prp->agmt), slapi_entry_get_dn(remote_entry));;
+ }
}
}
}
- /* We will attempt to bind to AD with the new password first. We do
- * this to avoid playing a password change that originated from AD
- * back to AD. If we just played the password change back, then
- * both sides would be in sync, but AD would contain the new password
- * twice in it's password history, which undermines the password
- * history policies in AD. */
+ /*
+ * We will attempt to bind to AD with the new password first. We do
+ * this to avoid playing a password change that originated from AD
+ * back to AD. If we just played the password change back, then
+ * both sides would be in sync, but AD would contain the new password
+ * twice in it's password history, which undermines the password
+ * history policies in AD.
+ */
if (windows_check_user_password(prp->conn, sdn, password)) {
char *quoted_password = NULL;
/* AD wants the password in quotes ! */
@@ -792,9 +832,18 @@ send_password_modify(Slapi_DN *sdn,
pw_mod.mod_bvalues = bvals;
pw_mods[0] = &pw_mod;
- if (force_reset_pw) {
- reset_bv.bv_len = 1;
- reset_bv.bv_val = "0";
+
+ if (force_reset_pw || pwd_already_reset) {
+ if (force_reset_pw) {
+ reset_bv.bv_val = "0";
+ reset_bv.bv_len = 1;
+ } else if (pwd_already_reset) {
+ /* Password was reset by the user, there is no
+ * need to make the user change their password
+ * again in AD so set pwdLastSet to -1 */
+ reset_bv.bv_val = "-1";
+ reset_bv.bv_len = 2;
+ }
reset_bvals[0] = &reset_bv;
reset_bvals[1] = NULL;
reset_pw_mod.mod_type = "pwdLastSet";
@@ -807,7 +856,6 @@ send_password_modify(Slapi_DN *sdn,
}
pw_return = windows_conn_send_modify(prp->conn, slapi_sdn_get_dn(sdn), pw_mods, NULL, NULL);
-
slapi_ch_free((void **)&unicode_password);
}
PR_smprintf_free(quoted_password);
diff --git a/ldap/servers/plugins/schema_reload/schema_reload.c b/ldap/servers/plugins/schema_reload/schema_reload.c
index ee3b00c3c..c2399e5c3 100644
--- a/ldap/servers/plugins/schema_reload/schema_reload.c
+++ b/ldap/servers/plugins/schema_reload/schema_reload.c
@@ -187,23 +187,6 @@ schemareload_thread(void *arg)
"schemareload_thread <-- refcount decremented.\n");
}
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
- slapi_attr_first_value(attr, &val);
-
- return slapi_value_get_string(val);
-}
-
static void
schemareload_destructor(Slapi_Task *task)
{
diff --git a/ldap/servers/plugins/syntaxes/validate_task.c b/ldap/servers/plugins/syntaxes/validate_task.c
index 2c625ba71..afec9ef7a 100644
--- a/ldap/servers/plugins/syntaxes/validate_task.c
+++ b/ldap/servers/plugins/syntaxes/validate_task.c
@@ -43,7 +43,6 @@ static int syntax_validate_task_add(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entr
static void syntax_validate_task_destructor(Slapi_Task *task);
static void syntax_validate_task_thread(void *arg);
static int syntax_validate_task_callback(Slapi_Entry *e, void *callback_data);
-static const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
static void syntax_validate_set_plugin_id(void *plugin_id);
static void *syntax_validate_get_plugin_id(void);
@@ -258,25 +257,6 @@ bail:
return rc;
}
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0) {
- return default_val;
- }
-
- slapi_attr_first_value(attr, &val);
-
- return slapi_value_get_string(val);
-}
-
/*
* Plug-in identity management helper functions
*/
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 4b75654e7..bdad4e59e 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -8294,6 +8294,8 @@ int32_t slapi_atomic_decr_32(int32_t *ptr, int memorder);
*/
uint64_t slapi_atomic_decr_64(uint64_t *ptr, int memorder);
+/* helper function */
+const char * fetch_attr(Slapi_Entry *e, const char *attrname, char *default_val);
#ifdef __cplusplus
}
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
index 3f9d5d995..698ee19b9 100644
--- a/ldap/servers/slapd/task.c
+++ b/ldap/servers/slapd/task.c
@@ -80,7 +80,6 @@ static void destroy_task(time_t when, void *arg);
static int task_modify(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter, int *returncode, char *returntext, void *arg);
static int task_deny(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter, int *returncode, char *returntext, void *arg);
static void task_generic_destructor(Slapi_Task *task);
-static const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
static Slapi_Entry *get_internal_entry(Slapi_PBlock *pb, char *dn);
static void modify_internal_entry(char *dn, LDAPMod **mods);
static void fixup_tombstone_task_destructor(Slapi_Task *task);
@@ -684,22 +683,6 @@ destroy_task(time_t when, void *arg)
slapi_ch_free((void **)&task);
}
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
- slapi_attr_first_value(attr, &val);
- return slapi_value_get_string(val);
-}
-
/* supply the pblock, destroy it when you're done */
static Slapi_Entry *
get_internal_entry(Slapi_PBlock *pb, char *dn)
diff --git a/ldap/servers/slapd/test-plugins/sampletask.c b/ldap/servers/slapd/test-plugins/sampletask.c
index d04f21b3d..22d43dd48 100644
--- a/ldap/servers/slapd/test-plugins/sampletask.c
+++ b/ldap/servers/slapd/test-plugins/sampletask.c
@@ -116,22 +116,6 @@ task_sampletask_thread(void *arg)
slapi_task_finish(task, rv);
}
-/* extract a single value from the entry (as a string) -- if it's not in the
- * entry, the default will be returned (which can be NULL).
- * you do not need to free anything returned by this.
- */
-static const char *
-fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val)
-{
- Slapi_Attr *attr;
- Slapi_Value *val = NULL;
-
- if (slapi_entry_attr_find(e, attrname, &attr) != 0)
- return default_val;
- slapi_attr_first_value(attr, &val);
- return slapi_value_get_string(val);
-}
-
static void
task_sampletask_destructor(Slapi_Task *task)
{
diff --git a/ldap/servers/slapd/util.c b/ldap/servers/slapd/util.c
index cb46efb3d..8563c5d27 100644
--- a/ldap/servers/slapd/util.c
+++ b/ldap/servers/slapd/util.c
@@ -1579,3 +1579,20 @@ slapi_create_errormsg(
va_end(ap);
}
}
+
+/*
+ * Extract a single value from an entry (as a string) -- if it's not in the
+ * entry, the default will be returned (which can be NULL). You do not need
+ * to free the returned string value.
+ */
+const char *
+fetch_attr(Slapi_Entry *e, const char *attrname, char *default_val)
+{
+ Slapi_Attr *attr;
+ Slapi_Value *val = NULL;
+
+ if (slapi_entry_attr_find(e, attrname, &attr) != 0)
+ return default_val;
+ slapi_attr_first_value(attr, &val);
+ return slapi_value_get_string(val);
+}
--
2.17.2