Hello,

DNS upgrade: change forwarding policy to "only" if private IPs are used.

https://fedorahosted.org/freeipa/ticket/5710

This is the upgrade part. I will add one more patch to print a warning in
dnsforwardzone* commands to avoid surprises. Please do not close the ticket yet.

-- 
Petr^2 Spacek
From b1d75807e0f8a117fe4b28f1c83054bb3ff1db6c Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 25 Apr 2016 14:07:16 +0200
Subject: [PATCH] Add ipaDNSVersion option to dnsconfig* commands and use new
 attribute

Ad-hoc LDAP calls in DNS upgrade code were hard to maintain and
ipaConfigString was bad idea from the very beginning as it was hard to
manipulate the number in it.

To avoid problems in future we are introducing new ipaDNSVersion
attribute which is used on cn=dns instead of ipaConfigString.
Original value of ipaConfigString is kept in the tree for now
so older upgraders see it and do not execute the upgrade procedure again.

The attribute can be changed only by installer/upgrade so it is not
exposed in dnsconfig_mod API.

Command dnsconfig_show displays it only if --all option was used.

https://fedorahosted.org/freeipa/ticket/5710
---
 install/share/60ipadns.ldif                    |  2 +
 install/share/dns.ldif                         |  4 +-
 install/updates/40-dns.update                  |  1 -
 install/updates/90-post_upgrade_plugins.update |  1 +
 ipalib/plugins/dns.py                          | 20 +++++--
 ipaserver/install/plugins/dns.py               | 73 ++++++++++++++++++++------
 6 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index e0ed0ab869cea0478d9640bb509c6267abed1a01..71b99d4d03c34591dc83a5706d300727f3f77f30 100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -70,9 +70,11 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME 'idnsSecKeyRevoke' DESC 'DNSKE
 attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME 'idnsSecKeySep' DESC 'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME 'idnsSecAlgorithm' DESC 'DNSKEY algorithm: string used as mnemonic' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME 'idnsSecKeyRef' DESC 'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
