URL: https://github.com/freeipa/freeipa/pull/5915 Author: rcritten Title: #5915: Clean up the PKI securitydomain when removing a server Action: opened
PR body: """ PKI has its own internal knowledge of servers and services in its securitydomain. This has not been cleaned up in the past but is becoming more of an issue as PKI now relies on its securitydomain for more things, and it has a healthcheck that reports inconsistencies. Removing entries is straightforward using the PKI REST API. In order to operate on the API access is needed. There was an unused Security Domain Administrators group that I've added to the resourceACLS we created for managing the securitydomain. The ipara user is added as a member of this group. The REST API binds to the CA using the IPA RA certificate. Related commits are b3c2197b7e4ed18a7febe3efa6396c2272ebccca and ba4df6449aaa0843ab43a1a2b3cb1df8bb022c24. These resourceACLS were originally created as a backwards compatibility mechanism for dogtag v9 and later only created when a replica was installed purportedly to save a restart. I don't see any reason to not have these defined. They are apparently needed due to the PKI database upgrade issues. In any case if the purpose was to suppress these ACLS it failed because as soon as a replica with a CA was installed they were as well, and we need this ACL in order to manage the securitydomain. https://pagure.io/freeipa/issue/8930 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5915/head:pr5915 git checkout pr5915
From dcf84b8369d81920d7133ce3a2f734d81da63d70 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 20 Jul 2021 17:35:56 -0400 Subject: [PATCH 1/3] Clean up the PKI securitydomain when removing a server PKI has its own internal knowledge of servers and services in its securitydomain. This has not been cleaned up in the past but is becoming more of an issue as PKI now relies on its securitydomain for more things, and it has a healthcheck that reports inconsistencies. Removing entries is straightforward using the PKI REST API. In order to operate on the API access is needed. There was an unused Security Domain Administrators group that I've added to the resourceACLS we created for managing the securitydomain. The ipara user is added as a member of this group. The REST API binds to the CA using the IPA RA certificate. Related commits are b3c2197b7e4ed18a7febe3efa6396c2272ebccca and ba4df6449aaa0843ab43a1a2b3cb1df8bb022c24. These resourceACLS were originally created as a backwards compatibility mechanism for dogtag v9 and later only created when a replica was installed purportedly to save a restart. I don't see any reason to not have these defined. They are apparently needed due to the PKI database upgrade issues. In any case if the purpose was to suppress these ACLS it failed because as soon as a replica with a CA was installed they were as well, and we need this ACL in order to manage the securitydomain. https://pagure.io/freeipa/issue/8930 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- install/updates/50-dogtag10-migration.update | 5 ++++- ipaserver/install/cainstance.py | 12 ++++++++---- ipaserver/install/server/upgrade.py | 11 +++++++++++ ipaserver/plugins/dogtag.py | 20 ++++++++++++++++++++ ipaserver/plugins/server.py | 3 +++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/install/updates/50-dogtag10-migration.update b/install/updates/50-dogtag10-migration.update index 0070c308aef..cbbf49a0815 100644 --- a/install/updates/50-dogtag10-migration.update +++ b/install/updates/50-dogtag10-migration.update @@ -14,6 +14,9 @@ addifexist:resourceACLS:certServer.ca.certrequests:execute:allow (execute) group addifexist:resourceACLS:certServer.ca.certs:execute:allow (execute) group="Certificate Manager Agents":Agents may execute cert operations addifexist:resourceACLS:certServer.ca.groups:execute:allow (execute) group="Administrators":Admins may execute group operations addifexist:resourceACLS:certServer.ca.users:execute:allow (execute) group="Administrators":Admins may execute user operations -replace:resourceACLS:certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group":Anybody is allowed to read domain.xml but only Subsystem group is allowed to modify the domain.xml::certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group" || group="Enterprise CA Administrators" || group="Enterprise KRA Administrators" || group="Enterprise RA Administrators" || group="Enterprise OCSP Administrators" || group="Enterprise TKS Administrators" || group="Enterprise TPS Administrators":Anybody is allowed to read domain.xml but only Subsystem group and Enterprise Administrators are allowed to modify the domain.xml +# new installation +replace:resourceACLS:certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group":Anybody is allowed to read domain.xml but only Subsystem group is allowed to modify the domain.xml::certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group" || group="Enterprise CA Administrators" || group="Enterprise KRA Administrators" || group="Enterprise RA Administrators" || group="Enterprise OCSP Administrators" || group="Enterprise TKS Administrators" || group="Enterprise TPS Administrators" || group="Security Domain Administrators":Anybody is allowed to read domain.xml but only Subsystem group and Enterprise Administrators are allowed to modify the domain.xml +# upgraded installation +replace:resourceACLS:certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group" || group="Enterprise CA Administrators" || group="Enterprise KRA Administrators" || group="Enterprise RA Administrators" || group="Enterprise OCSP Administrators" || group="Enterprise TKS Administrators" || group="Enterprise TPS Administrators":Anybody is allowed to read domain.xml but only Subsystem group and Enterprise Administrators are allowed to modify the domain.xml::certServer.securitydomain.domainxml:read,modify:allow (read) user="anybody";allow (modify) group="Subsystem Group" || group="Enterprise CA Administrators" || group="Enterprise KRA Administrators" || group="Enterprise RA Administrators" || group="Enterprise OCSP Administrators" || group="Enterprise TKS Administrators" || group="Enterprise TPS Administrators" || group="Security Domain Administrators":Anybody is allowed to read domain.xml but only Subsystem group and Enterprise Administrators are allowed to modify the domain.xml replace:resourceACLS:certServer.ca.connectorInfo:read,modify:allow (modify,read) group="Enterprise KRA Administrators":Only Enterprise Administrators are allowed to update the connector information::certServer.ca.connectorInfo:read,modify:allow (read) group="Enterprise KRA Administrators";allow (modify) group="Enterprise KRA Administrators" || group="Subsystem Group":Only Enterprise Administrators and Subsystem Group are allowed to update the connector information addifexist:resourceACLS:certServer.profile.configuration:read,modify:allow (read,modify) group="Certificate Manager Agents":Certificate Manager agents may modify (create/update/delete) and read profiles diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index dbcf5fbd294..9e842b33e51 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -435,10 +435,9 @@ def configure_instance(self, host_name, dm_password, admin_password, configure_lightweight_ca_acls) self.step("Ensure lightweight CAs container exists", ensure_lightweight_cas_container) - if self.clone and not promote: - self.step( - "Ensuring backward compatibility", - self.__dogtag10_migration) + self.step( + "Ensuring backward compatibility", + self.__dogtag10_migration) if promote: self.step("destroying installation admin user", self.teardown_admin) @@ -794,6 +793,11 @@ def __create_ca_agent(self): self.basedn) conn.add_entry_to_group(user_dn, group_dn, 'uniqueMember') + # add ipara user to Security Domain Administrators group + group_dn = DN(('cn', 'Security Domain Administrators'), + ('ou', 'groups'), self.basedn) + conn.add_entry_to_group(user_dn, group_dn, 'uniqueMember') + def __get_ca_chain(self): try: return dogtag.get_ca_certchain(ca_host=self.fqdn) diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py index 6bc63661e62..662450f3146 100644 --- a/ipaserver/install/server/upgrade.py +++ b/ipaserver/install/server/upgrade.py @@ -1157,6 +1157,16 @@ def add_default_caacl(ca): sysupgrade.set_upgrade_state('caacl', 'add_default_caacl', True) +def add_agent_to_security_domain_admins(): + user_dn = DN(('uid', "ipara"), ('ou', 'People'), ('o', 'ipaca')) + group_dn = DN(('cn', 'Security Domain Administrators'), ('ou', 'groups'), + ('o', 'ipaca')) + try: + api.Backend.ldap2.add_entry_to_group(user_dn, group_dn, 'uniqueMember') + except ipalib.errors.AlreadyGroupMember: + pass + + def setup_pkinit(krb): logger.info("[Setup PKINIT]") @@ -1837,6 +1847,7 @@ def upgrade_configuration(): migrate_to_authselect() add_systemd_user_hbac() add_admin_root_alias() + add_agent_to_security_domain_admins() sssd_update() diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index f2dc6e4ab09..be2e4bb4e2a 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -2117,3 +2117,23 @@ def enable_ca(self, ca_id): def delete_ca(self, ca_id): self._ssldo('DELETE', ca_id) + + +@register() +class ra_securitydomain(RestClient): + """ + Security domain management backend plugin. + + Dogtag handles the creation of securitydomain entries + we need to clean them up when an IPA server is removed. + """ + path = 'securityDomain/hosts' + + def delete_domain(self, hostname, type): + """ + Delete a security domain + """ + self._ssldo( + 'DELETE', f'{type}%20{hostname}%20443', + headers={'Accept': 'application/json'} + ) diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py index b3dda846923..c1092d6f4d5 100644 --- a/ipaserver/plugins/server.py +++ b/ipaserver/plugins/server.py @@ -753,6 +753,9 @@ def pre_callback(self, ldap, dn, *keys, **options): self._ensure_last_of_role( pkey, ignore_last_of_role=options.get('ignore_last_of_role', False) ) + with self.api.Backend.ra_securitydomain as domain_api: + domain_api.delete_domain(pkey, 'KRA') + domain_api.delete_domain(pkey, 'CA') # remove the references to master's ldap/http principals self._remove_server_principal_references(pkey) From e965493f3cc047cdcbf4794844354b4a2261d33e Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Wed, 21 Jul 2021 16:21:21 -0400 Subject: [PATCH 2/3] ipatests: Verify that securitydomain is updated on server-del For every server-del ensure that the server being deleted is also removed from the PKI securitydomain. https://pagure.io/freeipa/issue/8930 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipatests/test_integration/test_server_del.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py index 5e627d5db10..2241d88312a 100644 --- a/ipatests/test_integration/test_server_del.py +++ b/ipatests/test_integration/test_server_del.py @@ -44,6 +44,13 @@ def check_master_removal(host, hostname_to_remove, returncode=2 ) + host.run_command(['pki', 'client', 'init', '--force']) + result = host.run_command( + ['pki', 'securitydomain-host-find'], + stdin='y\n', + ).stdout_text + assert hostname_to_remove not in result + def check_removal_disconnects_topology( host, hostname_to_remove, From 020f660678bce129cb4b66f612bbcf0f220a2d30 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Mon, 19 Jul 2021 21:57:45 -0400 Subject: [PATCH 3/3] Temp commit --- .freeipa-pr-ci.yaml | 2 +- ipatests/prci_definitions/temp_commit.yaml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.freeipa-pr-ci.yaml b/.freeipa-pr-ci.yaml index abcf8c5b634..80656690080 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 af5885d0af2..1546d7f0194 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_server_del.py template: *ci-master-latest - timeout: 3600 - topology: *master_1repl_1client + timeout: 10800 + topology: *master_2repl_1client
_______________________________________________ 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
