On 19.02.2016 15:02, Alexander Bokovoy wrote:
On Fri, 19 Feb 2016, Petr Vobornik wrote:
On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:
On Fri, 19 Feb 2016, Martin Basti wrote:
WIP patch attached
https://fedorahosted.org/freeipa/ticket/5665
Comments inline.
+ # we need to run sidgen task
+ sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
+ "cn=config")
+ sidgen_tasks_attr = {
+ "objectclass": ["top", "extensibleObject"],
+ "cn": ["sidgen"],
+ "delay": [0],
+ "nsslapd-basedn": [self.api.env.basedn],
+ }
May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?
+
+ task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+ try:
+ ldap.add_entry(task_entry)
+ except errors.DuplicateEntry:
+ self.log.debug("sidgen task already created")
+ else:
+ self.log.debug("sidgen task has been created")
There could be multiple tasks running in parallel, that's why it could
be good to use a different and unique name.
+ # we have to check all trusts domains which have been added
after the
+ # upgrade that caused bug was done.
+
+ base_dn = DN(self.api.env.container_adtrusts,
self.api.env.basedn)
+ trust_domain_entries, truncated = ldap.find_entries(
+ base_dn=base_dn,
+ scope=ldap.SCOPE_ONELEVEL,
+ attrs_list=[attr_name, "cn"],
+ )
+
+ if truncated:
+ self.log.warning("update_sids: Search results were
truncated")
+
+ for entry in trust_domain_entries:
+ if entry.single_value[attr_name] is None:
+ domain = entry.single_value["cn"]
+ self.log.error(
+ "Your trust to %s is broken. Please re-create it
by "
+ "running 'ipa trust-add' again", domain)
+
+ sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+ return False, ()
This part looks fine. Basically, a similar check needs to be added to
trust_find, trust_show, and may be trust_add.
Why trust-add?
I'm not a big fan of cluttering existing commands(find, show, mod)
with logic to fix one upgrade bug. But I understand a need to
communicate it somehow.
Would it make sense to move such logic to a separate command, e.g.
trust-check/trust-verify? The command can do additional check in a
future.
No. What is the value of trust-add if it says you 'Trust established and
verified' while in reality it didn't? This specific issue is very easy
to catch.
Patch attached, patch with warning will land soon.
From 65fb67c32abb136fdd5568fd48c68b5e32a78446 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 18 Feb 2016 19:59:50 +0100
Subject: [PATCH] upgrade: fix config of sidgen and extdom plugins
During upgrade to IPA 4.2, literally "$SUFFIX" value was added to
configuration of sidgen and extdom plugins. This cause that SID are not properly configured.
Upgrade must fix "$SUFFIX" to reals suffix DN, and run sidgen task
against IPA domain (if exists).
All trusts added when plugins configuration was broken must be re-added.
https://fedorahosted.org/freeipa/ticket/5665
---
install/updates/90-post_upgrade_plugins.update | 2 +
ipaserver/install/dsinstance.py | 12 +-
ipaserver/install/plugins/adtrust.py | 151 +++++++++++++++++++++++++
ipaserver/install/server/upgrade.py | 4 +-
4 files changed, 161 insertions(+), 8 deletions(-)
diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 5642021ad93cf336db2d872bf3ef6db99b5ffa46..9c9ee160fffedbc8e8d59705169e6cf08ddc9779 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -5,6 +5,8 @@
plugin: update_ca_topology
plugin: update_dnszones
plugin: update_dns_limits
+plugin: update_sigden_extdom_broken_config
+plugin: update_sids
plugin: update_default_range
plugin: update_default_trust_view
plugin: update_ca_renewal_master
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 2905c31c64c4fa8bd034d2cdf6ce5d90ecfea091..f474e189a47f945b7c91cba4ccc17266b9c7e430 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -1054,9 +1054,9 @@ class DsInstance(service.Service):
"""
Add sidgen directory server plugin configuration if it does not already exist.
"""
- self._ldap_mod('ipa-sidgen-conf.ldif', self.sub_dict)
+ self.add_sidgen_plugin(self.sub_dict['SUFFIX'])
- def add_sidgen_plugin(self):
+ def add_sidgen_plugin(self, suffix):
"""
Add sidgen plugin configuration only if it does not already exist.
"""
@@ -1064,7 +1064,7 @@ class DsInstance(service.Service):
try:
self.admin_conn.get_entry(dn)
except errors.NotFound:
- self._add_sidgen_plugin()
+ self._ldap_mod('ipa-sidgen-conf.ldif', dict(SUFFIX=suffix))
else:
root_logger.debug("sidgen plugin is already configured")
@@ -1072,9 +1072,9 @@ class DsInstance(service.Service):
"""
Add directory server configuration for the extdom extended operation.
"""
- self._ldap_mod('ipa-extdom-extop-conf.ldif', self.sub_dict)
+ self.add_extdom_plugin(self.sub_dict['SUFFIX'])
- def add_extdom_plugin(self):
+ def add_extdom_plugin(self, suffix):
"""
Add extdom configuration if it does not already exist.
"""
@@ -1082,7 +1082,7 @@ class DsInstance(service.Service):
try:
self.admin_conn.get_entry(dn)
except errors.NotFound:
- self._add_extdom_plugin()
+ self._ldap_mod('ipa-extdom-extop-conf.ldif', dict(SUFFIX=suffix))
else:
root_logger.debug("extdom plugin is already configured")
diff --git a/ipaserver/install/plugins/adtrust.py b/ipaserver/install/plugins/adtrust.py
index df9412adba833251cc1c70d7d72ebebbdc77c2a4..32cd7d7eba54c78f06c7bee2f43c0c8be2d8e58f 100644
--- a/ipaserver/install/plugins/adtrust.py
+++ b/ipaserver/install/plugins/adtrust.py
@@ -21,6 +21,8 @@ from ipalib import api, errors
from ipalib import Updater
from ipapython.dn import DN
from ipapython.ipa_log_manager import root_logger
+from ipaserver.install import sysupgrade
+
DEFAULT_ID_RANGE_SIZE = 200000
@@ -161,5 +163,154 @@ class update_default_trust_view(Updater):
return False, [update]
+
+class update_sigden_extdom_broken_config(Updater):
+ """Fix configuration of sidgen and extdom plugins
+
+ Upgrade to IPA 4.2+ cause that sidgen and extdom plugins have improperly
+ configured basedn.
+
+ All trusts which have been added when config was broken must to be
+ re-added manually.
+
+ https://fedorahosted.org/freeipa/ticket/5665
+ """
+
+ sidgen_config_dn = DN("cn=IPA SIDGEN,cn=plugins,cn=config")
+ extdom_config_dn = DN("cn=ipa_extdom_extop,cn=plugins,cn=config")
+
+ def _fix_config(self):
+ """Due upgrade error configuration of sidgen and extdom plugins may
+ contain literally "$SUFFIX" value instead of real DN in nsslapd-basedn
+ attribute
+
+ :return: True if config was fixed, False if fix is not needed
+ """
+ ldap = self.api.Backend.ldap2
+ basedn_attr = 'nsslapd-basedn'
+ modified = False
+
+ for dn in (self.sidgen_config_dn, self.extdom_config_dn):
+ try:
+ entry = ldap.get_entry(dn, attrs_list=[basedn_attr])
+ except errors.NotFound:
+ self.log.debug("configuration for %s not found, skipping", dn)
+ else:
+ configured_suffix = entry.single_value.get(basedn_attr)
+ if configured_suffix is None:
+ raise RuntimeError(
+ "Missing attribute {attr} in {dn}".format(
+ attr=basedn_attr, dn=dn
+ )
+ )
+ elif configured_suffix == "$SUFFIX":
+ # configured value is wrong, fix it
+ entry.single_value[basedn_attr] = str(self.api.env.basedn)
+ self.log.debug("updating attribute %s of %s to correct "
+ "value %s", basedn_attr, dn,
+ self.api.env.basedn)
+ ldap.update_entry(entry)
+ modified = True
+ else:
+ self.log.debug("configured basedn for %s is okay", dn)
+
+ return modified
+
+ def execute(self, **options):
+ if sysupgrade.get_upgrade_state('sidgen', 'config_basedn_updated'):
+ self.log.debug("Already done, skipping")
+ return False, ()
+
+ restart = False
+ if self._fix_config():
+ sysupgrade.set_upgrade_state('sidgen', 'update_sids', True)
+ restart = True # DS has to be restarted to apply changes
+
+ sysupgrade.set_upgrade_state('sidgen', 'config_basedn_updated', True)
+ return restart, ()
+
+
+class update_sids(Updater):
+ """SIDs may be not created properly if bug with wrong configuration for
+ sidgen and extdom plugins is effective
+
+ This must be run after "update_sigden_extdom_broken_config"
+ https://fedorahosted.org/freeipa/ticket/5665
+ """
+ sidgen_config_dn = DN("cn=IPA SIDGEN,cn=plugins,cn=config")
+
+ def execute(self, **options):
+ ldap = self.api.Backend.ldap2
+
+ if sysupgrade.get_upgrade_state('sidgen', 'update_sids') is not True:
+ self.log.debug("SIDs do not need to be generated")
+ return False, ()
+
+ # check if IPA domain for AD trust has been created, and if we need to
+ # regenerate missing SIDs if attribute 'ipaNTSecurityIdentifier'
+ domain_IPA_AD_dn = DN(
+ ('cn', self.api.env.domain),
+ self.api.env.container_cifsdomains,
+ self.api.env.basedn)
+ attr_name = 'ipaNTSecurityIdentifier'
+
+ try:
+ entry = ldap.get_entry(domain_IPA_AD_dn, attrs_list=[attr_name])
+ except errors.NotFound:
+ self.log.debug("IPA domain object %s is not configured",
+ domain_IPA_AD_dn)
+ sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+ return False, ()
+ else:
+ if not entry.single_value.get(attr_name):
+ # we need to run sidgen task
+ sidgen_task_dn = DN(
+ "cn=generate domain sid,cn=ipa-sidgen-task,cn=tasks,"
+ "cn=config")
+ sidgen_tasks_attr = {
+ "objectclass": ["top", "extensibleObject"],
+ "cn": ["sidgen"],
+ "delay": [0],
+ "nsslapd-basedn": [self.api.env.basedn],
+ }
+
+ task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+ try:
+ ldap.add_entry(task_entry)
+ except errors.DuplicateEntry:
+ self.log.debug("sidgen task already created")
+ else:
+ self.log.debug("sidgen task has been created")
+
+ # we have to check all trusts domains which may been affected by the
+ # bug. Symptom is missing 'ipaNTSecurityIdentifier' attribute
+
+ base_dn = DN(self.api.env.container_adtrusts, self.api.env.basedn)
+ try:
+ trust_domain_entries, truncated = ldap.find_entries(
+ base_dn=base_dn,
+ scope=ldap.SCOPE_ONELEVEL,
+ attrs_list=[attr_name, "cn"],
+ )
+ except errors.EmptyResult:
+ trust_domain_entries = []
+ else:
+ if truncated:
+ self.log.warning("update_sids: Search results were truncated")
+
+ for entry in trust_domain_entries:
+ if entry.single_value.get(attr_name) is None:
+ domain = entry.single_value["cn"]
+ self.log.error(
+ "Your trust to %s is broken. Please re-create it by "
+ "running 'ipa trust-add' again.", domain)
+
+ sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+ return False, ()
+
+
api.register(update_default_range)
api.register(update_default_trust_view)
+api.register(update_sids)
+api.register(update_sigden_extdom_broken_config)
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 455b28acc3aedbefb22ee2e294ceab031d253348..61f33c9bd99db8581897f812767cb869301ad5f0 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1369,8 +1369,8 @@ def ds_enable_sidgen_extdom_plugins(ds):
root_logger.debug('sidgen and extdom plugins are enabled already')
return
- ds.add_sidgen_plugin()
- ds.add_extdom_plugin()
+ ds.add_sidgen_plugin(api.env.basedn)
+ ds.add_extdom_plugin(api.env.basedn)
sysupgrade.set_upgrade_state('ds', 'enable_ds_sidgen_extdom_plugins', True)
def ca_upgrade_schema(ca):
--
2.5.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
