URL: https://github.com/freeipa/freeipa/pull/4445
Author: abbra
Title: #4445: [backport][ipa-4-6] ipa-pwd-extop: don't check password policy
for non-Kerberos account set by DM or a passsync manager
Action: opened
PR body:
"""
This PR was opened manually because PR #4417 was pushed to master and backport
to ipa-4-6 is required.
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/4445/head:pr4445
git checkout pr4445
From 607cb5d1010d50b959954886880e92356b8ffb7d Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 10 Jul 2018 18:05:19 +0300
Subject: [PATCH 1/8] Fix indentation levels
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
.../ipa-slapi-plugins/ipa-pwd-extop/common.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index 61b46904ab..d3cd3a72b6 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -286,8 +286,8 @@ static struct ipapwd_krbcfg *ipapwd_getConfig(void)
* slapi_pblock_destroy(pb)
*/
static int pwd_get_values(const Slapi_Entry *ent, const char *attrname,
- Slapi_ValueSet** results, char** actual_type_name,
- int *buffer_flags)
+ Slapi_ValueSet** results, char** actual_type_name,
+ int *buffer_flags)
{
int flags=0;
int type_name_disposition = 0;
@@ -560,7 +560,7 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data)
LOG_TRACE("No password policy, use defaults");
}
break;
- case IPA_CHANGETYPE_ADMIN:
+ case IPA_CHANGETYPE_ADMIN:
/* The expiration date needs to be older than the current time
* otherwise the KDC may not immediately register the password
* as expired. The last password change needs to match the
@@ -636,7 +636,7 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data)
}
/* Searches the dn in directory,
- * If found : fills in slapi_entry structure and returns 0
+ * If found : fills in slapi_entry structure and returns 0
* If NOT found : returns the search result as LDAP_NO_SUCH_OBJECT
*/
int ipapwd_getEntry(const char *dn, Slapi_Entry **e2, char **attrlist)
@@ -795,22 +795,21 @@ int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
slapi_mods_add_mod_values(smods, LDAP_MOD_REPLACE,
"krbPrincipalKey", svals);
- /* krbLastPwdChange is used to tell whether a host entry has a
- * keytab so don't set it on hosts.
- */
+ /* krbLastPwdChange is used to tell whether a host entry has a
+ * keytab so don't set it on hosts. */
if (!is_host) {
- /* change Last Password Change field with the current date */
+ /* change Last Password Change field with the current date */
ret = ipapwd_setdate(data->target, smods, "krbLastPwdChange",
data->timeNow, false);
if (ret != LDAP_SUCCESS)
goto free_and_return;
- /* set Password Expiration date */
+ /* set Password Expiration date */
ret = ipapwd_setdate(data->target, smods, "krbPasswordExpiration",
data->expireTime, (data->expireTime == 0));
if (ret != LDAP_SUCCESS)
goto free_and_return;
- }
+ }
}
if (nt && is_smb) {
From af151ed1310739d5e3df8efdf2089deccc46d5ee Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Fri, 6 Jul 2018 11:07:48 +0300
Subject: [PATCH 2/8] ipatests: allow changing sysaccount passwords as
cn=Directory Manager
Extend ldappasswd_sysaccount_change() helper to allow changing
passwords as a cn=Directory Manager.
Related to: https://pagure.io/freeipa/issue/7181
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
ipatests/pytest_ipa/integration/tasks.py | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py
index 1db4fdd6d0..14c06a4c6f 100755
--- a/ipatests/pytest_ipa/integration/tasks.py
+++ b/ipatests/pytest_ipa/integration/tasks.py
@@ -1629,15 +1629,23 @@ def ldappasswd_user_change(user, oldpw, newpw, master):
master.run_command(args)
-def ldappasswd_sysaccount_change(user, oldpw, newpw, master):
+def ldappasswd_sysaccount_change(user, oldpw, newpw, master, use_dirman=False):
container_sysaccounts = dict(DEFAULT_CONFIG)['container_sysaccounts']
basedn = master.domain.basedn
userdn = "uid={},{},{}".format(user, container_sysaccounts, basedn)
master_ldap_uri = "ldap://{}".format(master.hostname)
- args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw,
- '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri]
+ if use_dirman:
+ args = [paths.LDAPPASSWD, '-D',
+ str(master.config.dirman_dn), # pylint: disable=no-member
+ '-w', master.config.dirman_password,
+ '-a', oldpw,
+ '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri,
+ userdn]
+ else:
+ args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw,
+ '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri]
master.run_command(args)
From df6343674cf609b02c5c3d5f2defa10a5d800666 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Thu, 30 Aug 2018 19:10:17 +0300
Subject: [PATCH 3/8] ipatests: test sysaccount password change with a password
policy applied
ipa-pwd-extop plugin had a bug which prevented a cn=Directory Manager
to change a password to a value that is not allowed by an associated
password policy. Password policy checks should not apply to any
operations done as cn=Directory Manager.
The test creates a system account with associated policy that prevents
password reuse. It then goes to try to change a password three times:
- as a user: must succeeed
- as a cn=Directory Manager: must succeed even with a password re-use
- as a user again: must fail due to password re-use
Related: https://pagure.io/freeipa/issue/7181
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
ipatests/test_integration/test_commands.py | 100 ++++++++++++++++++---
1 file changed, 89 insertions(+), 11 deletions(-)
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index 15989876d1..43f99e1eb1 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -17,6 +17,7 @@
import time
import paramiko
import pytest
+from subprocess import CalledProcessError
from cryptography.hazmat.backends import default_backend
from cryptography import x509
@@ -28,6 +29,7 @@
from ipapython.dn import DN
from ipatests.test_integration.base import IntegrationTest
+
from ipatests.pytest_ipa.integration import tasks
from ipaplatform.tasks import tasks as platform_tasks
from ipatests.pytest_ipa.integration.create_external_ca import ExternalCA
@@ -107,25 +109,19 @@ def test_change_sysaccount_password_issue7561(self):
tf = NamedTemporaryFile()
ldif_file = tf.name
entry_ldif = textwrap.dedent("""
- dn: uid=system,cn=sysaccounts,cn=etc,{base_dn}
+ dn: uid={sysuser},cn=sysaccounts,cn=etc,{base_dn}
changetype: add
objectclass: account
objectclass: simplesecurityobject
- uid: system
+ uid: {sysuser}
userPassword: {original_passwd}
passwordExpirationTime: 20380119031407Z
nsIdleTimeout: 0
""").format(
base_dn=base_dn,
- original_passwd=original_passwd)
- master.put_file_contents(ldif_file, entry_ldif)
- arg = ['ldapmodify',
- '-h', master.hostname,
- '-p', '389', '-D',
- str(master.config.dirman_dn), # pylint: disable=no-member
- '-w', master.config.dirman_password,
- '-f', ldif_file]
- master.run_command(arg)
+ original_passwd=original_passwd,
+ sysuser=sysuser)
+ tasks.ldapmodify_dm(master, entry_ldif)
tasks.ldappasswd_sysaccount_change(sysuser, original_passwd,
new_passwd, master)
@@ -232,6 +228,88 @@ def test_ldapmodify_password_issue7601(self):
assert newkrblastpwdchange != newkrblastpwdchange2
assert newkrbexp != newkrbexp2
+ def test_change_sysaccount_password_issue7181(self):
+ """
+ Test how a password change performed by a cn=Directory Manager
+ works against a non-Kerberos account with a policy preventing
+ re-use of previously used passwords
+ """
+ sysuser = 'forcedpolicy'
+ policy_group = 'forcedpolicy'
+ original_passwd = 'Secret123'
+ new_passwd = 'userPasswd123'
+
+ master = self.master
+
+ # Add a group with a custom password policy
+ tasks.kinit_admin(self.master)
+ result = master.run_command(
+ ["ipa", "group-add", policy_group]
+ )
+ assert 'Added group "{}"'.format(policy_group) in result.stdout_text
+
+ result = master.run_command(
+ ["ipa", "pwpolicy-add", policy_group,
+ "--history=5", "--priority=1"],
+ )
+ assert 'History size: 5' in result.stdout_text
+
+ # Add a system account and add it to a group managed by the policy
+ base_dn = str(master.domain.basedn) # pylint: disable=no-member
+ entry_ldif = textwrap.dedent("""
+ dn: uid={account_name},cn=sysaccounts,cn=etc,{base_dn}
+ changetype: add
+ objectclass: account
+ objectclass: simplesecurityobject
+ objectclass: nsMemberOf
+ objectclass: krbPrincipalAux
+ uid: {account_name}
+ userPassword: {original_passwd}
+ passwordExpirationTime: 20380119031407Z
+ nsIdleTimeout: 0
+ memberOf: cn={group_name},cn=groups,cn=accounts,{base_dn}
+ """).format(
+ account_name=sysuser,
+ group_name=policy_group,
+ base_dn=base_dn,
+ original_passwd=original_passwd)
+
+ tasks.ldapmodify_dm(master, entry_ldif)
+
+ # For an LDAP object not managed by IPA we have to use
+ # --addattr to add it as a member of a group
+ value = "member=uid={account_name},cn=sysaccounts,cn=etc,{base_dn}"
+ result = master.run_command(
+ ["ipa", "group-mod", policy_group,
+ "--addattr={value}".format(
+ value=value.format(
+ account_name=sysuser,
+ base_dn=base_dn
+ )
+ )],
+ )
+ assert 'Modified group "{}"'.format(policy_group) in result.stdout_text
+
+ # Now try to change password thrice:
+ # as a user, as a cn=Directory Manager, and as a user again
+ # If ticket 7181 is not fixed, the second change will fail
+ # Third one must fail as we are reusing the password as non-DM
+ tasks.ldappasswd_sysaccount_change(sysuser, original_passwd,
+ new_passwd, master,
+ use_dirman=False)
+ tasks.ldappasswd_sysaccount_change(sysuser, new_passwd,
+ new_passwd, master,
+ use_dirman=True)
+ try:
+ tasks.ldappasswd_sysaccount_change(sysuser, new_passwd,
+ original_passwd, master,
+ use_dirman=False)
+ except CalledProcessError as e:
+ if e.returncode != 1:
+ raise
+ else:
+ pytest.fail("Password change violating policy did not fail")
+
def test_change_selinuxusermaporder(self):
"""
An update file meant to ensure a more sane default was
From ecf89c93de41d113cbca5274476b3dc4211cc807 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 12 Mar 2019 16:09:50 +0200
Subject: [PATCH 4/8] ipa-pwd-extop: use SLAPI_BIND_TARGET_SDN
SLAPI_BIND_TARGET_DN is deprecated since 2011 by 389-ds team,
see commit f6397113666f06848412bb12f754f04258cfa5fa in 389-ds:
https://pagure.io/389-ds-base/c/f6397113666f06848412bb12f754f04258cfa5fa?branch=master
Use SLAPI_BIND_TARGET_SDN instead and move internal ipa-pwd-extop
helpers to accept Slapi_DN references rather than strings.
Related: https://pagure.io/freeipa/issue/7181
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
.../ipa-slapi-plugins/ipa-pwd-extop/common.c | 31 +++++++---
.../ipa-pwd-extop/ipa_pwd_extop.c | 29 +++++-----
.../ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h | 2 +-
.../ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 58 +++++++++++--------
4 files changed, 74 insertions(+), 46 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index d3cd3a72b6..dd42760099 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -72,6 +72,7 @@ static struct ipapwd_krbcfg *ipapwd_getConfig(void)
Slapi_Entry *config_entry = NULL;
Slapi_Attr *a;
Slapi_Value *v;
+ Slapi_DN *sdn = NULL;
BerElement *be = NULL;
ber_tag_t tag, tvno;
ber_int_t ttype;
@@ -107,7 +108,9 @@ static struct ipapwd_krbcfg *ipapwd_getConfig(void)
}
/* get the Realm Container entry */
- ret = ipapwd_getEntry(ipa_realm_dn, &realm_entry, NULL);
+ sdn = slapi_sdn_new_dn_byval(ipa_realm_dn);
+ ret = ipapwd_getEntry(sdn, &realm_entry, NULL);
+ slapi_sdn_free(&sdn);
if (ret != LDAP_SUCCESS) {
LOG_FATAL("No realm Entry?\n");
goto free_and_error;
@@ -212,7 +215,9 @@ static struct ipapwd_krbcfg *ipapwd_getConfig(void)
slapi_entry_free(realm_entry);
/* get the Realm Container entry */
- ret = ipapwd_getEntry(ipa_pwd_config_dn, &config_entry, NULL);
+ sdn = slapi_sdn_new_dn_byval(ipa_pwd_config_dn);
+ ret = ipapwd_getEntry(sdn, &config_entry, NULL);
+ slapi_sdn_free(&sdn);
if (ret != LDAP_SUCCESS) {
LOG_FATAL("No config Entry? Impossible!\n");
goto free_and_error;
@@ -236,7 +241,9 @@ static struct ipapwd_krbcfg *ipapwd_getConfig(void)
if (ipapwd_fips_enabled()) {
LOG("FIPS mode is enabled, NT hashes are not allowed.\n");
} else {
- ret = ipapwd_getEntry(ipa_etc_config_dn, &config_entry, NULL);
+ sdn = slapi_sdn_new_dn_byval(ipa_etc_config_dn);
+ ret = ipapwd_getEntry(sdn, &config_entry, NULL);
+ slapi_sdn_free(&sdn);
if (ret != LDAP_SUCCESS) {
LOG_FATAL("No config Entry?\n");
goto free_and_error;
@@ -639,22 +646,28 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data)
* If found : fills in slapi_entry structure and returns 0
* If NOT found : returns the search result as LDAP_NO_SUCH_OBJECT
*/
-int ipapwd_getEntry(const char *dn, Slapi_Entry **e2, char **attrlist)
+int ipapwd_getEntry(Slapi_DN *sdn, Slapi_Entry **e2, char **attrlist)
{
- Slapi_DN *sdn;
int search_result = 0;
+ Slapi_DN *local_sdn = NULL;
LOG_TRACE("=>\n");
- sdn = slapi_sdn_new_dn_byref(dn);
- search_result = slapi_search_internal_get_entry(sdn, attrlist, e2,
+ if (sdn == NULL) {
+ LOG_TRACE("No entry to fetch!\n");
+ return LDAP_PARAM_ERROR;
+ }
+
+ local_sdn = slapi_sdn_dup(sdn);
+ search_result = slapi_search_internal_get_entry(local_sdn, attrlist, e2,
ipapwd_plugin_id);
if (search_result != LDAP_SUCCESS) {
- LOG_TRACE("No such entry-(%s), err (%d)\n", dn, search_result);
+ LOG_TRACE("No such entry-(%s), err (%d)\n",
+ slapi_sdn_get_dn(sdn), search_result);
}
- slapi_sdn_free(&sdn);
LOG_TRACE("<= result: %d\n", search_result);
+ slapi_sdn_free(&local_sdn);
return search_result;
}
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 7e52a8855a..a6d91000fd 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -382,15 +382,18 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
}
}
- /* Now we have the DN, look for the entry */
- ret = ipapwd_getEntry(dn, &targetEntry, attrlist);
- /* If we can't find the entry, then that's an error */
- if (ret) {
- /* Couldn't find the entry, fail */
- errMesg = "No such Entry exists.\n" ;
- rc = LDAP_NO_SUCH_OBJECT;
- goto free_and_return;
- }
+ /* Now we have the DN, look for the entry */
+ target_sdn = slapi_sdn_new_dn_byval(dn);
+ ret = ipapwd_getEntry(target_sdn, &targetEntry, attrlist);
+ slapi_sdn_free(&target_sdn);
+
+ /* If we can't find the entry, then that's an error */
+ if (ret) {
+ /* Couldn't find the entry, fail */
+ errMesg = "No such Entry exists.\n" ;
+ rc = LDAP_NO_SUCH_OBJECT;
+ goto free_and_return;
+ }
if (dn) {
Slapi_DN *bind_sdn;
@@ -1821,7 +1824,7 @@ static int ipapwd_start( Slapi_PBlock *pb )
krb5_context krbctx = NULL;
krb5_error_code krberr;
char *realm = NULL;
- char *config_dn;
+ Slapi_DN *config_sdn = NULL;
Slapi_Entry *config_entry = NULL;
int ret;
@@ -1834,13 +1837,13 @@ static int ipapwd_start( Slapi_PBlock *pb )
return LDAP_SUCCESS;
}
- if (slapi_pblock_get(pb, SLAPI_TARGET_DN, &config_dn) != 0) {
+ if (slapi_pblock_get(pb, SLAPI_TARGET_SDN, &config_sdn) != 0) {
LOG_FATAL("No config DN?\n");
ret = LDAP_OPERATIONS_ERROR;
goto done;
}
- if (ipapwd_getEntry(config_dn, &config_entry, NULL) != LDAP_SUCCESS) {
+ if (ipapwd_getEntry(config_sdn, &config_entry, NULL) != LDAP_SUCCESS) {
LOG_FATAL("No config Entry extop?\n");
ret = LDAP_SUCCESS;
goto done;
@@ -1869,7 +1872,7 @@ static int ipapwd_start( Slapi_PBlock *pb )
goto done;
}
- ipa_pwd_config_dn = slapi_ch_strdup(config_dn);
+ ipa_pwd_config_dn = slapi_ch_strdup(slapi_sdn_get_dn(config_sdn));
if (!ipa_pwd_config_dn) {
LOG_OOM();
ret = LDAP_OPERATIONS_ERROR;
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
index e96aa44d2f..d21d74ec12 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
@@ -117,7 +117,7 @@ int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
int ipapwd_gen_checks(Slapi_PBlock *pb, char **errMesg,
struct ipapwd_krbcfg **config, int check_flags);
int ipapwd_CheckPolicy(struct ipapwd_data *data);
-int ipapwd_getEntry(const char *dn, Slapi_Entry **e2, char **attrlist);
+int ipapwd_getEntry(Slapi_DN *sdn, Slapi_Entry **e2, char **attrlist);
int ipapwd_get_cur_kvno(Slapi_Entry *target);
int ipapwd_setdate(Slapi_Entry *source, Slapi_Mods *smods, const char *attr,
time_t date, bool remove);
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index 9aef2f7d7d..cf88d0f370 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -129,6 +129,7 @@ static char *ipapwd_getIpaConfigAttr(const char *attr)
const char *attrs_list[] = {attr, 0};
char *value = NULL;
char *dn = NULL;
+ Slapi_DN *sdn = NULL;
int ret;
dn = slapi_ch_smprintf("cn=ipaconfig,cn=etc,%s", ipa_realm_tree);
@@ -137,9 +138,12 @@ static char *ipapwd_getIpaConfigAttr(const char *attr)
goto done;
}
- ret = ipapwd_getEntry(dn, &entry, (char **) attrs_list);
+ /* _byref() will take ownership of the dn */
+ sdn = slapi_sdn_new_dn_byref(dn);
+
+ ret = ipapwd_getEntry(sdn, &entry, (char **) attrs_list);
if (ret) {
- LOG("failed to retrieve config entry: %s\n", dn);
+ LOG("failed to retrieve config entry: %s\n", slapi_sdn_get_dn(sdn));
goto done;
}
@@ -147,7 +151,7 @@ static char *ipapwd_getIpaConfigAttr(const char *attr)
done:
slapi_entry_free(entry);
- slapi_ch_free_string(&dn);
+ slapi_sdn_free(&sdn);
return value;
}
@@ -210,7 +214,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
char *errMesg = "Internal operations error\n";
struct slapi_entry *e = NULL;
char *userpw = NULL;
- char *dn = NULL;
+ Slapi_DN *sdn = NULL;
struct ipapwd_operation *pwdop = NULL;
void *op;
int is_repl_op, is_root, is_krb, is_smb, is_ipant;
@@ -292,8 +296,10 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
* a valid krbPrincipalKey
*/
if (has_krbprincipalkey(e)) {
- slapi_pblock_get(pb, SLAPI_TARGET_DN, &dn);
- LOG("User Life Cycle: %s is a activated stage user (with prehashed password and krb keys)\n", dn ? dn : "unknown");
+ slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
+ LOG("User Life Cycle: %s is a activated stage user "
+ "(with prehashed password and krb keys)\n",
+ sdn ? slapi_sdn_get_dn(sdn) : "unknown");
return 0;
}
@@ -317,7 +323,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
}
/* Get target DN */
- ret = slapi_pblock_get(pb, SLAPI_TARGET_DN, &dn);
+ ret = slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (ret) {
rc = LDAP_OPERATIONS_ERROR;
goto done;
@@ -441,7 +447,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
#define NTHASH_REGEN_VAL "MagicRegen"
#define NTHASH_REGEN_LEN sizeof(NTHASH_REGEN_VAL)
static int ipapwd_regen_nthash(Slapi_PBlock *pb, Slapi_Mods *smods,
- char *dn, struct slapi_entry *entry,
+ const char *dn, struct slapi_entry *entry,
struct ipapwd_krbcfg *krbcfg);
/* PRE MOD Operation:
@@ -463,8 +469,8 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
Slapi_Mods *smods = NULL;
char *userpw = NULL;
char *unhashedpw = NULL;
- char *dn = NULL;
- Slapi_DN *tmp_dn;
+ Slapi_DN *sdn = NULL;
+ Slapi_DN *tmp_sdn;
struct slapi_entry *e = NULL;
struct ipapwd_operation *pwdop = NULL;
void *op;
@@ -570,14 +576,14 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
* pre-requisites */
/* Get target DN */
- ret = slapi_pblock_get(pb, SLAPI_TARGET_DN, &dn);
+ ret = slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (ret) {
rc = LDAP_OPERATIONS_ERROR;
goto done;
}
- tmp_dn = slapi_sdn_new_dn_byref(dn);
- if (tmp_dn) {
+ tmp_sdn = slapi_sdn_dup(sdn);
+ if (tmp_sdn) {
/* xxxPAR: Ideally SLAPI_MODIFY_EXISTING_ENTRY should be
* available but it turns out that is only true if you are
* a dbm backend pre-op plugin - lucky dbm backend pre-op
@@ -589,8 +595,8 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
*
slapi_pblock_get( pb, SLAPI_MODIFY_EXISTING_ENTRY, &e);
*/
- ret = slapi_search_internal_get_entry(tmp_dn, 0, &e, ipapwd_plugin_id);
- slapi_sdn_free(&tmp_dn);
+ ret = slapi_search_internal_get_entry(tmp_sdn, 0, &e, ipapwd_plugin_id);
+ slapi_sdn_free(&tmp_sdn);
if (ret != LDAP_SUCCESS) {
LOG("Failed to retrieve entry?!\n");
rc = LDAP_NO_SUCH_OBJECT;
@@ -617,7 +623,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
/* Make sense to call only if this entry has krb keys to source
* the nthash from */
if (is_krb) {
- rc = ipapwd_regen_nthash(pb, smods, dn, e, krbcfg);
+ rc = ipapwd_regen_nthash(pb, smods, slapi_sdn_get_dn(sdn), e, krbcfg);
} else {
rc = LDAP_UNWILLING_TO_PERFORM;
}
@@ -818,7 +824,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
/* Check Bind DN */
slapi_pblock_get(pb, SLAPI_CONN_DN, &binddn);
bdn = slapi_sdn_new_dn_byref(binddn);
- tdn = slapi_sdn_new_dn_byref(dn);
+ tdn = slapi_sdn_dup(sdn);
/* if the change is performed by someone else,
* it is an admin change that will require a new
@@ -841,7 +847,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
}
- pwdop->pwdata.dn = slapi_ch_strdup(dn);
+ pwdop->pwdata.dn = slapi_ch_strdup(slapi_sdn_get_dn(sdn));
pwdop->pwdata.timeNow = time(NULL);
pwdop->pwdata.target = e;
@@ -932,7 +938,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
}
static int ipapwd_regen_nthash(Slapi_PBlock *pb, Slapi_Mods *smods,
- char *dn, struct slapi_entry *entry,
+ const char *dn, struct slapi_entry *entry,
struct ipapwd_krbcfg *krbcfg)
{
Slapi_Attr *attr;
@@ -1388,7 +1394,9 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
};
struct berval *credentials = NULL;
Slapi_Entry *entry = NULL;
- char *dn = NULL;
+ Slapi_DN *target_sdn = NULL;
+ Slapi_DN *sdn = NULL;
+ const char *dn = NULL;
int method = 0;
bool syncreq;
bool otpreq;
@@ -1399,7 +1407,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
struct tm expire_tm;
/* get BIND parameters */
- ret |= slapi_pblock_get(pb, SLAPI_BIND_TARGET, &dn);
+ ret |= slapi_pblock_get(pb, SLAPI_BIND_TARGET_SDN, &target_sdn);
ret |= slapi_pblock_get(pb, SLAPI_BIND_METHOD, &method);
ret |= slapi_pblock_get(pb, SLAPI_BIND_CREDENTIALS, &credentials);
if (ret) {
@@ -1412,7 +1420,9 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
return 0;
/* Retrieve the user's entry. */
- ret = ipapwd_getEntry(dn, &entry, (char **) attrs_list);
+ sdn = slapi_sdn_dup(target_sdn);
+ dn = slapi_sdn_get_dn(sdn);
+ ret = ipapwd_getEntry(sdn, &entry, (char **) attrs_list);
if (ret) {
LOG("failed to retrieve user entry: %s\n", dn);
return 0;
@@ -1469,13 +1479,15 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
goto invalid_creds;
/* Attempt to write out kerberos keys for the user. */
- ipapwd_write_krb_keys(pb, dn, entry, credentials);
+ ipapwd_write_krb_keys(pb, discard_const(dn), entry, credentials);
slapi_entry_free(entry);
+ slapi_sdn_free(&sdn);
return 0;
invalid_creds:
slapi_entry_free(entry);
+ slapi_sdn_free(&sdn);
slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS,
NULL, NULL, 0, NULL);
return 1;
From 72d41b6ba3c601606e85325311ffacf511a043e8 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 12 Mar 2019 16:14:53 +0200
Subject: [PATCH 5/8] ipa-pwd-extop: don't check password policy for
non-Kerberos account set by DM or a passsync manager
Password changes performed by cn=Directory Manager are excluded from
password policy checks according to [1]. This is correctly handled by
ipa-pwd-extop in case of a normal Kerberos principal in IPA. However,
non-kerberos accounts were not excluded from the check.
As result, password updates for PKI CA admin account in o=ipaca were
failing if a password policy does not allow a password reuse. We are
re-setting the password for PKI CA admin in ipa-replica-prepare in case
the original directory manager's password was updated since creation of
`cacert.p12`.
Do password policy check for non-Kerberos accounts only if it was set by
a regular user or admin. Changes performed by a cn=Directory Manager and
passsync managers should be excluded from the policy check.
Fixes: https://pagure.io/freeipa/issue/7181
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
[1] https://access.redhat.com/documentation/en-us/red_hat_directory_server/10/html/administration_guide/user_account_management-managing_the_password_policy
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
.../ipa-slapi-plugins/ipa-pwd-extop/common.c | 25 +++++++++++++-
.../ipa-pwd-extop/ipa_pwd_extop.c | 26 ++++++++------
.../ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h | 2 ++
.../ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 34 +++++++++++++------
4 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index dd42760099..60dfe1b162 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -422,6 +422,7 @@ int ipapwd_getPolicy(const char *dn,
int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
int *is_root, int *is_krb, int *is_smb, int *is_ipant,
+ int *is_memberof,
char *attr, int acc)
{
Slapi_Value *sval;
@@ -440,6 +441,10 @@ int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
}
}
+ /* Default to not setting memberof flag: only set it for non-Kerberos principals
+ * when they have krbPrincipalAux but no krbPrincipalName */
+ *is_memberof = 0;
+
/* Check if this is a krbPrincial and therefore needs us to generate other
* hashes */
sval = slapi_value_new_string("krbPrincipalAux");
@@ -450,6 +455,24 @@ int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
*is_krb = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS, sval);
slapi_value_free(&sval);
+ /* If entry has krbPrincipalAux object class but lacks krbPrincipalName and
+ * memberOf attributes consider this not a Kerberos principal object. In
+ * FreeIPA krbPrincipalAux allows to store krbPwdPolicyReference attribute
+ * which is added by a CoS plugin configuration based on a memberOf
+ * attribute value.
+ * Note that slapi_entry_attr_find() returns 0 if attr exists, -1 for absence
+ */
+ if (*is_krb) {
+ Slapi_Attr *attr_prname = NULL;
+ Slapi_Attr *attr_memberof = NULL;
+ int has_prname = slapi_entry_attr_find(e, "krbPrincipalName", &attr_prname);
+ int has_memberOf = slapi_entry_attr_find(e, "memberOf", &attr_memberof);
+ if ((has_prname == -1) && (has_memberOf == 0)) {
+ *is_memberof = 1;
+ *is_krb = 0;
+ }
+ }
+
sval = slapi_value_new_string("sambaSamAccount");
if (!sval) {
rc = LDAP_OPERATIONS_ERROR;
@@ -655,7 +678,7 @@ int ipapwd_getEntry(Slapi_DN *sdn, Slapi_Entry **e2, char **attrlist)
if (sdn == NULL) {
LOG_TRACE("No entry to fetch!\n");
- return LDAP_PARAM_ERROR;
+ return LDAP_PARAM_ERROR;
}
local_sdn = slapi_sdn_dup(sdn);
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index a6d91000fd..f92706810d 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -206,7 +206,7 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
Slapi_Value *objectclass=NULL;
char *attrlist[] = {"*", "passwordHistory", NULL };
struct ipapwd_data pwdata;
- int is_krb, is_smb, is_ipant;
+ int is_krb, is_smb, is_ipant, is_memberof;
char *principal = NULL;
Slapi_PBlock *chpwop_pb = NULL;
Slapi_DN *target_sdn = NULL;
@@ -332,7 +332,7 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
/* Determine the target DN for this operation */
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &target_sdn);
if (target_sdn != NULL) {
- /* If there is a TARGET_DN we are consuming it */
+ /* If there is a TARGET_SDN we are consuming it */
slapi_pblock_set(pb, SLAPI_TARGET_SDN, NULL);
target_dn = slapi_sdn_get_ndn(target_sdn);
}
@@ -356,11 +356,11 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
}
slapi_sdn_free(&target_sdn);
- if (slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, dn )) {
- LOG_FATAL("slapi_pblock_set failed!\n");
- rc = LDAP_OPERATIONS_ERROR;
- goto free_and_return;
- }
+ if (slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, dn )) {
+ LOG_FATAL("slapi_pblock_set failed!\n");
+ rc = LDAP_OPERATIONS_ERROR;
+ goto free_and_return;
+ }
if (usetxn) {
Slapi_DN *sdn = slapi_sdn_new_dn_byref(dn);
@@ -469,6 +469,7 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
rc = ipapwd_entry_checks(pb, targetEntry,
&is_root, &is_krb, &is_smb, &is_ipant,
+ &is_memberof,
SLAPI_USERPWD_ATTR, SLAPI_ACL_WRITE);
if (rc) {
goto free_and_return;
@@ -564,16 +565,21 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
/* check the policy */
ret = ipapwd_CheckPolicy(&pwdata);
if (ret) {
- errMesg = ipapwd_error2string(ret);
if (ret == IPAPWD_POLICY_ERROR) {
errMesg = "Internal error";
rc = ret;
- } else {
+ goto free_and_return;
+ }
+ /* ipapwd_CheckPolicy happily will try to apply a policy
+ * even if it doesn't need to be applied for Directory Manager
+ * or passsync managers, filter that error out */
+ if (pwdata.changetype != IPA_CHANGETYPE_DSMGR) {
+ errMesg = ipapwd_error2string(ret);
ret = ipapwd_to_ldap_pwpolicy_error(ret);
slapi_pwpolicy_make_response_control(pb, -1, -1, ret);
rc = LDAP_CONSTRAINT_VIOLATION;
+ goto free_and_return;
}
- goto free_and_return;
}
/* Now we're ready to set the kerberos key material */
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
index d21d74ec12..31c76b3f1a 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
@@ -90,6 +90,7 @@ struct ipapwd_operation {
struct ipapwd_data pwdata;
int pwd_op;
int is_krb;
+ int is_memberof;
int skip_keys;
int skip_history;
};
@@ -113,6 +114,7 @@ struct ipapwd_krbcfg {
int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
int *is_root, int *is_krb, int *is_smb, int *is_ipant,
+ int *is_memberof,
char *attr, int access);
int ipapwd_gen_checks(Slapi_PBlock *pb, char **errMesg,
struct ipapwd_krbcfg **config, int check_flags);
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index cf88d0f370..001f615ecd 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -80,6 +80,8 @@ struct ipapwd_op_ext {
int object_type; /* handle to the extended object */
int handle; /* extension handle */
};
+
+
/*****************************************************************************
* pre/post operations to intercept writes to userPassword
****************************************************************************/
@@ -217,7 +219,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
Slapi_DN *sdn = NULL;
struct ipapwd_operation *pwdop = NULL;
void *op;
- int is_repl_op, is_root, is_krb, is_smb, is_ipant;
+ int is_repl_op, is_root, is_krb, is_smb, is_ipant, is_memberof;
int ret;
int rc = LDAP_SUCCESS;
@@ -242,8 +244,8 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
/* check this is something interesting for us first */
userpw = slapi_entry_attr_get_charptr(e, SLAPI_USERPWD_ATTR);
if (!userpw) {
- /* nothing interesting here */
- return 0;
+ /* nothing interesting here */
+ return 0;
}
/* Ok this is interesting,
@@ -312,6 +314,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
rc = ipapwd_entry_checks(pb, e,
&is_root, &is_krb, &is_smb, &is_ipant,
+ &is_memberof,
NULL, SLAPI_ACL_ADD);
if (rc != LDAP_SUCCESS) {
goto done;
@@ -346,6 +349,7 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
pwdop->pwd_op = IPAPWD_OP_ADD;
pwdop->pwdata.password = slapi_ch_strdup(userpw);
+ pwdop->is_memberof = is_memberof;
if (is_root) {
pwdop->pwdata.changetype = IPA_CHANGETYPE_DSMGR;
@@ -367,12 +371,15 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
}
}
- pwdop->pwdata.dn = slapi_ch_strdup(dn);
+ pwdop->pwdata.dn = slapi_ch_strdup(slapi_sdn_get_dn(sdn));
pwdop->pwdata.timeNow = time(NULL);
pwdop->pwdata.target = e;
ret = ipapwd_CheckPolicy(&pwdop->pwdata);
- if (ret) {
+ /* For accounts created by cn=Directory Manager or a passsync
+ * managers, ignore result of a policy check */
+ if ((pwdop->pwdata.changetype != IPA_CHANGETYPE_DSMGR) &&
+ (ret != 0) ) {
errMesg = ipapwd_error2string(ret);
rc = LDAP_CONSTRAINT_VIOLATION;
goto done;
@@ -474,7 +481,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
struct slapi_entry *e = NULL;
struct ipapwd_operation *pwdop = NULL;
void *op;
- int is_repl_op, is_pwd_op, is_root, is_krb, is_smb, is_ipant;
+ int is_repl_op, is_pwd_op, is_root, is_krb, is_smb, is_ipant, is_memberof;
int has_krb_keys = 0;
int has_history = 0;
int gen_krb_keys = 0;
@@ -606,6 +613,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
rc = ipapwd_entry_checks(pb, e,
&is_root, &is_krb, &is_smb, &is_ipant,
+ &is_memberof,
is_pwd_op ? SLAPI_USERPWD_ATTR : "ipaNTHash",
SLAPI_ACL_WRITE);
if (rc) {
@@ -808,6 +816,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
}
pwdop->is_krb = is_krb;
+ pwdop->is_memberof = is_memberof;
pwdop->pwd_op = IPAPWD_OP_MOD;
pwdop->pwdata.password = slapi_ch_strdup(unhashedpw);
pwdop->pwdata.changetype = IPA_CHANGETYPE_NORMAL;
@@ -852,11 +861,13 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
pwdop->pwdata.target = e;
/* if krb keys are being set by an external agent we assume password
- * policies have been properly checked already, so we check them only
- * if no krb keys are available */
+ * policies have been properly checked already. We check them only if no
+ * krb keys are available and raise error if the change is not done by a
+ * cn=Directory Manager or one of passsync managers */
if (has_krb_keys == 0) {
ret = ipapwd_CheckPolicy(&pwdop->pwdata);
- if (ret) {
+ if ((pwdop->pwdata.changetype != IPA_CHANGETYPE_DSMGR) &&
+ (ret != 0)) {
errMesg = ipapwd_error2string(ret);
rc = LDAP_CONSTRAINT_VIOLATION;
goto done;
@@ -1066,7 +1077,7 @@ static int ipapwd_post_modadd(Slapi_PBlock *pb)
if (IPAPWD_OP_NULL == pwdop->pwd_op)
return 0;
- if ( ! (pwdop->is_krb)) {
+ if ( !pwdop->is_krb || pwdop->is_memberof) {
LOG("Not a kerberos user, ignore krb attributes\n");
return 0;
}
@@ -1425,6 +1436,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
ret = ipapwd_getEntry(sdn, &entry, (char **) attrs_list);
if (ret) {
LOG("failed to retrieve user entry: %s\n", dn);
+ slapi_sdn_free(&sdn);
return 0;
}
@@ -1449,6 +1461,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
if (current_time > expire_time && expire_time > 0) {
LOG_FATAL("kerberos principal in %s is expired\n", dn);
slapi_entry_free(entry);
+ slapi_sdn_free(&sdn);
slapi_send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL,
"Account (Kerberos principal) is expired",
0, NULL);
@@ -1471,6 +1484,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
ret = ipapwd_authenticate(dn, entry, credentials);
if (ret) {
slapi_entry_free(entry);
+ slapi_sdn_free(&sdn);
return 0;
}
From 546873e7cf3ea902deb3d2a329ecfad5bf683f46 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 20 Mar 2020 13:17:23 -0400
Subject: [PATCH 6/8] Don't save password history on non-Kerberos accounts
While other password policies were properly ignored the password
history was always being saved if the global history size was
non-zero.
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index 60dfe1b162..ba5c54e58e 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -888,8 +888,8 @@ int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
slapi_mods_add_string(smods, LDAP_MOD_REPLACE,
"userPassword", data->password);
- /* set password history */
- if (data->policy.history_length > 0) {
+ /* set password history if a Kerberos object */
+ if (data->policy.history_length > 0 && is_krb) {
pwvals = ipapwd_setPasswordHistory(smods, data);
if (pwvals) {
slapi_mods_add_mod_values(smods, LDAP_MOD_REPLACE,
From a0f78ec85b47d123b4e5cdb6c453ee468e33d658 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 23 Mar 2020 11:40:01 -0400
Subject: [PATCH 7/8] Add ability to change a user password as the Directory
Manager
This is to confirm that the Directory Manager is not affected by
password policy.
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
ipatests/pytest_ipa/integration/tasks.py | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py
index 14c06a4c6f..26e03e90cb 100755
--- a/ipatests/pytest_ipa/integration/tasks.py
+++ b/ipatests/pytest_ipa/integration/tasks.py
@@ -1617,15 +1617,21 @@ def get_host_ip_with_hostmask(host):
return None
-def ldappasswd_user_change(user, oldpw, newpw, master):
+def ldappasswd_user_change(user, oldpw, newpw, master, use_dirman=False):
container_user = dict(DEFAULT_CONFIG)['container_user']
basedn = master.domain.basedn
userdn = "uid={},{},{}".format(user, container_user, basedn)
master_ldap_uri = "ldap://{}".format(master.hostname)
- args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw,
- '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri]
+ if use_dirman:
+ args = [paths.LDAPPASSWD, '-D',
+ str(master.config.dirman_dn), # pylint: disable=no-member
+ '-w', master.config.dirman_password,
+ '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri, userdn]
+ else:
+ args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw,
+ '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri]
master.run_command(args)
From bd0f92487b7da3b08d8165d2642013dcab1764f7 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Sat, 21 Mar 2020 12:46:32 -0400
Subject: [PATCH 8/8] Test that pwpolicy only applied on Kerberos entries
Also test that a normal user has password history enforcement
Reviewed-By: Alexander Bokovoy <aboko...@redhat.com>
Reviewed-By: Christian Heimes <chei...@redhat.com>
---
ipatests/test_integration/test_commands.py | 125 +++++++++++++--------
1 file changed, 77 insertions(+), 48 deletions(-)
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index 43f99e1eb1..be22482568 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -50,6 +50,18 @@ class TestIPACommand(IntegrationTest):
"""
topology = 'line'
+ @pytest.fixture
+ def pwpolicy_global(self):
+ """Fixture to change global password history policy and reset it"""
+ tasks.kinit_admin(self.master)
+ self.master.run_command(
+ ["ipa", "pwpolicy-mod", "--history=5", "--minlife=0"],
+ )
+ yield
+ self.master.run_command(
+ ["ipa", "pwpolicy-mod", "--history=0", "--minlife=1"],
+ )
+
def get_cert_base64(self, host, path):
"""Retrieve cert and return content as single line, base64 encoded
"""
@@ -228,32 +240,17 @@ def test_ldapmodify_password_issue7601(self):
assert newkrblastpwdchange != newkrblastpwdchange2
assert newkrbexp != newkrbexp2
- def test_change_sysaccount_password_issue7181(self):
+ def test_change_sysaccount_pwd_history_issue7181(self, pwpolicy_global):
"""
- Test how a password change performed by a cn=Directory Manager
- works against a non-Kerberos account with a policy preventing
- re-use of previously used passwords
+ Test that a sysacount user maintains no password history
+ because they do not have a Kerberos identity.
"""
- sysuser = 'forcedpolicy'
- policy_group = 'forcedpolicy'
+ sysuser = 'sysuser'
original_passwd = 'Secret123'
new_passwd = 'userPasswd123'
master = self.master
- # Add a group with a custom password policy
- tasks.kinit_admin(self.master)
- result = master.run_command(
- ["ipa", "group-add", policy_group]
- )
- assert 'Added group "{}"'.format(policy_group) in result.stdout_text
-
- result = master.run_command(
- ["ipa", "pwpolicy-add", policy_group,
- "--history=5", "--priority=1"],
- )
- assert 'History size: 5' in result.stdout_text
-
# Add a system account and add it to a group managed by the policy
base_dn = str(master.domain.basedn) # pylint: disable=no-member
entry_ldif = textwrap.dedent("""
@@ -261,55 +258,87 @@ def test_change_sysaccount_password_issue7181(self):
changetype: add
objectclass: account
objectclass: simplesecurityobject
- objectclass: nsMemberOf
- objectclass: krbPrincipalAux
uid: {account_name}
userPassword: {original_passwd}
passwordExpirationTime: 20380119031407Z
nsIdleTimeout: 0
- memberOf: cn={group_name},cn=groups,cn=accounts,{base_dn}
""").format(
account_name=sysuser,
- group_name=policy_group,
base_dn=base_dn,
original_passwd=original_passwd)
tasks.ldapmodify_dm(master, entry_ldif)
- # For an LDAP object not managed by IPA we have to use
- # --addattr to add it as a member of a group
- value = "member=uid={account_name},cn=sysaccounts,cn=etc,{base_dn}"
- result = master.run_command(
- ["ipa", "group-mod", policy_group,
- "--addattr={value}".format(
- value=value.format(
- account_name=sysuser,
- base_dn=base_dn
- )
- )],
- )
- assert 'Modified group "{}"'.format(policy_group) in result.stdout_text
-
- # Now try to change password thrice:
- # as a user, as a cn=Directory Manager, and as a user again
- # If ticket 7181 is not fixed, the second change will fail
- # Third one must fail as we are reusing the password as non-DM
+ # Now change the password. It should succeed since password
+ # policy doesn't apply to non-Kerberos users.
tasks.ldappasswd_sysaccount_change(sysuser, original_passwd,
- new_passwd, master,
- use_dirman=False)
+ new_passwd, master)
tasks.ldappasswd_sysaccount_change(sysuser, new_passwd,
- new_passwd, master,
- use_dirman=True)
+ original_passwd, master)
+ tasks.ldappasswd_sysaccount_change(sysuser, original_passwd,
+ new_passwd, master)
+
+ def test_change_user_pwd_history_issue7181(self, pwpolicy_global):
+ """
+ Test that password history for a normal IPA user is honored.
+ """
+ user = 'user1'
+ original_passwd = 'Secret123'
+ new_passwd = 'userPasswd123'
+
+ master = self.master
+
+ tasks.user_add(master, user, password=original_passwd)
+
+ tasks.ldappasswd_user_change(user, original_passwd,
+ new_passwd, master)
+ tasks.ldappasswd_user_change(user, new_passwd,
+ original_passwd, master)
+ try:
+ tasks.ldappasswd_user_change(user, original_passwd,
+ new_passwd, master)
+ except CalledProcessError as e:
+ if e.returncode != 1:
+ raise
+ else:
+ pytest.fail("Password change violating policy did not fail")
+
+ def test_dm_change_user_pwd_history_issue7181(self, pwpolicy_global):
+ """
+ Test that password policy is not applied with Directory Manager.
+
+ The minimum lifetime of the password is set to 1 hour. Confirm
+ that the user cannot re-change their password immediately but
+ the DM can.
+ """
+ user = 'user1'
+ original_passwd = 'Secret123'
+ new_passwd = 'newPasswd123'
+
+ master = self.master
+
+ # reset minimum life to 1 hour.
+ self.master.run_command(
+ ["ipa", "pwpolicy-mod", "--minlife=1"],
+ )
+
try:
- tasks.ldappasswd_sysaccount_change(sysuser, new_passwd,
- original_passwd, master,
- use_dirman=False)
+ tasks.ldappasswd_user_change(user, original_passwd,
+ new_passwd, master)
except CalledProcessError as e:
if e.returncode != 1:
raise
else:
pytest.fail("Password change violating policy did not fail")
+ # DM should be able to change any password regardless of policy
+ try:
+ tasks.ldappasswd_user_change(user, new_passwd,
+ original_passwd, master,
+ use_dirman=True)
+ except CalledProcessError:
+ pytest.fail("Password change failed when it should not")
+
def test_change_selinuxusermaporder(self):
"""
An update file meant to ensure a more sane default was
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
