From f5beddb45d1058227a9a1530abe156d27ea5e534 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 22 Sep 2020 16:43:08 -0400 Subject: [PATCH 01/10] Add LDAP schema for new libpwquality attributes Add new attributes for the maxrepeat, maxsequence, dictcheck and usercheck features of libpwquality. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- install/share/60basev2.ldif | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/install/share/60basev2.ldif b/install/share/60basev2.ldif index 94b4376ac8..f253f30c91 100644 --- a/install/share/60basev2.ldif +++ b/install/share/60basev2.ldif @@ -52,3 +52,9 @@ attributeTypes: (2.16.840.1.113730.3.8.3.17 NAME 'hostCApolicy' DESC 'Policy on objectClasses: (2.16.840.1.113730.3.8.4.9 NAME 'ipaCAaccess' STRUCTURAL MAY (member $ hostCApolicy) X-ORIGIN 'IPA v2' ) objectClasses: (2.16.840.1.113730.3.8.4.10 NAME 'ipaHBACService' STRUCTURAL MUST ( cn ) MAY ( description $ memberOf ) X-ORIGIN 'IPA v2' ) objectClasses: (2.16.840.1.113730.3.8.4.11 NAME 'ipaHBACServiceGroup' DESC 'IPA HBAC service group object class' SUP groupOfNames STRUCTURAL X-ORIGIN 'IPA v2' ) +# IPA password policy configuration via libpwquality +attributeTypes: (2.16.840.1.113730.3.8.23.2 NAME 'ipaPwdMaxRepeat' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA v4') +attributeTypes: (2.16.840.1.113730.3.8.23.3 NAME 'ipaPwdMaxSequence' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA v4') +attributeTypes: (2.16.840.1.113730.3.8.23.4 NAME 'ipaPwdDictCheck' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4') +attributeTypes: (2.16.840.1.113730.3.8.23.5 NAME 'ipaPwdUserCheck' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4') +objectClasses: (2.16.840.1.113730.3.8.24.1 NAME 'ipaPwdPolicy' DESC 'IPA Password policy object class' SUP top MAY (ipaPwdMaxRepeat $ ipaPwdMaxSequence $ ipaPwdDictCheck $ ipaPwdUserCheck) X-ORIGIN 'IPA v4') From 10302f6dd0abdb807bdf01278235ec0b241062f8 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 22 Sep 2020 16:43:50 -0400 Subject: [PATCH 02/10] Extend IPA pwquality plugin to include libpwquality support Add options to support maxrepeat, maxsequence, dictcheck and usercheck pwquality options. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- ACI.txt | 8 +-- API.txt | 18 +++++-- VERSION.m4 | 4 +- ipaserver/plugins/pwpolicy.py | 99 +++++++++++++++++++++++++++++++++-- 4 files changed, 115 insertions(+), 14 deletions(-) diff --git a/ACI.txt b/ACI.txt index cd660a4fc1..bb72563744 100644 --- a/ACI.txt +++ b/ACI.txt @@ -227,13 +227,13 @@ aci: (targetattr = "businesscategory || cn || createtimestamp || description || dn: cn=privileges,cn=pbac,dc=ipa,dc=example aci: (targetfilter = "(objectclass=groupofnames)")(version 3.0;acl "permission:System: Remove Privileges";allow (delete) groupdn = "ldap:///cn=System: Remove Privileges,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example -aci: (targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Add Group Password Policy";allow (add) groupdn = "ldap:///cn=System: Add Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) +aci: (targetfilter = "(|(objectclass=ipapwdpolicy)(objectclass=krbpwdpolicy))")(version 3.0;acl "permission:System: Add Group Password Policy";allow (add) groupdn = "ldap:///cn=System: Add Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example -aci: (targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Delete Group Password Policy";allow (delete) groupdn = "ldap:///cn=System: Delete Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) +aci: (targetfilter = "(|(objectclass=ipapwdpolicy)(objectclass=krbpwdpolicy))")(version 3.0;acl "permission:System: Delete Group Password Policy";allow (delete) groupdn = "ldap:///cn=System: Delete Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example -aci: (targetattr = "krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength")(targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Modify Group Password Policy";allow (write) groupdn = "ldap:///cn=System: Modify Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) +aci: (targetattr = "ipapwddictcheck || ipapwdmaxrepeat || ipapwdmaxsequence || ipapwdusercheck || krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength")(targetfilter = "(|(objectclass=ipapwdpolicy)(objectclass=krbpwdpolicy))")(version 3.0;acl "permission:System: Modify Group Password Policy";allow (write) groupdn = "ldap:///cn=System: Modify Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example -aci: (targetattr = "cn || cospriority || createtimestamp || entryusn || krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength || modifytimestamp || objectclass")(targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Read Group Password Policy";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) +aci: (targetattr = "cn || cospriority || createtimestamp || entryusn || ipapwddictcheck || ipapwdmaxrepeat || ipapwdmaxsequence || ipapwdusercheck || krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength || modifytimestamp || objectclass")(targetfilter = "(|(objectclass=ipapwdpolicy)(objectclass=krbpwdpolicy))")(version 3.0;acl "permission:System: Read Group Password Policy";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=radiusproxy,dc=ipa,dc=example aci: (targetattr = "cn || createtimestamp || description || entryusn || ipatokenradiusretries || ipatokenradiusserver || ipatokenradiustimeout || ipatokenusermapattribute || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipatokenradiusconfiguration)")(version 3.0;acl "permission:System: Read Radius Servers";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Radius Servers,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=Realm Domains,cn=ipa,cn=etc,dc=ipa,dc=example diff --git a/API.txt b/API.txt index 300b3d9b16..a71a9306ff 100644 --- a/API.txt +++ b/API.txt @@ -3960,11 +3960,15 @@ output: Entry('result') output: Output('summary', type=[, ]) output: PrimaryKey('value') command: pwpolicy_add/1 -args: 1,14,3 +args: 1,18,3 arg: Str('cn', cli_name='group') option: Str('addattr*', cli_name='addattr') option: Flag('all', autofill=True, cli_name='all', default=False) option: Int('cospriority', cli_name='priority') +option: Bool('ipapwddictcheck?', cli_name='dictcheck', default=False) +option: Int('ipapwdmaxrepeat?', cli_name='maxrepeat', default=0) +option: Int('ipapwdmaxsequence?', cli_name='maxsequence', default=0) +option: Bool('ipapwdusercheck?', cli_name='usercheck', default=False) option: Int('krbmaxpwdlife?', cli_name='maxlife') option: Int('krbminpwdlife?', cli_name='minlife') option: Int('krbpwdfailurecountinterval?', cli_name='failinterval') @@ -3988,11 +3992,15 @@ output: Output('result', type=[]) output: Output('summary', type=[, ]) output: ListOfPrimaryKeys('value') command: pwpolicy_find/1 -args: 1,16,4 +args: 1,20,4 arg: Str('criteria?') option: Flag('all', autofill=True, cli_name='all', default=False) option: Str('cn?', autofill=False, cli_name='group') option: Int('cospriority?', autofill=False, cli_name='priority') +option: Bool('ipapwddictcheck?', autofill=False, cli_name='dictcheck', default=False) +option: Int('ipapwdmaxrepeat?', autofill=False, cli_name='maxrepeat', default=0) +option: Int('ipapwdmaxsequence?', autofill=False, cli_name='maxsequence', default=0) +option: Bool('ipapwdusercheck?', autofill=False, cli_name='usercheck', default=False) option: Int('krbmaxpwdlife?', autofill=False, cli_name='maxlife') option: Int('krbminpwdlife?', autofill=False, cli_name='minlife') option: Int('krbpwdfailurecountinterval?', autofill=False, cli_name='failinterval') @@ -4011,12 +4019,16 @@ output: ListOfEntries('result') output: Output('summary', type=[, ]) output: Output('truncated', type=[]) command: pwpolicy_mod/1 -args: 1,16,3 +args: 1,20,3 arg: Str('cn?', cli_name='group') option: Str('addattr*', cli_name='addattr') option: Flag('all', autofill=True, cli_name='all', default=False) option: Int('cospriority?', autofill=False, cli_name='priority') option: Str('delattr*', cli_name='delattr') +option: Bool('ipapwddictcheck?', autofill=False, cli_name='dictcheck', default=False) +option: Int('ipapwdmaxrepeat?', autofill=False, cli_name='maxrepeat', default=0) +option: Int('ipapwdmaxsequence?', autofill=False, cli_name='maxsequence', default=0) +option: Bool('ipapwdusercheck?', autofill=False, cli_name='usercheck', default=False) option: Int('krbmaxpwdlife?', autofill=False, cli_name='maxlife') option: Int('krbminpwdlife?', autofill=False, cli_name='minlife') option: Int('krbpwdfailurecountinterval?', autofill=False, cli_name='failinterval') diff --git a/VERSION.m4 b/VERSION.m4 index 788db19e81..683af342bb 100644 --- a/VERSION.m4 +++ b/VERSION.m4 @@ -86,8 +86,8 @@ define(IPA_DATA_VERSION, 20100614120000) # # ######################################################## define(IPA_API_VERSION_MAJOR, 2) -define(IPA_API_VERSION_MINOR, 239) -# Last change: allow ID overrides for users to be members of groups and roles +define(IPA_API_VERSION_MINOR, 240) +# Last change: add pwquality options to pwpolicy ######################################################## diff --git a/ipaserver/plugins/pwpolicy.py b/ipaserver/plugins/pwpolicy.py index fa148ea4de..9e20bb7a4d 100644 --- a/ipaserver/plugins/pwpolicy.py +++ b/ipaserver/plugins/pwpolicy.py @@ -21,7 +21,7 @@ import logging from ipalib import api -from ipalib import Int, Str, DNParam +from ipalib import Int, Str, DNParam, Bool from ipalib import errors from .baseldap import ( LDAPObject, @@ -236,13 +236,15 @@ class pwpolicy(LDAPObject): container_dn = DN(('cn', api.env.realm), ('cn', 'kerberos')) object_name = _('password policy') object_name_plural = _('password policies') - object_class = ['top', 'nscontainer', 'krbpwdpolicy'] - permission_filter_objectclasses = ['krbpwdpolicy'] + object_class = ['top', 'nscontainer', 'krbpwdpolicy', 'ipapwdpolicy'] + permission_filter_objectclasses = ['krbpwdpolicy', 'ipapwdpolicy'] default_attributes = [ 'cn', 'cospriority', 'krbmaxpwdlife', 'krbminpwdlife', 'krbpwdhistorylength', 'krbpwdmindiffchars', 'krbpwdminlength', 'krbpwdmaxfailure', 'krbpwdfailurecountinterval', - 'krbpwdlockoutduration', + 'krbpwdlockoutduration', 'ipapwdmaxrepeat', + 'ipapwdmaxsequence', 'ipapwddictcheck', + 'ipapwdusercheck', ] managed_permissions = { 'System: Read Group Password Policy': { @@ -254,6 +256,8 @@ class pwpolicy(LDAPObject): 'krbpwdfailurecountinterval', 'krbpwdhistorylength', 'krbpwdlockoutduration', 'krbpwdmaxfailure', 'krbpwdmindiffchars', 'krbpwdminlength', 'objectclass', + 'ipapwdmaxrepeat', 'ipapwdmaxsequence', 'ipapwddictcheck', + 'ipapwdusercheck', }, 'default_privileges': { 'Password Policy Readers', @@ -279,7 +283,9 @@ class pwpolicy(LDAPObject): 'ipapermdefaultattr': { 'krbmaxpwdlife', 'krbminpwdlife', 'krbpwdfailurecountinterval', 'krbpwdhistorylength', 'krbpwdlockoutduration', - 'krbpwdmaxfailure', 'krbpwdmindiffchars', 'krbpwdminlength' + 'krbpwdmaxfailure', 'krbpwdmindiffchars', 'krbpwdminlength', + 'ipapwdmaxrepeat', 'ipapwdmaxsequence', 'ipapwddictcheck', + 'ipapwdusercheck', }, 'replaces': [ '(targetattr = "krbmaxpwdlife || krbminpwdlife || krbpwdhistorylength || krbpwdmindiffchars || krbpwdminlength || krbpwdmaxfailure || krbpwdfailurecountinterval || krbpwdlockoutduration")(target = "ldap:///cn=*,cn=$REALM,cn=kerberos,$SUFFIX")(version 3.0;acl "permission:Modify Group Password Policy";allow (write) groupdn = "ldap:///cn=Modify Group Password Policy,cn=permissions,cn=pbac,$SUFFIX";)', @@ -358,6 +364,38 @@ class pwpolicy(LDAPObject): doc=_('Period for which lockout is enforced (seconds)'), minvalue=0, ), + Int( + 'ipapwdmaxrepeat?', + cli_name='maxrepeat', + label=_('Max repeat'), + doc=_('Maximum number of same consecutive characters'), + minvalue=0, + maxvalue=256, + default=0, + ), + Int( + 'ipapwdmaxsequence?', + cli_name='maxsequence', + label=_('Max sequence'), + doc=_('The max. length of monotonic character sequences (abcd)'), + minvalue=0, + maxvalue=256, + default=0, + ), + Bool( + 'ipapwddictcheck?', + cli_name='dictcheck', + label=_('Dictionary check'), + doc=_('Check if the password is a dictionary word'), + default=False, + ), + Bool( + 'ipapwdusercheck?', + cli_name='usercheck', + label=_('User check'), + doc=_('Check if the password contains the username'), + default=False, + ), ) def get_dn(self, *keys, **options): @@ -387,6 +425,51 @@ def convert_time_on_input(self, entry_attrs): if 'krbminpwdlife' in entry_attrs and entry_attrs['krbminpwdlife']: entry_attrs['krbminpwdlife'] = entry_attrs['krbminpwdlife'] * 3600 + def validate_minlength(self, ldap, entry_attrs, add=False, *keys): + """ + If any of the libpwquality options are used then the minimum + length must be >= 6 which is the built-in default of libpwquality. + Allowing a lower value to be set will result in a failed policy + check and a generic error message. + """ + def get_val(entry, attr): + """Get a single value from a list or a string""" + val = entry.get(attr, 0) + if isinstance(val, list): + val = val[0] + return val + + def has_pwquality_set(entry): + for attr in ['ipapwdmaxrepeat', 'ipapwdmaxsequence', + 'ipapwddictcheck', 'ipapwdusercheck']: + val = get_val(entry, attr) + if val not in ('FALSE', '0', 0, None): + return True + return False + + has_pwquality_value = False + if not add: + if len(keys) > 0: + existing_entry = self.api.Command.pwpolicy_show( + keys[-1], all=True,)['result'] + else: + existing_entry = self.api.Command.pwpolicy_show( + all=True,)['result'] + existing_entry.update(entry_attrs) + min_length = int(get_val(existing_entry, 'krbpwdminlength')) + + has_pwquality_value = has_pwquality_set(existing_entry) + else: + min_length = int(get_val(entry_attrs, 'krbpwdminlength')) + has_pwquality_value = has_pwquality_set(entry_attrs) + + if min_length and min_length < 6 and has_pwquality_value: + raise errors.ValidationError( + name='minlength', + error=_('Minimum length must be >= 6 if maxrepeat, ' + 'maxsequence, dictcheck or usercheck are defined') + ) + def validate_lifetime(self, entry_attrs, add=False, *keys): """ Ensure that the maximum lifetime is greater than the minimum. @@ -435,6 +518,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): assert isinstance(dn, DN) self.obj.convert_time_on_input(entry_attrs) self.obj.validate_lifetime(entry_attrs, True) + self.obj.validate_minlength(ldap, entry_attrs, True) self.api.Command.cosentry_add( keys[-1], krbpwdpolicyreference=dn, cospriority=options.get('cospriority') @@ -486,7 +570,12 @@ def execute(self, cn=None, **options): def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): assert isinstance(dn, DN) + old_entry_attrs = ldap.get_entry(dn, ['objectclass']) + if 'ipapwdpolicy' not in old_entry_attrs['objectclass']: + old_entry_attrs['objectclass'].append('ipapwdpolicy') + entry_attrs['objectclass'] = old_entry_attrs['objectclass'] self.obj.convert_time_on_input(entry_attrs) + self.obj.validate_minlength(ldap, entry_attrs, False, *keys) self.obj.validate_lifetime(entry_attrs, False, *keys) setattr(context, 'cosupdate', False) if options.get('cospriority') is not None: From ed0e42ae448033561f0bc9ad2d5ce04c990bd757 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 24 Sep 2020 15:33:17 -0400 Subject: [PATCH 03/10] Require libpwolicy and configure it in the build system https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- configure.ac | 6 ++++++ freeipa.spec.in | 1 + util/Makefile.am | 7 ++++--- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 1a6e40c7c6..cda7d75af6 100644 --- a/configure.ac +++ b/configure.ac @@ -113,6 +113,11 @@ dnl - Check for OpenSSL Crypto library dnl --------------------------------------------------------------------------- PKG_CHECK_MODULES([CRYPTO], [libcrypto]) +dnl --------------------------------------------------------------------------- +dnl - Check for pwquality library +dnl --------------------------------------------------------------------------- +PKG_CHECK_MODULES([PWQUALITY], [pwquality]) + dnl --------------------------------------------------------------------------- dnl - Check for Python 3 dnl - Check for platform Python interpreter @@ -665,6 +670,7 @@ echo " jslint: ${JSLINT} LDAP libs: ${LDAP_LIBS} OpenSSL crypto libs: ${CRYPTO_LIBS} + pwquality libs: ${PWQUALITY_LIBS} KRB5 libs: ${KRB5_LIBS} systemdsystemunitdir: ${systemdsystemunitdir}" diff --git a/freeipa.spec.in b/freeipa.spec.in index 00669b1fc6..23b75609a4 100755 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -207,6 +207,7 @@ BuildRequires: samba-devel >= %{samba_version} BuildRequires: libtalloc-devel BuildRequires: libtevent-devel BuildRequires: libuuid-devel +BuildRequires: libpwquality-devel BuildRequires: libsss_idmap-devel BuildRequires: libsss_certmap-devel BuildRequires: libsss_nss_idmap-devel >= %{sssd_version} diff --git a/util/Makefile.am b/util/Makefile.am index a2400063d0..319809ba3c 100644 --- a/util/Makefile.am +++ b/util/Makefile.am @@ -1,6 +1,6 @@ AUTOMAKE_OPTIONS = 1.7 subdir-objects -AM_CPPFLAGS = $(CRYPTO_CFLAGS) $(KRB5_CFLAGS) $(LDAP_CFLAGS) +AM_CPPFLAGS = $(CRYPTO_CFLAGS) $(KRB5_CFLAGS) $(LDAP_CFLAGS) $(PWQUALITY_CFLAGS) noinst_LTLIBRARIES = libutil.la @@ -13,8 +13,9 @@ libutil_la_SOURCES = ipa_krb5.c \ ipa_pwd.h \ ipa_pwd_ntlm.c -libutil_la_LIBADD = $(CRYPTO_LIBS) $(KRB5_LIBS) $(LDAP_LIBS) +libutil_la_LIBADD = $(CRYPTO_LIBS) $(KRB5_LIBS) $(LDAP_LIBS) $(PWQUALITY_LIBS) -check_PROGRAMS = t_pwd +check_PROGRAMS = t_pwd t_policy TESTS = $(check_PROGRAMS) t_pwd_LDADD = libutil.la +t_policy_LDADD = libutil.la From 6df3e92fe447cf5856746a59742f8f2aee6a1b39 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 24 Sep 2020 15:43:59 -0400 Subject: [PATCH 04/10] Extend password policy to evaluate passwords using libpwpolicy Enable checking: maxrepeat - reject passwrods which contain more than N consecutive characters. maxsequence - rejected passwords which contain character sequences (abcde). dictcheck - check passwords using cracklib usercheck - check whether the password contains the user name. The class checking provided by libpwpolicy is not used because this overlaps with the existing IPA checking. This includes the options dcredit, ucredit, lcredit, ocredit, minclass and maxclassrepeat. The pwquality min length is fixed at 6 so if there is a conflict between the system policy and pwquality log that length is enforced at 6. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- .../ipa-slapi-plugins/ipa-pwd-extop/common.c | 12 ++- util/ipa_pwd.c | 87 ++++++++++++++++++- util/ipa_pwd.h | 13 ++- 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c index f520be2b5d..c5ea60dba3 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c @@ -322,7 +322,9 @@ int ipapwd_getPolicy(const char *dn, Slapi_PBlock *pb = NULL; char *attrs[] = { "krbMaxPwdLife", "krbMinPwdLife", "krbPwdMinDiffChars", "krbPwdMinLength", - "krbPwdHistoryLength", NULL}; + "krbPwdHistoryLength", "ipaPwdMaxRepeat", + "ipaPwdMaxSequence", "ipaPwdDictCheck", + "ipaPwdUserCheck", NULL}; Slapi_Entry **es = NULL; Slapi_Entry *pe = NULL; int ret, res, scope, i; @@ -402,6 +404,10 @@ int ipapwd_getPolicy(const char *dn, policy->min_complexity = slapi_entry_attr_get_int(pe, "krbPwdMinDiffChars"); + policy->max_repeat = slapi_entry_attr_get_int(pe, "ipaPwdMaxRepeat"); + policy->max_sequence = slapi_entry_attr_get_int(pe, "ipaPwdMaxSequence"); + policy->dictcheck = slapi_entry_attr_get_bool(pe, "ipaPwdDictCheck"); + policy->usercheck = slapi_entry_attr_get_bool(pe, "ipaPwdUserCheck"); ret = 0; @@ -576,6 +582,7 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data) time_t pwd_expiration; time_t last_pwd_change; char **pwd_history; + char *uid; char *tmpstr; int ret; @@ -641,9 +648,11 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data) pwd_history = slapi_entry_attr_get_charray(data->target, "passwordHistory"); + uid = slapi_entry_attr_get_charptr(data->target, "uid"); /* check policy */ ret = ipapwd_check_policy(&pol, data->password, + uid, data->timeNow, acct_expiration, pwd_expiration, @@ -651,6 +660,7 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data) pwd_history); slapi_ch_array_free(pwd_history); + slapi_ch_free_string(&uid); if (data->expireTime == 0) { if (pol.max_pwd_life > 0) { diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c index 0d3c351c24..dbff5ef1cb 100644 --- a/util/ipa_pwd.c +++ b/util/ipa_pwd.c @@ -28,8 +28,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -406,6 +408,7 @@ static char *ipapwd_hash_to_history(time_t hash_time, */ int ipapwd_check_policy(struct ipapwd_policy *policy, char *password, + char *user, time_t cur_time, time_t acct_expiration, time_t pwd_expiration, @@ -414,6 +417,11 @@ int ipapwd_check_policy(struct ipapwd_policy *policy, { int pwdlen, blen; int ret; + pwquality_settings_t *pwq; + int check_pwquality = 0; + int entropy; + char buf[PWQ_MAX_ERROR_MESSAGE_LEN]; + void *auxerror; if (!policy || !password) { return IPAPWD_POLICY_ERROR; @@ -462,7 +470,7 @@ int ipapwd_check_policy(struct ipapwd_policy *policy, char *p, *n; int size, len; - /* we want the actual lenght in bytes here */ + /* we want the actual length in bytes here */ len = blen; p = password; @@ -526,6 +534,74 @@ int ipapwd_check_policy(struct ipapwd_policy *policy, } } + /* Only call into libpwquality if at least one setting is made + * because there are a number of checks that don't have knobs + * so preserve the previous behavior. + */ + check_pwquality = policy->max_repeat + policy->max_sequence + policy->dictcheck + policy->usercheck; + + if (check_pwquality > 0) { + /* Call libpwquality */ + openlog(NULL, LOG_CONS | LOG_NDELAY, LOG_DAEMON); + pwq = pwquality_default_settings(); + if (pwq == NULL) { + syslog(LOG_ERR, "Not able to set pwquality defaults\n"); + return IPAPWD_POLICY_ERROR; + } + if (policy->min_pwd_length < 6) + syslog(LOG_WARNING, "password policy min length is < 6. Will be enforced as 6\n"); + pwquality_set_int_value(pwq, PWQ_SETTING_MIN_LENGTH, policy->min_pwd_length); + pwquality_set_int_value(pwq, PWQ_SETTING_MAX_REPEAT, policy->max_repeat); + pwquality_set_int_value(pwq, PWQ_SETTING_MAX_SEQUENCE, policy->max_sequence); + pwquality_set_int_value(pwq, PWQ_SETTING_DICT_CHECK, policy->dictcheck); + pwquality_set_int_value(pwq, PWQ_SETTING_USER_CHECK, policy->usercheck); + + entropy = pwquality_check(pwq, password, NULL, user, &auxerror); + pwquality_free_settings(pwq); + +#ifdef TEST + if (user != NULL) { + fprintf(stderr, "Checking password for %s\n", user); + } else { + fprintf(stderr, "No user provided\n"); + } + + fprintf(stderr, "min length %d\n", policy->min_pwd_length); + fprintf(stderr, "max repeat %d\n", policy->max_repeat); + fprintf(stderr, "max sequence %d\n", policy->max_sequence); + fprintf(stderr, "dict check %d\n", policy->dictcheck); + fprintf(stderr, "user check %d\n", policy->usercheck); +#endif + + if (entropy < 0) { +#ifdef TEST + fprintf(stderr, "Bad password '%s': %s\n", password, pwquality_strerror(buf, sizeof(buf), entropy, auxerror)); +#endif + syslog(LOG_ERR, "Password is rejected with error %d: %s\n", entropy, pwquality_strerror(buf, sizeof(buf), entropy, auxerror)); + switch (entropy) { + case PWQ_ERROR_MIN_LENGTH: + return IPAPWD_POLICY_PWD_TOO_SHORT; + case PWQ_ERROR_PALINDROME: + return IPAPWD_POLICY_PWD_PALINDROME; + case PWQ_ERROR_MAX_CONSECUTIVE: + return IPAPWD_POLICY_PWD_CONSECUTIVE; + case PWQ_ERROR_MAX_SEQUENCE: + return IPAPWD_POLICY_PWD_SEQUENCE; + case PWQ_ERROR_CRACKLIB_CHECK: + return IPAPWD_POLICY_PWD_DICT_WORD; + case PWQ_ERROR_USER_CHECK: + return IPAPWD_POLICY_PWD_USER; + default: + return IPAPWD_POLICY_PWD_COMPLEXITY; + } + +#ifdef TEST + } else { + fprintf(stderr, "Password '%s' is ok, entropy is %d\n", password, entropy); +#endif + } + } + if (pwd_history) { char *hash; int i; @@ -549,13 +625,18 @@ char * IPAPWD_ERROR_STRINGS[] = { "Too soon to change password", "Password is too short", "Password reuse not permitted", - "Password is too simple" + "Password is too simple", + "Password has too many consecutive characters", + "Password contains a monotonic sequence", + "Password is based on a dictionary word", + "Password is a palindrone", + "Password contains username" }; char * IPAPWD_ERROR_STRING_GENERAL = "Password does not meet the policy requirements"; char * ipapwd_error2string(enum ipapwd_error err) { - if (err < 0 || err > IPAPWD_POLICY_PWD_COMPLEXITY) { + if (err < 0 || err > IPAPWD_POLICY_PWD_USER) { /* IPAPWD_POLICY_ERROR or out of boundary, return general error */ return IPAPWD_ERROR_STRING_GENERAL; } diff --git a/util/ipa_pwd.h b/util/ipa_pwd.h index afe4fdf3eb..b3bb2be6e2 100644 --- a/util/ipa_pwd.h +++ b/util/ipa_pwd.h @@ -44,7 +44,12 @@ enum ipapwd_error { IPAPWD_POLICY_PWD_TOO_YOUNG = 2, IPAPWD_POLICY_PWD_TOO_SHORT = 3, IPAPWD_POLICY_PWD_IN_HISTORY = 4, - IPAPWD_POLICY_PWD_COMPLEXITY = 5 + IPAPWD_POLICY_PWD_COMPLEXITY = 5, + IPAPWD_POLICY_PWD_CONSECUTIVE = 6, + IPAPWD_POLICY_PWD_SEQUENCE = 7, + IPAPWD_POLICY_PWD_DICT_WORD = 8, + IPAPWD_POLICY_PWD_PALINDROME = 9, + IPAPWD_POLICY_PWD_USER = 10 }; struct ipapwd_policy { @@ -56,6 +61,11 @@ struct ipapwd_policy { int max_fail; int failcnt_interval; int lockout_duration; + int max_repeat; + int max_sequence; + int max_classrepeat; + int dictcheck; + int usercheck; }; time_t ipapwd_gentime_to_time_t(char *timestr); @@ -68,6 +78,7 @@ int ipapwd_hash_password(char *password, int ipapwd_check_policy(struct ipapwd_policy *policy, char *password, + char *user, time_t cur_time, time_t acct_expiration, time_t pwd_expiration, From a6375aec052f9f5b3ecb38292d1dec1b81799c41 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 24 Sep 2020 15:42:45 -0400 Subject: [PATCH 05/10] Add a unit test for libpwquality-based password policy - with all policies disabled passwords are not evaluated - the pwpolicy minimum overrides the existing IPA minimum - max character repeats - max character sequences (12345) - palindrome - dictionary check - user name in the password check https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- util/t_policy.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 util/t_policy.c diff --git a/util/t_policy.c b/util/t_policy.c new file mode 100644 index 0000000000..88ac1b53b6 --- /dev/null +++ b/util/t_policy.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2020 FreeIPA Contributors see COPYING for license + */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + +#include +#include +#include +#include + +#include "ipa_pwd.h" + +static void +set_policy(struct ipapwd_policy *policy, + int min_pwd_length, int min_diff_chars, int max_repeat, + int max_sequence, int max_class_repeat, int dict_check, + int user_check) + +{ + /* defaults for things we aren't testing */ + policy->min_pwd_life = 0; + policy->max_pwd_life = 0; + policy->history_length = 0; + + /* Note: min password length in libpwqualty is hardcoded at 6 */ + policy->min_pwd_length = min_pwd_length; + policy->min_complexity = min_diff_chars; + policy->max_repeat = max_repeat; + policy->max_sequence = max_sequence; + policy->max_classrepeat = max_class_repeat; + policy->dictcheck = dict_check; + policy->usercheck = user_check; +} + +int main(int argc, const char *argv[]) { + (void) argc; + (void) argv; + + struct ipapwd_policy policy = {0}; + + /* No policy applied */ + set_policy(&policy, 0, 0, 0, 0, 0, 0, 0); + assert(ipapwd_check_policy(&policy, "Secret123", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "password", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "abcddcba", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + + /* Check that with no policy the IPA minimum is in force */ + assert(ipapwd_check_policy(&policy, "abc", NULL, 3, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + + /* Max repeats of 1 */ + set_policy(&policy, 0, 0, 1, 0, 0, 0, 0); + assert(ipapwd_check_policy(&policy, "password", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_CONSECUTIVE); + assert(ipapwd_check_policy(&policy, "Assembly", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_CONSECUTIVE); + + /* Minimum length lower than libpwquality allows (6) */ + assert(ipapwd_check_policy(&policy, "abc", NULL, 3, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_TOO_SHORT); + + /* Max repeats of 2 */ + set_policy(&policy, 0, 0, 2, 0, 0, 0, 0); + assert(ipapwd_check_policy(&policy, "password", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "Assembly", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "permisssive", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_CONSECUTIVE); + + /* Max sequence of 1 */ + set_policy(&policy, 0, 0, 0, 1, 0, 0, 0); + assert(ipapwd_check_policy(&policy, "abacab", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_SEQUENCE); + assert(ipapwd_check_policy(&policy, "AbacAb", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_SEQUENCE); + + /* Max sequence of 2 */ + set_policy(&policy, 0, 0, 0, 2, 0, 0, 0); + assert(ipapwd_check_policy(&policy, "AbacAb", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "abacabc", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_SEQUENCE); + + /* Palindrone */ + set_policy(&policy, 0, 0, 0, 0, 0, 0, 0); /* Note there is no policy */ + assert(ipapwd_check_policy(&policy, "password", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + assert(ipapwd_check_policy(&policy, "abccba", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + set_policy(&policy, 0, 0, 3, 0, 0, 0, 0); /* Set anything */ + assert(ipapwd_check_policy(&policy, "abccba", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_PALINDROME); + + /* Dictionary check */ + set_policy(&policy, 0, 0, 0, 0, 0, 1, 0); + assert(ipapwd_check_policy(&policy, "password", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_DICT_WORD); + assert(ipapwd_check_policy(&policy, "Secret123", NULL, 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_DICT_WORD); + + /* User check */ + assert(ipapwd_check_policy(&policy, "userPDQ123", "user", 0, 0, 0, 0, NULL) == IPAPWD_POLICY_OK); + set_policy(&policy, 0, 0, 0, 0, 0, 0, 1); + assert(ipapwd_check_policy(&policy, "userPDQ123", "user", 0, 0, 0, 0, NULL) == IPAPWD_POLICY_PWD_USER); + + return 0; +} From 441bbc71bf03c415e4e3a029bdbb036e9d92d4ac Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 24 Sep 2020 22:39:36 -0400 Subject: [PATCH 06/10] Pass the user to the password policy check in the kdb driver If the entry contains a uid then pass that into the policy checker for the usercheck policy check. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- daemons/ipa-kdb/ipa_kdb.h | 1 + daemons/ipa-kdb/ipa_kdb_passwords.c | 21 +++++++++++++++++- daemons/ipa-kdb/ipa_kdb_principals.c | 10 +++++++++ daemons/ipa-kdb/ipa_kdb_pwdpolicy.c | 33 ++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index 5db3a52f86..3e9b24a531 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -149,6 +149,7 @@ struct ipadb_e_data { bool ipa_user; char *entry_dn; char *passwd; + char *user; time_t last_pwd_change; char *pw_policy_dn; char **pw_history; diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index 9362f4305d..adfefb3a2e 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -53,6 +53,25 @@ static krb5_error_code ipapwd_error_to_kerr(krb5_context context, kerr = KADM5_PASS_Q_CLASS; krb5_set_error_message(context, kerr, "Password is too simple"); break; + case IPAPWD_POLICY_PWD_CONSECUTIVE: + kerr = KADM5_PASS_Q_GENERIC; + krb5_set_error_message(context, kerr, "Password has repeating characters"); + break; + case IPAPWD_POLICY_PWD_SEQUENCE: + kerr = KADM5_PASS_Q_GENERIC; + krb5_set_error_message(context, kerr, "Password contains a monotonic sequence"); + case IPAPWD_POLICY_PWD_PALINDROME: + kerr = KADM5_PASS_Q_GENERIC; + krb5_set_error_message(context, kerr, "Password is a palindrome"); + break; + case IPAPWD_POLICY_PWD_USER: + kerr = KADM5_PASS_Q_GENERIC; + krb5_set_error_message(context, kerr, "Password contains the user name"); + break; + case IPAPWD_POLICY_PWD_DICT_WORD: + kerr = KADM5_PASS_Q_DICT; + krb5_set_error_message(context, kerr, "Password contains dictionary words"); + break; default: kerr = KADM5_PASS_Q_GENERIC; break; @@ -95,7 +114,7 @@ static krb5_error_code ipadb_check_pw_policy(krb5_context context, if (kerr != 0) { return kerr; } - ret = ipapwd_check_policy(ied->pol, passwd, time(NULL), + ret = ipapwd_check_policy(ied->pol, passwd, ied->user, time(NULL), db_entry->expiration, db_entry->pw_expiration, ied->last_pwd_change, diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 3f9d7dcebf..197b798097 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -74,6 +74,7 @@ static char *std_principal_attrs[] = { "krbMaxRenewableAge", /* IPA SPECIFIC ATTRIBUTES */ + "uid", "nsaccountlock", "passwordHistory", IPA_KRB_AUTHZ_DATA_ATTR, @@ -589,6 +590,7 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, krb5_kvno mkvno = 0; char **restrlist; char *restring; + char *uidstring; char **authz_data_list; krb5_timestamp restime; bool resbool; @@ -839,6 +841,13 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, } if (ret == 0) { ied->ipa_user = true; + ret = ipadb_ldap_attr_to_str(lcontext, lentry, + "uid", &uidstring); + if (ret != 0 && ret != ENOENT) { + kerr = ret; + goto done; + } + ied->user = uidstring; } /* check if it has the krbTicketPolicyAux objectclass */ @@ -1551,6 +1560,7 @@ void ipadb_free_principal_e_data(krb5_context kcontext, krb5_octet *e_data) if (ied->magic == IPA_E_DATA_MAGIC) { ldap_memfree(ied->entry_dn); free(ied->passwd); + free(ied->user); free(ied->pw_policy_dn); for (i = 0; ied->pw_history && ied->pw_history[i]; i++) { free(ied->pw_history[i]); diff --git a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c index 10f128700b..03d742b5c3 100644 --- a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c +++ b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c @@ -34,6 +34,10 @@ char *std_pwdpolicy_attrs[] = { "krbpwdmaxfailure", "krbpwdfailurecountinterval", "krbpwdlockoutduration", + "ipapwdmaxrepeat", + "ipapwdmaxsequence", + "ipapwddictcheck", + "ipapwdusercheck", NULL }; @@ -47,6 +51,7 @@ krb5_error_code ipadb_get_ipapwd_policy(struct ipadb_context *ipactx, LDAPMessage *res = NULL; LDAPMessage *lentry; uint32_t result; + bool resbool; int ret; pol = calloc(1, sizeof(struct ipapwd_policy)); @@ -117,6 +122,34 @@ krb5_error_code ipadb_get_ipapwd_policy(struct ipadb_context *ipactx, pol->lockout_duration = result; } + ret = ipadb_ldap_attr_to_uint32(ipactx->lcontext, lentry, + "ipaPwdMaxRepeat", &result); + if (ret == 0) { + pol->max_repeat = result; + } + + ret = ipadb_ldap_attr_to_uint32(ipactx->lcontext, lentry, + "ipaPwdMaxSequence", &result); + if (ret == 0) { + pol->max_sequence = result; + } + + ret = ipadb_ldap_attr_to_bool(ipactx->lcontext, lentry, + "ipaPwdDictCheck", &resbool); + if (ret == 0 && resbool == true) { + pol->dictcheck = 1; + } + + ret = ipadb_ldap_attr_to_bool(ipactx->lcontext, lentry, + "ipaPwdUserCheck", &resbool); + if (ret == 0 && resbool == true) { + pol->usercheck = 1; + } + + if (ret == 0) { + pol->max_sequence = result; + } + *_pol = pol; done: From c4e6932145d0d8768a05ceaed92baec91218cd7e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Sat, 26 Sep 2020 19:40:02 -0400 Subject: [PATCH 07/10] Add a raiseonerr option to ldappasswd_user_change This is so on tests for bad password one can catch the error message. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- ipatests/pytest_ipa/integration/tasks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py index fceac1b628..e7c88363ec 100755 --- a/ipatests/pytest_ipa/integration/tasks.py +++ b/ipatests/pytest_ipa/integration/tasks.py @@ -1751,7 +1751,8 @@ def get_host_ip_with_hostmask(host): return None -def ldappasswd_user_change(user, oldpw, newpw, master, use_dirman=False): +def ldappasswd_user_change(user, oldpw, newpw, master, use_dirman=False, + raiseonerr=True): container_user = dict(DEFAULT_CONFIG)['container_user'] basedn = master.domain.basedn @@ -1766,7 +1767,7 @@ def ldappasswd_user_change(user, oldpw, newpw, master, use_dirman=False): else: args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw, '-s', newpw, '-x', '-ZZ', '-H', master_ldap_uri] - master.run_command(args) + return master.run_command(args, raiseonerr=raiseonerr) def ldappasswd_sysaccount_change(user, oldpw, newpw, master, use_dirman=False): From 0b7e72ab5cc32237366fb6c837627127eeecbefd Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Sat, 26 Sep 2020 19:42:12 -0400 Subject: [PATCH 08/10] ipatests: add test for password policies Primarily testing integration of libpwpolicy but it also exercises some of the existing policy. https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- ipatests/test_integration/test_pwpolicy.py | 265 +++++++++++++++++++++ 1 file changed, 265 insertions(+) create mode 100644 ipatests/test_integration/test_pwpolicy.py diff --git a/ipatests/test_integration/test_pwpolicy.py b/ipatests/test_integration/test_pwpolicy.py new file mode 100644 index 0000000000..75626add4c --- /dev/null +++ b/ipatests/test_integration/test_pwpolicy.py @@ -0,0 +1,265 @@ +# +# Copyright (C) 2020 FreeIPA Contributors see COPYING for license +# +"""Misc test for 'ipa' CLI regressions +""" +from __future__ import absolute_import + +import pytest + +from ipatests.test_integration.base import IntegrationTest + +from ipatests.pytest_ipa.integration import tasks + +USER = 'tuser' +PASSWORD = 'Secret123' +POLICY = 'test' + + +class TestPWPolicy(IntegrationTest): + """ + Test password policy in action. + """ + + topology = 'line' + + @classmethod + def install(cls, mh): + super(TestPWPolicy, cls).install(mh) + + tasks.kinit_admin(cls.master) + cls.master.run_command(['ipa', 'user-add', USER, + '--first', 'Test', + '--last', 'User']) + cls.master.run_command(['ipa', 'group-add', POLICY]) + cls.master.run_command(['ipa', 'group-add-member', POLICY, + '--users', USER]) + cls.master.run_command(['ipa', 'pwpolicy-add', POLICY, + '--priority', '1']) + cls.master.run_command(['ipa', 'passwd', USER], + stdin_text='{password}\n{password}\n'.format( + password=PASSWORD + )) + + def kinit_as_user(self, host, old_password, new_password, user=USER, + raiseonerr=True): + """kinit to an account with an expired password""" + return host.run_command( + ['kinit', user], + raiseonerr=raiseonerr, + stdin_text='{old}\n{new}\n{new}\n'.format( + old=old_password, new=new_password + ), + ) + + def reset_password(self, host, user=USER, password=PASSWORD): + tasks.kinit_admin(host) + host.run_command(['ipa', 'passwd', user], + stdin_text='{password}\n{password}\n'.format( + password=password + ) + ) + + def set_pwpolicy(self, minlength=None, maxrepeat=None, maxsequence=None, + dictcheck=None, usercheck=None, minclasses=None): + tasks.kinit_admin(self.master) + args = ["ipa", "pwpolicy-mod", POLICY] + if minlength is not None: + args.append("--minlength={}".format(minlength)) + if maxrepeat is not None: + args.append("--maxrepeat={}".format(maxrepeat)) + if maxsequence is not None: + args.append("--maxsequence={}".format(maxsequence)) + if dictcheck is not None: + args.append("--dictcheck={}".format(dictcheck)) + if usercheck is not None: + args.append("--usercheck={}".format(usercheck)) + if minclasses is not None: + args.append("--minclasses={}".format(minclasses)) + self.master.run_command(args) + + self.reset_password(self.master) + + def clean_pwpolicy(self): + """Set all policy values we care about to zero/false""" + self.master.run_command( + ["ipa", "pwpolicy-mod", POLICY, + "--maxrepeat", "0", + "--maxsequence", "0", + "--usercheck", "false", + "--dictcheck" ,"false", + "--minlife", "0", + "--minlength", "0", + "--minclasses", "0", + ], + ) + + @pytest.fixture + def reset_pwpolicy(self): + """Fixture to ensure policy is reset between tests""" + yield + tasks.kinit_admin(self.master) + self.clean_pwpolicy() + + def test_maxrepeat(self, reset_pwpolicy): + self.set_pwpolicy(maxrepeat=2) + # good passwords + for password in ('Secret123', 'Password'): + self.reset_password(self.master) + self.kinit_as_user(self.master, PASSWORD, password) + self.reset_password(self.master) + tasks.ldappasswd_user_change(USER, PASSWORD, password, self.master) + + self.reset_password(self.master) + + # bad passwords + for password in ('Secret1111', 'passsword'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password has too many consecutive characters' in \ + result.stdout_text + + def test_maxsequence(self, reset_pwpolicy): + self.set_pwpolicy(maxsequence=3) + # good passwords + for password in ('Password123', 'Passwordabc'): + self.reset_password(self.master) + self.kinit_as_user(self.master, PASSWORD, password) + self.reset_password(self.master) + tasks.ldappasswd_user_change(USER, PASSWORD, password, self.master) + + self.reset_password(self.master) + + # bad passwords + for password in ('Password1234', 'Passwordabcde'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password contains a monotonic sequence' in \ + result.stdout_text + + def test_usercheck(self, reset_pwpolicy): + self.set_pwpolicy(usercheck=True) + for password in ('tuserpass', 'passoftuser'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password contains username' in \ + result.stdout_text + + # test with valid password + self.kinit_as_user(self.master, PASSWORD, 'bamOncyftAv0') + + def test_dictcheck(self, reset_pwpolicy): + self.set_pwpolicy(dictcheck=True) + for password in ('password', 'bookends', 'BaLtim0re'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password is based on a dictionary word' in \ + result.stdout_text + + # test with valid password + self.kinit_as_user(self.master, PASSWORD, 'bamOncyftAv0') + + def test_minclasses(self, reset_pwpolicy): + self.set_pwpolicy(minclasses=2) + for password in ('password', 'bookends'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password does not contain enough character' in \ + result.stdout_text + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password is too simple' in \ + result.stdout_text + + # test with valid password + for valid in ('Password', 'password1', 'password!'): + self.kinit_as_user(self.master, PASSWORD, valid) + self.reset_password(self.master) + + self.set_pwpolicy(minclasses=3) + for password in ('password1', 'Bookends'): + result = self.kinit_as_user(self.master, PASSWORD, password, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password does not contain enough character' in \ + result.stdout_text + result = tasks.ldappasswd_user_change(USER, PASSWORD, password, + self.master, + raiseonerr=False) + assert result.returncode == 1 + assert 'Password is too simple' in \ + result.stdout_text + + # test with valid password + for valid in ('Passw0rd', 'password1!', 'Password!'): + self.kinit_as_user(self.master, PASSWORD, valid) + self.reset_password(self.master) + + def test_minlength_mod(self, reset_pwpolicy): + """Test that the pwpolicy minlength overrides our policy + """ + + # With a minlength of 4 all settings of pwq should fail + self.master.run_command( + ["ipa", "pwpolicy-mod", POLICY, + "--minlength", "4",] + ) + for values in (('--maxrepeat', '4'), + ('--maxsequence', '4'), + ('--dictcheck', 'true'), + ('--usercheck', 'true')): + args = ["ipa", "pwpolicy-mod", POLICY] + args.extend(values) + result = self.master.run_command(args, raiseonerr=False) + assert result.returncode != 0 + assert 'minlength' in result.stderr_text + + # With any pwq value set, setting minlife < 6 should fail + for values in (('--maxrepeat', '4'), + ('--maxsequence', '4'), + ('--dictcheck', 'true'), + ('--usercheck', 'true')): + self.clean_pwpolicy() + args = ["ipa", "pwpolicy-mod", POLICY] + args.extend(values) + self.master.run_command(args) + result = self.master.run_command( + ["ipa", "pwpolicy-mod", POLICY, + "--minlength", "4",], raiseonerr=False + ) + assert result.returncode != 0 + assert 'minlength' in result.stderr_text + + def test_minlength_add(self): + """Test that adding a new policy with minlength is caught. + """ + result = self.master.run_command( + ["ipa", "pwpolicy-add", "test_add", + "--maxrepeat", "4", "--minlength", "4", "--priority", "2"], + raiseonerr=False + ) + assert result.returncode != 0 + assert 'minlength' in result.stderr_text From 16b69fe64a596786d9d202e23a9c3cca7f0402ee Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Sun, 27 Sep 2020 18:02:54 -0400 Subject: [PATCH 09/10] Add SELinux policy so kadmind can read the crackdb dictionary https://pagure.io/freeipa/issue/6964 https://pagure.io/freeipa/issue/5948 https://pagure.io/freeipa/issue/2445 https://pagure.io/freeipa/issue/298 Signed-off-by: Rob Crittenden --- selinux/ipa.te | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/selinux/ipa.te b/selinux/ipa.te index d80e64a0bf..e658c175f3 100644 --- a/selinux/ipa.te +++ b/selinux/ipa.te @@ -115,6 +115,24 @@ optional_policy(` sssd_stream_connect(ipa_otpd_t) ') +######################################## +# +# password policy local policy +# +optional_policy(` + gen_require(` + type kadmind_t; + type crack_db_t; + class file getattr; + class file open; + class file read; + class dir search; + ') + allow kadmind_t crack_db_t:file { getattr open read }; + allow kadmind_t crack_db_t:dir search; +') + + ######################################## # # ipa-helper local policy From 1d80f813f65c26a9b3114e1238845b766008ce16 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Mon, 28 Sep 2020 14:57:01 -0400 Subject: [PATCH 10/10] Temp commit --- .freeipa-pr-ci.yaml | 2 +- ipatests/prci_definitions/temp_commit.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.freeipa-pr-ci.yaml b/.freeipa-pr-ci.yaml index abcf8c5b63..8065669008 120000 --- a/.freeipa-pr-ci.yaml +++ b/.freeipa-pr-ci.yaml @@ -1 +1 @@ -ipatests/prci_definitions/gating.yaml \ No newline at end of file +ipatests/prci_definitions/temp_commit.yaml \ No newline at end of file diff --git a/ipatests/prci_definitions/temp_commit.yaml b/ipatests/prci_definitions/temp_commit.yaml index ef2e4bfa90..ce057cb344 100644 --- a/ipatests/prci_definitions/temp_commit.yaml +++ b/ipatests/prci_definitions/temp_commit.yaml @@ -68,7 +68,7 @@ jobs: class: RunPytest args: build_url: '{fedora-latest/build_url}' - test_suite: test_integration/test_REPLACEME.py + test_suite: test_integration/test_pwpolicy.py template: *ci-master-latest timeout: 3600 - topology: *master_1repl_1client + topology: *master_1repl