+attributeTypes: ( 2.16.840.1.113730.3.8.11.74 NAME 'ipaDNSVersion' DESC 'IPA DNS data version' EQUALITY integerMatch ORDERING integerOrderingMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 X-ORIGIN 'IPA v4.3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord' DESC 'dns Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $ idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $ a6Record $ nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $ mXRecord $ mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $ KeyRecord $ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $ dNameRecord $ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $ TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $ DHCIDRecord $ HIPRecord $ SPFRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $ idnsSOAmName $ idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $ idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowQuery $ idnsAllowTransfer $ idnsAllowSyncPTR $ idnsForwardPolicy $ idnsForwarders $ idnsSecInlineSigning $ nSEC3PARAMRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.2 NAME 'idnsConfigObject' DESC 'DNS global config options' STRUCTURAL MAY ( idnsForwardPolicy $ idnsForwarders $ idnsAllowSyncPTR $ idnsZoneRefresh $ idnsPersistentSearch ) )
 objectClasses: ( 2.16.840.1.113730.3.8.12.18 NAME 'ipaDNSZone' SUP top AUXILIARY MUST idnsName MAY managedBy X-ORIGIN 'IPA v3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.3 NAME 'idnsForwardZone' DESC 'Forward Zone class' SUP top STRUCTURAL MUST ( idnsName $ idnsZoneActive ) MAY ( idnsForwarders $ idnsForwardPolicy ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.4 NAME 'idnsSecKey' DESC 'DNSSEC key metadata' STRUCTURAL MUST ( idnsSecKeyRef $ idnsSecKeyCreated $ idnsSecAlgorithm ) MAY ( idnsSecKeyPublish $ idnsSecKeyActivate $ idnsSecKeyInactive $ idnsSecKeyDelete $ idnsSecKeyZone $ idnsSecKeyRevoke $ idnsSecKeySep $ cn ) X-ORIGIN 'IPA v4.1' )
+objectClasses: ( 2.16.840.1.113730.3.8.12.36 NAME 'ipaDNSContainer' DESC 'IPA DNS container' AUXILIARY MUST ( ipaDNSVersion ) X-ORIGIN 'IPA v4.3' )
diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index 42b41a8d706a8a3fd826320aff6c9333264128fc..d71e2ad7d7ca530247198d9db71aebd497d0c121 100644
--- a/install/share/dns.ldif
+++ b/install/share/dns.ldif
@@ -2,10 +2,10 @@ dn: cn=dns,$SUFFIX
 changetype: add
 objectClass: idnsConfigObject
 objectClass: nsContainer
-objectClass: ipaConfigObject
+objectClass: ipaDNSContainer
 objectClass: top
 cn: dns
-ipaConfigString: DNSVersion 1
+ipaDNSVersion: 1
 aci: (targetattr = "*")(version 3.0; acl "Allow read access"; allow (read,search,compare) groupdn = "ldap:///cn=Read DNS Entries,cn=permissions,cn=pbac,$SUFFIX" or userattr = "parent[0,1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Add DNS entries in a zone";allow (add) userattr = "parent[1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Remove DNS entries from a zone";allow (delete) userattr = "parent[1].managedby#GROUPDN";)
diff --git a/install/updates/40-dns.update b/install/updates/40-dns.update
index 9f64a2f707db5cb0e3503259a0e64d9831ae92f2..4c0824b8350052e47e6f73fc4985cc03e08ee36e 100644
--- a/install/updates/40-dns.update
+++ b/install/updates/40-dns.update
@@ -2,7 +2,6 @@
 # update DNS container
 dn: cn=dns, $SUFFIX
 addifexist: objectClass: idnsConfigObject
-addifexist: objectClass: ipaConfigObject
 addifexist: aci:(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Add DNS entries in a zone";allow (add) userattr = "parent[1].managedby#GROUPDN";)
 addifexist: aci:(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Remove DNS entries from a zone";allow (delete) userattr = "parent[1].managedby#GROUPDN";)
 addifexist: aci:(targetattr = "a6record || aaaarecord || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || mdrecord || minforecord || mxrecord || naptrrecord || nsecrecord || nsec3paramrecord || nsrecord || nxtrecord || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord ")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Update DNS entries in a zone";allow (write) userattr = "parent[0,1].managedby#GROUPDN";)
diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 9c9ee160fffedbc8e8d59705169e6cf08ddc9779..d8498edd4565fbad91ca5dea48e5bc2296e3fc25 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -3,6 +3,7 @@
 
 # middle
 plugin: update_ca_topology
+plugin: update_ipaconfigstring_dnsversion_to_ipadnsversion
 plugin: update_dnszones
 plugin: update_dns_limits
 plugin: update_sigden_extdom_broken_config
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 079cb6d989c2d5d6fef64fa6c6590bd07574bcfa..d352fbddcff955d1b96fb561d456a93a8c8198e5 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -4311,6 +4311,9 @@ class dnsconfig(LDAPObject):
             cli_name='zone_refresh',
             label=_('Zone refresh interval'),
         ),
+        Int('ipadnsversion?',
+            label=_('IPA DNS version'),
+        ),
     )
     managed_permissions = {
         'System: Write DNS Configuration': {
@@ -4353,16 +4356,22 @@ class dnsconfig(LDAPObject):
 
         return entry
 
-    def postprocess_result(self, result):
+    def postprocess_result(self, result, show_version):
         if not any(param in result['result'] for param in self.params):
             result['summary'] = unicode(_('Global DNS configuration is empty'))
 
 
-
 @register()
 class dnsconfig_mod(LDAPUpdate):
     __doc__ = _('Modify global DNS configuration.')
 
+    def get_options(self):
+        """hide ipadnsversion outside of installer/upgrade"""
+        for option in super(dnsconfig_mod, self).get_options():
+            if option.name == 'ipadnsversion':
+                option = option.clone(include=('installer', 'updates'))
+            yield option
+
     def interactive_prompt_callback(self, kw):
 
         # show informative message on client side
@@ -4378,7 +4387,7 @@ class dnsconfig_mod(LDAPUpdate):
         forwarders = options.get('idnsforwarders')
 
         result = super(dnsconfig_mod, self).execute(*keys, **options)
-        self.obj.postprocess_result(result)
+        self.obj.postprocess_result(result, show_version=False)
 
         if forwarders:
             for forwarder in forwarders:
@@ -4416,10 +4425,13 @@ class dnsconfig_show(LDAPRetrieve):
 
     def execute(self, *keys, **options):
         result = super(dnsconfig_show, self).execute(*keys, **options)
-        self.obj.postprocess_result(result)
+        self.obj.postprocess_result(result,
+                                    show_version=options.get('all', False) and
+                                    not options.get('raw', False))
         return result
 
 
+
 @register()
 class dnsforwardzone(DNSZoneBase):
     """
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index c723953274b96c4f3201affef77cbf3d8033106b..b0a8dd4a60510f88eafe2bae6d62358505937b2b 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -30,6 +30,57 @@ from ipalib.plugins.dns import dns_container_exists
 from ipapython.ipa_log_manager import root_logger
 
 
+class DNSUpdater(Updater):
+    def version_update_needed(self, target_version):
+        """Test if IPA DNS version is smaller than target version."""
+        assert isinstance(target_version, int)
+
+        try:
+            return int(self.api.Command['dnsconfig_show'](
+                all=True)['result']['ipadnsversion'][0]) < target_version
+        except errors.NotFound:
+            # IPA DNS is not configured
+            return False
+
+class update_ipaconfigstring_dnsversion_to_ipadnsversion(Updater):
+    """
+    IPA <= 4.3.1 used ipaConfigString "DNSVersion 1" on DNS container.
+    This was hard to deal with in API so from IPA 4.3.2 we are using
+    new ipaDNSVersion attribute with integer syntax.
+    Old ipaConfigString is left there for now so if someone accidentally
+    executes upgrade on an old replica again it will not re-upgrade the data.
+    """
+    def execute(self, **options):
+        ldap = self.api.Backend.ldap2
+        dns_container_dn = DN(self.api.env.container_dns, self.api.env.basedn)
+        try:
+            container_entry = ldap.get_entry(dns_container_dn)
+        except errors.NotFound:
+            # DNS container not found, nothing to upgrade
+            return False, []
+
+        if 'ipadnscontainer' in [
+            o.lower() for o in container_entry['objectclass']
+        ]:
+            # version data are already migrated
+            return False, []
+
+        self.log.debug('Migrating DNS ipaConfigString to ipaDNSVersion')
+        container_entry['objectclass'].append('ipadnscontainer')
+        version = 0
+        for config_option in container_entry.get("ipaConfigString", []):
+            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
+                               config_option, flags=re.I)
+            if matched:
+                version = int(matched.group("version"))
+        container_entry['ipadnsversion'] = version
+        ldap.update_entry(container_entry)
+        self.log.debug('ipaDNSVersion = %s', version)
+        return False, []
+
+api.register(update_ipaconfigstring_dnsversion_to_ipadnsversion)
+
+
 class update_dnszones(Updater):
     """
     Update all zones to meet requirements in the new FreeIPA versions
@@ -140,7 +191,7 @@ class update_dns_limits(Updater):
 api.register(update_dns_limits)
 
 
-class update_master_to_dnsforwardzones(Updater):
+class update_master_to_dnsforwardzones(DNSUpdater):
     """
     Update all zones to meet requirements in the new FreeIPA versions
 
@@ -157,26 +208,14 @@ class update_master_to_dnsforwardzones(Updater):
     def execute(self, **options):
         ldap = self.api.Backend.ldap2
         # check LDAP if forwardzones already uses new semantics
-        dns_container_dn = DN(self.api.env.container_dns, self.api.env.basedn)
-        try:
-            container_entry = ldap.get_entry(dns_container_dn)
-        except errors.NotFound:
-            # DNS container not found, nothing to upgrade
+        if not self.version_update_needed(target_version=1):
+            # forwardzones already uses new semantics,
+            # no upgrade is required
             return False, []
 
-        for config_option in container_entry.get("ipaConfigString", []):
-            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
-                               config_option, flags=re.I)
-            if matched and int(matched.group("version")) >= 1:
-                # forwardzones already uses new semantics,
-                # no upgrade is required
-                return False, []
-
         self.log.debug('Updating forward zones')
         # update the DNSVersion, following upgrade can be executed only once
-        container_entry.setdefault(
-            'ipaConfigString', []).append(u"DNSVersion 1")
-        ldap.update_entry(container_entry)
+        self.api.Command['dnsconfig_mod'](ipadnsversion=1)
 
         # Updater in IPA version from 4.0 to 4.1.2 doesn't work well, this
         # should detect if update in past has been executed, and set proper
-- 
2.5.5

From 3205f96ec48d97c581bb0821ec06dc7c6dad303c Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 27 Apr 2016 12:49:23 +0200
Subject: [PATCH] DNS upgrade: separate backup logic to make it reusable

https://fedorahosted.org/freeipa/ticket/5710
---
 ipaserver/install/plugins/dns.py | 146 ++++++++++++++++++++-------------------
 1 file changed, 74 insertions(+), 72 deletions(-)

diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index b0a8dd4a60510f88eafe2bae6d62358505937b2b..6b540348c8a79751c12b60b7d08fc217e37a51cb 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -31,6 +31,19 @@ from ipapython.ipa_log_manager import root_logger
 
 
 class DNSUpdater(Updater):
+    backup_dir = u'/var/lib/ipa/backup/'
+    # override backup_filename in subclass, it will be mangled by strftime
+    backup_filename = None
+
+    def __init__(self, api):
+        super(DNSUpdater, self).__init__(api)
+        backup_path = u'%s%s' % (self.backup_dir, self.backup_filename)
+        self.backup_path = time.strftime(backup_path)
+        self.ldap = self.api.Backend.ldap2
+        self._ldif_writer = None
+        self._saved_privileges = set()  # store privileges only once
+        self.saved_zone_to_privilege = {}
+
     def version_update_needed(self, target_version):
         """Test if IPA DNS version is smaller than target version."""
         assert isinstance(target_version, int)
@@ -42,6 +55,56 @@ class DNSUpdater(Updater):
             # IPA DNS is not configured
             return False
 
+    @property
+    def ldif_writer(self):
+        if not self._ldif_writer:
+            self.log.info('Original zones will be saved in LDIF format in '
+                          '%s file' % self.backup_path)
+            self._ldif_writer = LDIFWriter(open(self.backup_path, 'w'))
+        return self._ldif_writer
+
+    def backup_zone(self, zone):
+        """Backup zone object, its records, permissions, and privileges.
+
+        Mapping from zone to privilege (containing zone's permissions)
+        will be stored in saved_zone_to_privilege dict for further usage.
+        """
+        dn = str(zone['dn'])
+        del zone['dn']  # dn shouldn't be as attribute in ldif
+        self.ldif_writer.unparse(dn, zone)
+
+        if 'managedBy' in zone:
+            permission = self.ldap.get_entry(DN(zone['managedBy'][0]))
+            self.ldif_writer.unparse(str(permission.dn), dict(permission.raw))
+            for privilege_dn in permission.get('member', []):
+                # privileges can be shared by multiples zones
+                if privilege_dn not in self._saved_privileges:
+                    self._saved_privileges.add(privilege_dn)
+                    privilege = self.ldap.get_entry(privilege_dn)
+                    self.ldif_writer.unparse(str(privilege.dn),
+                                             dict(privilege.raw))
+
+            # remember privileges referened by permission
+            if 'member' in permission:
+                self.saved_zone_to_privilege[
+                    zone['idnsname'][0]
+                ] = permission['member']
+
+        if 'idnszone' in zone['objectClass']:
+            # raw values are required to store into ldif
+            records = self.api.Command['dnsrecord_find'](zone['idnsname'][0],
+                                                         all=True,
+                                                         raw=True,
+                                                         sizelimit=0)['result']
+            for record in records:
+                if record['idnsname'][0] == u'@':
+                    # zone record was saved before
+                    continue
+                dn = str(record['dn'])
+                del record['dn']
+                self.ldif_writer.unparse(dn, record)
+
+
 class update_ipaconfigstring_dnsversion_to_ipadnsversion(Updater):
     """
     IPA <= 4.3.1 used ipaConfigString "DNSVersion 1" on DNS container.
@@ -201,9 +264,7 @@ class update_master_to_dnsforwardzones(DNSUpdater):
 
     This should be applied only once, and only if original version was lower than 4.0
     """
-    backup_dir = u'/var/lib/ipa/backup/'
-    backup_filename = u'dns-forward-zones-backup-%Y-%m-%d-%H-%M-%S.ldif'
-    backup_path = u'%s%s' % (backup_dir, backup_filename)
+    backup_filename = u'dns-master-to-forward-zones-%Y-%m-%d-%H-%M-%S.ldif'
 
     def execute(self, **options):
         ldap = self.api.Backend.ldap2
@@ -255,77 +316,18 @@ class update_master_to_dnsforwardzones(DNSUpdater):
             zones_to_transform.append(zone)
 
         if zones_to_transform:
-            # add time to filename
-            self.backup_path = time.strftime(self.backup_path)
-
-            # DNs of privileges which contain dns managed permissions
-            privileges_to_ldif = set()  # store priviledges only once
-            zone_to_privileges = {}  # zone: [privileges cn]
-
             self.log.info('Zones with specified forwarders with policy different'
                           ' than none will be transformed to forward zones.')
-            self.log.info('Original zones will be saved in LDIF format in '
-                          '%s file' % self.backup_path)
-            try:
-
-                with open(self.backup_path, 'w') as f:
-                    writer = LDIFWriter(f)
-                    for zone in zones_to_transform:
-                        # save backup to ldif
-                        try:
-
-                            dn = str(zone['dn'])
-                            del zone['dn']  # dn shouldn't be as attribute in ldif
-                            writer.unparse(dn, zone)
-
-                            if 'managedBy' in zone:
-                                entry = ldap.get_entry(DN(zone['managedBy'][0]))
-                                for privilege_member_dn in entry.get('member', []):
-                                    privileges_to_ldif.add(privilege_member_dn)
-                                writer.unparse(str(entry.dn), dict(entry.raw))
-
-                                # privileges where permission is used
-                                if entry.get('member'):
-                                    zone_to_privileges[zone['idnsname'][0]] = entry['member']
-
-                            # raw values are required to store into ldif
-                            records = self.api.Command['dnsrecord_find'](
-                                        zone['idnsname'][0],
-                                        all=True,
-                                        raw=True,
-                                        sizelimit=0)['result']
-                            for record in records:
-                                if record['idnsname'][0] == u'@':
-                                    # zone record was saved before
-                                    continue
-                                dn = str(record['dn'])
-                                del record['dn']
-                                writer.unparse(dn, record)
-
-                        except Exception as e:
-                            self.log.error('Unable to backup zone %s' %
-                                           zone['idnsname'][0])
-                            self.log.error(traceback.format_exc())
-                            return False, []
-
-                    for privilege_dn in privileges_to_ldif:
-                        try:
-                            entry = ldap.get_entry(privilege_dn)
-                            writer.unparse(str(entry.dn), dict(entry.raw))
-                        except Exception as e:
-                            self.log.error('Unable to backup privilege %s' %
-                                           privilege_dn)
-                            self.log.error(traceback.format_exc())
-                            return False, []
-
-                    f.close()
-            except Exception:
-                self.log.error('Unable to create backup file')
-                self.log.error(traceback.format_exc())
-                return False, []
-
             # update
             for zone in zones_to_transform:
+                try:
+                    self.backup_zone(zone)
+                except Exception:
+                    self.log.error('Unable to create backup for zone, '
+                                   'terminating zone upgrade')
+                    self.log.error(traceback.format_exc())
+                    return False, []
+
                 # delete master zone
                 try:
                     self.api.Command['dnszone_del'](zone['idnsname'])
@@ -369,9 +371,9 @@ class update_master_to_dnsforwardzones(DNSUpdater):
                         continue
 
                     else:
-                        if zone['idnsname'][0] in zone_to_privileges:
+                        if zone['idnsname'][0] in self.saved_zone_to_privilege:
                             privileges = [
-                                dn[0].value for dn in zone_to_privileges[zone['idnsname'][0]]
+                                dn[0].value for dn in self.saved_zone_to_privilege[zone['idnsname'][0]]
                             ]
                             try:
                                 self.api.Command['permission_add_member'](perm_name,
-- 
2.5.5

From 6ef5f7441bede39c4698f486b48dc35d03d492a5 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 27 Apr 2016 13:26:48 +0200
Subject: [PATCH] Add function ipapython.dnsutil.related_to_auto_empty_zone()

It allows to test if given DNS name is sub/super domain
of an automatic empty zone.

https://fedorahosted.org/freeipa/ticket/5710
---
 ipapython/dnsutil.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index 240b7c9cd2168db17b9fa673dc4eefcfa78fef45..0195f4507183a82eae30925b39de1796f0b0f2cf 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -228,3 +228,33 @@ def inside_auto_empty_zone(name):
         if name.is_subdomain(aez):
             return True
     return False
+
+
+def related_to_auto_empty_zone(name):
+    """True if specified absolute name is a sub/superdomain of an automatic
+    empty zone.
+
+    DNS domain is a subdomain of itself so this function
+    returns True for zone apexes, too.
+
+    >>> related_to_auto_empty_zone(DNSName('.'))
+    True
+    >>> related_to_auto_empty_zone(DNSName('in-addr.arpa.'))
+    True
+    >>> related_to_auto_empty_zone(DNSName('10.in-addr.arpa.'))
+    True
+    >>> related_to_auto_empty_zone(DNSName('1.10.in-addr.arpa.'))
+    True
+    >>> related_to_auto_empty_zone(DNSName('unrelated.example.'))
+    False
+    >>> related_to_auto_empty_zone(DNSName('1.10.in-addr.arpa'))
+    Traceback (most recent call last):
+      ...
+    AssertionError: ...
+    """
+    assert_absolute_dnsname(name)
+    relations = {dns.name.NAMERELN_SUBDOMAIN,
+                 dns.name.NAMERELN_EQUAL,
+                 dns.name.NAMERELN_SUPERDOMAIN}
+    return any(name.fullcompare(aez)[0] in relations
+               for aez in EMPTY_ZONES)
-- 
2.5.5

From c8aa14c7fe446479c3c47ffb609e8dfaf9a7b372 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 27 Apr 2016 14:44:17 +0200
Subject: [PATCH] DNS upgrade: change forwarding policy to = only for
 conflicting forward zones

This change is necessary to override automatic empty zone configuration
in latest BIND and bind-dyndb-ldap 9.0+.

This procedure is still not complete because we need to handle global
forwarders too (in LDAP and in named.conf on each server).

https://fedorahosted.org/freeipa/ticket/5710
---
 install/share/dns.ldif                         |  4 +-
 install/updates/90-post_upgrade_plugins.update |  3 +
 ipaserver/install/plugins/dns.py               | 76 ++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index d71e2ad7d7ca530247198d9db71aebd497d0c121..bd5cc57f90ed66066699af06a74e1426cc8f9a59 100644
--- a/install/share/dns.ldif
+++ b/install/share/dns.ldif
@@ -2,10 +2,12 @@ dn: cn=dns,$SUFFIX
 changetype: add
 objectClass: idnsConfigObject
 objectClass: nsContainer
+objectClass: ipaConfigObject
 objectClass: ipaDNSContainer
 objectClass: top
 cn: dns
-ipaDNSVersion: 1
+ipaConfigString: DNSVersion 1
+ipaDNSVersion: 2
 aci: (targetattr = "*")(version 3.0; acl "Allow read access"; allow (read,search,compare) groupdn = "ldap:///cn=Read DNS Entries,cn=permissions,cn=pbac,$SUFFIX" or userattr = "parent[0,1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Add DNS entries in a zone";allow (add) userattr = "parent[1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Remove DNS entries from a zone";allow (delete) userattr = "parent[1].managedby#GROUPDN";)
diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index d8498edd4565fbad91ca5dea48e5bc2296e3fc25..ccb72efa4fa7af7fd0391b489d9c746332426fb9 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -17,7 +17,10 @@ plugin: update_service_principalalias
 plugin: update_upload_cacrt
 
 # last
+# DNS version 1
 plugin: update_master_to_dnsforwardzones
+# DNS version 2
+plugin: update_dnsforward_emptyzones
 plugin: update_managed_post
 plugin: update_managed_permissions
 plugin: update_read_replication_agreements_permission
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 6b540348c8a79751c12b60b7d08fc217e37a51cb..57e8ac8b21c1006d8fcde1ab7078dacf8260e0be 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -26,6 +26,7 @@ from ldif import LDIFWriter
 from ipalib import api, errors, util
 from ipalib import Updater
 from ipapython.dn import DN
+from ipapython import dnsutil
 from ipalib.plugins.dns import dns_container_exists
 from ipapython.ipa_log_manager import root_logger
 
@@ -394,3 +395,78 @@ class update_master_to_dnsforwardzones(DNSUpdater):
         return False, []
 
 api.register(update_master_to_dnsforwardzones)
+
+
+class update_dnsforward_emptyzones(DNSUpdater):
+    """
+    Migrate forward policies which conflict with automatic empty zones
+    (RFC 6303) to use forward policy = only.
+
+    BIND ignores conflicting forwarding configuration
+    when forwarding policy != only.
+    bind-dyndb-ldap 9.0+ will do the same so we have to adjust FreeIPA zones
+    accordingly.
+    """
+    backup_filename = u'dns-forwarding-empty-zones-%Y-%m-%d-%H-%M-%S.ldif'
+
+    def update_zones(self):
+        try:
+            fwzones = self.api.Command.dnsforwardzone_find(all=True,
+                                                           raw=True)['result']
+        except errors.NotFound:
+            # No forwardzones found, we are done
+            return
+
+        logged_once = False
+        for zone in fwzones:
+            if not (
+                dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
+                and zone.get('idnsforwardpolicy', [u'first'])[0] != u'only'
+                and zone.get('idnsforwarders', []) != []
+            ):
+                # this zone does not conflict with automatic empty zone
+                continue
+
+            if not logged_once:
+                self.log.info('Forward policy for zones conflicting with '
+                              'automatic empty zones will be changed to '
+                              '"only"')
+                logged_once = True
+
+            # backup
+            try:
+                self.backup_zone(zone)
+            except Exception:
+                self.log.error('Unable to create backup for zone %s, '
+                               'terminating zone upgrade', zone['idnsname'][0])
+                self.log.error(traceback.format_exc())
+                continue
+
+            # change forward policy
+            try:
+                self.api.Command['dnsforwardzone_mod'](
+                    zone['idnsname'][0],
+                    idnsforwardpolicy=u'only'
+                )
+            except Exception as e:
+                self.log.error('Forward policy update for zone %s failed '
+                               '(%s)' % (zone['idnsname'][0], e))
+                self.log.error(traceback.format_exc())
+                continue
+
+            self.log.debug('Zone %s was sucessfully modified to use '
+                           'forward policy "only"', zone['idnsname'][0])
+    def execute(self, **options):
+        # check LDAP if DNS subtree already uses new semantics
+        if not self.version_update_needed(target_version=2):
+            # forwardzones already use new semantics, no upgrade is required
+            return
+
+        self.log.debug('Updating forwarding policies to avoid conflicts '
+                       'with automatic empty zones')
+        # update the DNSVersion, following upgrade can be executed only once
+        self.api.Command['dnsconfig_mod'](ipadnsversion=2)
+
+        self.update_zones()
+
+api.register(update_dnsforward_emptyzones)
-- 
2.5.5

From 8cc3b44dd555156e06715a1e252e20eaac9936ab Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 27 Apr 2016 15:24:01 +0200
Subject: [PATCH] DNS upgrade: change global forwarding policy in LDAP to
 "only" if private IPs are used

This change is necessary to override automatic empty zone configuration
in latest BIND and bind-dyndb-ldap 9.0+.

This procedure is still not complete because we need to handle global
forwarders in named.conf too (independently on each server).

https://fedorahosted.org/freeipa/ticket/5710
---
 ipaserver/install/plugins/dns.py | 43 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 57e8ac8b21c1006d8fcde1ab7078dacf8260e0be..c669db1633e698fefc14a9f90ae54a0821a6318e 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -26,9 +26,11 @@ from ldif import LDIFWriter
 from ipalib import api, errors, util
 from ipalib import Updater
 from ipapython.dn import DN
+from ipapython.ipautil import CheckedIPAddress
 from ipapython import dnsutil
 from ipalib.plugins.dns import dns_container_exists
 from ipapython.ipa_log_manager import root_logger
+from ipaserver.install import installutils
 
 
 class DNSUpdater(Updater):
@@ -420,7 +422,8 @@ class update_dnsforward_emptyzones(DNSUpdater):
         logged_once = False
         for zone in fwzones:
             if not (
-                dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
+                dnsutil.related_to_auto_empty_zone(
+                    dnsutil.DNSName(zone.get('idnsname')[0]))
                 and zone.get('idnsforwardpolicy', [u'first'])[0] != u'only'
                 and zone.get('idnsforwarders', []) != []
             ):
@@ -456,17 +459,53 @@ class update_dnsforward_emptyzones(DNSUpdater):
 
             self.log.debug('Zone %s was sucessfully modified to use '
                            'forward policy "only"', zone['idnsname'][0])
+
+    def has_empty_zone_addresses(self):
+        """Detect if local server is using IP address belonging to
+        a automatic empty zone.
+
+        Information from --ip-address option used in installed is lost by
+        the time when upgrade is run. Use IP addresses from DNS as best
+        approximation.
+
+        This is brain-dead and duplicates logic from DNS installer
+        but I did not find other way around.
+        """
+        ip_addresses = installutils.resolve_host(self.api.env.host)
+        return any(
+            dnsutil.inside_auto_empty_zone(dnsutil.DNSName(
+                CheckedIPAddress(ip, parse_netmask=False).reverse_dns))
+            for ip in ip_addresses
+        )
+
+    def update_global_ldap_forwarder(self):
+        config = self.api.Command['dnsconfig_show'](all=True,
+                                                    raw=True)['result']
+        if (
+            config.get('idnsforwardpolicy', [u'first'])[0] == u'first'
+            and config.get('idnsforwarders', [])
+        ):
+            self.log.info('Global forward policy in LDAP for all servers will '
+                          'be changed to "only" to avoid conflicts with '
+                          'automatic empty zones')
+            self.backup_zone(config)
+            self.api.Command['dnsconfig_mod'](idnsforwardpolicy=u'only')
+
     def execute(self, **options):
         # check LDAP if DNS subtree already uses new semantics
         if not self.version_update_needed(target_version=2):
             # forwardzones already use new semantics, no upgrade is required
-            return
+            return False, []
 
         self.log.debug('Updating forwarding policies to avoid conflicts '
                        'with automatic empty zones')
         # update the DNSVersion, following upgrade can be executed only once
         self.api.Command['dnsconfig_mod'](ipadnsversion=2)
 
         self.update_zones()
+        if self.has_empty_zone_addresses():
+            self.update_global_ldap_forwarder()
+
+        return False, []
 
 api.register(update_dnsforward_emptyzones)
-- 
2.5.5

From 1d753965439dcdd1eddcbdc5d590d8f8376791e6 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Thu, 28 Apr 2016 22:19:03 +0200
Subject: [PATCH] DNS upgrade: change global forwarding policy in named.conf to
 "only" if private IPs are used

This change is necessary to override automatic empty zone configuration
in latest BIND and bind-dyndb-ldap 9.0+.

This upgrade has to be done on each IPA DNS server independently.

https://fedorahosted.org/freeipa/ticket/5710
---
 ipaserver/install/bindinstance.py |  7 ++++
 ipaserver/install/plugins/dns.py  | 68 +++++++++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 0cc8d2589be7ae835c807218b5c63ab3cb02a3a7..067e27a8e52859e1f5a22e0e27dc44cfab284a90 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -32,6 +32,7 @@ import six
 
 from ipaserver.install import installutils
 from ipaserver.install import service
+from ipaserver.install import sysupgrade
 from ipaserver.install.cainstance import IPA_CA_RECORD
 from ipapython import sysrestore, ipautil, ipaldap
 from ipapython.ipa_log_manager import root_logger
@@ -1033,6 +1034,12 @@ class BindInstance(service.Service):
                                      section=NAMED_SECTION_OPTIONS,
                                      str_val=False)
 
+        # prevent repeated upgrade on new installs
+        sysupgrade.set_upgrade_state(
+            'named.conf',
+            'forward_policy_conflict_with_empty_zones_handled', True
+        )
+
     def __setup_resolv_conf(self):
         if not self.fstore.has_file(RESOLV_CONF):
             self.fstore.backup_file(RESOLV_CONF)
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index c669db1633e698fefc14a9f90ae54a0821a6318e..19d8d45e37ada7ca5bccfbd45f1b64a3942ba951 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -18,19 +18,24 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
+import shutil
 import traceback
 import time
 
 from ldif import LDIFWriter
 
 from ipalib import api, errors, util
 from ipalib import Updater
+from ipaplatform.paths import paths
 from ipapython.dn import DN
+from ipapython.ipautil import CalledProcessError
 from ipapython.ipautil import CheckedIPAddress
 from ipapython import dnsutil
 from ipalib.plugins.dns import dns_container_exists
 from ipapython.ipa_log_manager import root_logger
+from ipaserver.install import bindinstance
 from ipaserver.install import installutils
+from ipaserver.install import sysupgrade
 
 
 class DNSUpdater(Updater):
@@ -42,6 +47,8 @@ class DNSUpdater(Updater):
         super(DNSUpdater, self).__init__(api)
         backup_path = u'%s%s' % (self.backup_dir, self.backup_filename)
         self.backup_path = time.strftime(backup_path)
+        self.backup_path_ldif = '%s.ldif' % self.backup_path
+        self.backup_path_conf = '%s.conf' % self.backup_path
         self.ldap = self.api.Backend.ldap2
         self._ldif_writer = None
         self._saved_privileges = set()  # store privileges only once
@@ -62,8 +69,8 @@ class DNSUpdater(Updater):
     def ldif_writer(self):
         if not self._ldif_writer:
             self.log.info('Original zones will be saved in LDIF format in '
-                          '%s file' % self.backup_path)
-            self._ldif_writer = LDIFWriter(open(self.backup_path, 'w'))
+                          '%s file' % self.backup_path_ldif)
+            self._ldif_writer = LDIFWriter(open(self.backup_path_ldif, 'w'))
         return self._ldif_writer
 
     def backup_zone(self, zone):
@@ -107,6 +114,11 @@ class DNSUpdater(Updater):
                 del record['dn']
                 self.ldif_writer.unparse(dn, record)
 
+    def backup_named_conf(self):
+        self.log.info('Original named.conf will be saved in %s file'
+                      % self.backup_path_conf)
+        shutil.copyfile(paths.NAMED_CONF, self.backup_path_conf)
+
 
 class update_ipaconfigstring_dnsversion_to_ipadnsversion(Updater):
     """
@@ -265,9 +277,10 @@ class update_master_to_dnsforwardzones(DNSUpdater):
     than none, will be tranformed to forward zones.
     Original masters zone will be backed up to ldif file.
 
-    This should be applied only once, and only if original version was lower than 4.0
+    This should be applied only once,
+    and only if original version was lower than 4.0
     """
-    backup_filename = u'dns-master-to-forward-zones-%Y-%m-%d-%H-%M-%S.ldif'
+    backup_filename = u'dns-master-to-forward-zones-%Y-%m-%d-%H-%M-%S'
 
     def execute(self, **options):
         ldap = self.api.Backend.ldap2
@@ -409,7 +422,7 @@ class update_dnsforward_emptyzones(DNSUpdater):
     bind-dyndb-ldap 9.0+ will do the same so we have to adjust FreeIPA zones
     accordingly.
     """
-    backup_filename = u'dns-forwarding-empty-zones-%Y-%m-%d-%H-%M-%S.ldif'
+    backup_filename = u'dns-forwarding-empty-zones-%Y-%m-%d-%H-%M-%S'
 
     def update_zones(self):
         try:
@@ -491,14 +504,55 @@ class update_dnsforward_emptyzones(DNSUpdater):
             self.backup_zone(config)
             self.api.Command['dnsconfig_mod'](idnsforwardpolicy=u'only')
 
+    def update_global_named_conf_forwarder(self, bind):
+        if bind.named_conf_get_directive(
+                'forward',
+                section=bindinstance.NAMED_SECTION_OPTIONS,
+                str_val=False
+        ) != 'only':
+            self.log.info('Global forward policy in named.conf will '
+                          'be changed to "only" to avoid conflicts with '
+                          'automatic empty zones')
+
+            self.backup_named_conf()
+            bindinstance.named_conf_set_directive(
+                'forward',
+                'only',
+                section=bindinstance.NAMED_SECTION_OPTIONS,
+                str_val=False
+            )
+            self.log.info('Changes to named.conf have been made, '
+                'restarting named')
+            try:
+                bind.restart()
+            except CalledProcessError as ex:
+                self.log.error("Failed to restart %s: %s",
+                    bind.service_name, ex)
+
+
     def execute(self, **options):
+        # upgrade forward policy in local named.conf only once
+        bind = bindinstance.BindInstance(api=self.api)
+        if bind.is_configured() and not sysupgrade.get_upgrade_state(
+            'named.conf', 'forward_policy_conflict_with_empty_zones_handled'
+        ):
+            self.log.debug('Checking global forwarding policy in named.conf '
+                           'to avoid conflicts with automatic empty zones')
+            sysupgrade.set_upgrade_state(
+                'named.conf',
+                'forward_policy_conflict_with_empty_zones_handled',
+                True
+            )
+            if self.has_empty_zone_addresses():
+                self.update_global_named_conf_forwarder(bind)
+
         # check LDAP if DNS subtree already uses new semantics
         if not self.version_update_needed(target_version=2):
             # forwardzones already use new semantics, no upgrade is required
             return False, []
 
-        self.log.debug('Updating forwarding policies to avoid conflicts '
-                       'with automatic empty zones')
+        self.log.debug('Updating forwarding policies in LDAP '
+                       'to avoid conflicts with automatic empty zones')
         # update the DNSVersion, following upgrade can be executed only once
         self.api.Command['dnsconfig_mod'](ipadnsversion=2)
 
-- 
2.5.5

-- 
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

Reply via email to