URL: https://github.com/freeipa/freeipa/pull/1830 Author: frasertweedale Title: #1830: certprofile: reject config with multiple profileIds Action: opened
PR body: """ In certprofile-import if the config file contains two profileId directives with different values, with the first matching the profile ID CLI argument and the second differing, the profile gets imported under the second ID. This leads to: - failure to enable the profile - failure to add the IPA "tracking" certprofile object - inability to delete the misnamed profile from Dogtag (via ipa CLI) To avert this scenario, detect and reject profile configurations where profileId is specified multiple times (whether or not the values differ). https://pagure.io/freeipa/issue/7503 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1830/head:pr1830 git checkout pr1830
From 3af5489e59573b7642b328596f97b304a7b3b1aa Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 18 Apr 2018 17:10:10 +1000 Subject: [PATCH 1/2] certprofile: reject config with multiple profileIds In certprofile-import if the config file contains two profileId directives with different values, with the first matching the profile ID CLI argument and the second differing, the profile gets imported under the second ID. This leads to: - failure to enable the profile - failure to add the IPA "tracking" certprofile object - inability to delete the misnamed profile from Dogtag (via ipa CLI) To avert this scenario, detect and reject profile configurations where profileId is specified multiple times (whether or not the values differ). https://pagure.io/freeipa/issue/7503 --- ipaserver/plugins/certprofile.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ipaserver/plugins/certprofile.py b/ipaserver/plugins/certprofile.py index 4eab09f24f..5e8dbca046 100644 --- a/ipaserver/plugins/certprofile.py +++ b/ipaserver/plugins/certprofile.py @@ -236,14 +236,25 @@ def pre_callback(self, ldap, dn, entry, entry_attrs, *keys, **options): ca_enabled_check(self.api) context.profile = options['file'] - match = self.PROFILE_ID_PATTERN.search(options['file']) - if match is None: + matches = self.PROFILE_ID_PATTERN.findall(options['file']) + if len(matches) == 0: # no profileId found, use CLI value as profileId. context.profile = u'profileId=%s\n%s' % (keys[0], context.profile) - elif keys[0] != match.group(1): - raise errors.ValidationError(name='file', - error=_("Profile ID '%(cli_value)s' does not match profile data '%(file_value)s'") - % {'cli_value': keys[0], 'file_value': match.group(1)} + elif len(matches) > 1: + raise errors.ValidationError( + name='file', + error=_( + "Profile data specifies profileId multiple times: " + "%(values)s" + ) % dict(values=matches) + ) + elif keys[0] != matches[0]: + raise errors.ValidationError( + name='file', + error=_( + "Profile ID '%(cli_value)s' " + "does not match profile data '%(file_value)s'" + ) % dict(cli_value=keys[0], file_value=matches[0]) ) return dn From 9c9b4c1479da3fa7408a3a4acbd8e42e07e6b76e Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 18 Apr 2018 19:35:49 +1000 Subject: [PATCH 2/2] certprofile: add tests for config profileId scenarios Update the certprofile tests to cover the various scenarios concerning the profileId property in the profile configuration. The scenarios now explicitly tested are: - profileId not specified (should succeed) - mismatched profileId property (should fail) - multiple profileId properties (should fail) - one profileId property, matching given ID (should succeed) https://pagure.io/freeipa/issue/7503 --- .../test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl | 1 - ipatests/test_xmlrpc/test_certprofile_plugin.py | 33 ++++++++++++++++++++++ ipatests/test_xmlrpc/tracker/certprofile_plugin.py | 11 ++++++-- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl b/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl index cff1544627..f9e8ce441d 100644 --- a/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl +++ b/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl @@ -1,6 +1,5 @@ auth.instance_id=raCertAuth classId=caEnrollImpl -profileId=caIPAserviceCert_mod visible=false desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication. enable=true diff --git a/ipatests/test_xmlrpc/test_certprofile_plugin.py b/ipatests/test_xmlrpc/test_certprofile_plugin.py index 1b3edda89c..7cefd0a8d7 100644 --- a/ipatests/test_xmlrpc/test_certprofile_plugin.py +++ b/ipatests/test_xmlrpc/test_certprofile_plugin.py @@ -222,3 +222,36 @@ class TestImportFromXML(XMLRPC_test): def test_import_xml(self, xmlprofile): with pytest.raises(errors.ExecutionError): xmlprofile.ensure_exists() + + +# The initial user_profile configuration does not specify profileId. +# This is fine (it gets derived from the profile-id CLI argument), +# but this case was already tested in TestProfileCRUD. +# +# This test case tests various scenarios where the profileId *is* +# specified in the profile configuration. These are: +# +# - mismatched profileId property (should fail) +# - multiple profileId properties (should fail) +# - one profileId property, matching given ID (should succeed) +# +@pytest.mark.tier1 +class TestImportProfileIdHandling(XMLRPC_test): + def test_import_with_mismatched_profile_id(self, user_profile): + command = user_profile.make_create_command( + extra_lines=['profileId=bogus'] + ) + with pytest.raises(errors.ValidationError): + command() + + def test_import_with_multiple_profile_id(self, user_profile): + # correct profile id, but two occurrences + prop = u'profileId={}'.format(user_profile.name) + command = user_profile.make_create_command(extra_lines=[prop, prop]) + with pytest.raises(errors.ValidationError): + command() + + def test_import_with_correct_profile_id(self, user_profile): + prop = u'profileId={}'.format(user_profile.name) + command = user_profile.make_create_command(extra_lines=[prop]) + command() diff --git a/ipatests/test_xmlrpc/tracker/certprofile_plugin.py b/ipatests/test_xmlrpc/tracker/certprofile_plugin.py index d9aa302025..a573c2288e 100644 --- a/ipatests/test_xmlrpc/tracker/certprofile_plugin.py +++ b/ipatests/test_xmlrpc/tracker/certprofile_plugin.py @@ -57,7 +57,14 @@ def profile(self): content = f.read() return unicode(content) - def make_create_command(self): + def make_create_command(self, extra_lines=None): + """ + :param extra_lines: list of extra lines to append to profile config. + + """ + if extra_lines is None: + extra_lines = [] + if not self.profile: raise RuntimeError('Tracker object without path to profile ' 'cannot be used to create profile entry.') @@ -65,7 +72,7 @@ def make_create_command(self): return self.make_command('certprofile_import', self.name, description=self.description, ipacertprofilestoreissued=self.store, - file=self.profile) + file=u'\n'.join([self.profile] + extra_lines)) def check_create(self, result): assert_deepequal(dict(
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
