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