URL: https://github.com/freeipa/freeipa/pull/460
Author: MartinBasti
 Title: #460: ipa-server-install, ipa-server-upgrade fixes
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/460/head:pr460
git checkout pr460
From 69369de4b67ca1a0e0286253ffe1cd42c853a0cd Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 10 Feb 2017 17:05:02 +0100
Subject: [PATCH 1/9] py3: use ConfigParser instead of SafeConfigParser

DeprecationWarning: The SafeConfigParser class has been renamed
to ConfigParser in Python 3.2. This alias will be removed in
future versions. Use ConfigParser directly instead.

https://fedorahosted.org/freeipa/ticket/4985
---
 ipalib/install/sysrestore.py             |  6 +++++-
 ipaserver/install/installutils.py        |  7 ++++++-
 ipaserver/install/ipa_backup.py          |  7 ++++++-
 ipaserver/install/ipa_replica_prepare.py |  7 ++++++-
 ipaserver/install/ipa_restore.py         | 11 ++++++++++-
 ipaserver/install/server/upgrade.py      |  6 +++++-
 6 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/ipalib/install/sysrestore.py b/ipalib/install/sysrestore.py
index b1bf4b9127..5c21956898 100644
--- a/ipalib/install/sysrestore.py
+++ b/ipalib/install/sysrestore.py
@@ -31,7 +31,11 @@
 
 import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
 # pylint: enable=import-error
 
 from ipaplatform.tasks import tasks
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index d2283af204..15ab65974e 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -41,7 +41,12 @@
 import ldapurl
 import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser, NoOptionError
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
+from six.moves.configparser import NoOptionError
 # pylint: enable=import-error
 
 from ipalib.install import sysrestore
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index f8cdd56d26..f17de1e6f2 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -23,8 +23,13 @@
 import time
 import pwd
 
+import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
 # pylint: enable=import-error
 
 from ipaplatform.paths import paths
diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index d4456dd796..fcb652859c 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -30,8 +30,13 @@
 # pylint: enable=deprecated-module
 
 import dns.resolver
+import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
 # pylint: enable=import-error
 
 from ipaserver.install import certs, installutils, bindinstance, dsinstance, ca
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index f786c746bb..ea308dba4e 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -25,8 +25,13 @@
 import ldif
 import itertools
 
+import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
 # pylint: enable=import-error
 
 from ipaclient.install.client import update_ipa_nssdb
@@ -715,7 +720,11 @@ def read_header(self):
         self.backup_host = config.get('ipa', 'host')
         self.backup_ipa_version = config.get('ipa', 'ipa_version')
         self.backup_version = config.get('ipa', 'version')
+        # pylint: disable=no-member
+        # we can assume that returned object is string and it has .split()
+        # method
         self.backup_services = config.get('ipa', 'services').split(',')
+        # pylint: enable=no-member
 
 
     def extract_backup(self, keyring=None):
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index db86353165..e8ceafa310 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -16,7 +16,11 @@
 
 import six
 # pylint: disable=import-error
-from six.moves.configparser import SafeConfigParser
+if six.PY3:
+    # The SafeConfigParser class has been renamed to ConfigParser in Py3
+    from configparser import ConfigParser as SafeConfigParser
+else:
+    from ConfigParser import SafeConfigParser
 # pylint: enable=import-error
 
 from ipalib import api

From f021e470fd34c2bca3479818530bb21de587d817 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 30 May 2017 13:41:56 +0200
Subject: [PATCH 2/9] py3: ConfigParser: replace deprecated readfd with read

ConfigParser.readfd() is deprecated in py3, we can use .read() which is
compatible with py2

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/installutils.py | 5 ++---
 ipaserver/install/ipa_restore.py  | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 15ab65974e..04bf4fcd20 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -749,10 +749,9 @@ def read_replica_info(dir_path, rconfig):
 
     rconfig is a ReplicaConfig object
     """
-    filename = dir_path + "/realm_info"
-    fd = open(filename)
+    filename = os.path.join(dir_path, "realm_info")
     config = SafeConfigParser()
-    config.readfp(fd)
+    config.read(filename)
 
     rconfig.realm_name = config.get("realm", "realm_name")
     rconfig.master_host_name = config.get("realm", "master_host_name")
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index ea308dba4e..d85c4874d8 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -711,9 +711,8 @@ def read_header(self):
         Read the backup file header that contains the meta data about
         this particular backup.
         '''
-        with open(self.header) as fd:
-            config = SafeConfigParser()
-            config.readfp(fd)
+        config = SafeConfigParser()
+        config.read(self.header)
 
         self.backup_type = config.get('ipa', 'type')
         self.backup_time = config.get('ipa', 'time')

From 4ef0ebf134e793914c2689afa50f105f0db4582d Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 10 Feb 2017 17:13:15 +0100
Subject: [PATCH 3/9] py3: ipaldap: encode Boolean as bytes

Python LDAP requires bytes

https://fedorahosted.org/freeipa/ticket/4985
---
 ipapython/ipaldap.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 1b0aaddd63..9a42312f05 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -847,9 +847,9 @@ def encode(self, val):
         # entered for a boolean value instead of the boolean clause.
         if isinstance(val, bool):
             if val:
-                return 'TRUE'
+                return b'TRUE'
             else:
-                return 'FALSE'
+                return b'FALSE'
         elif isinstance(val, (unicode, six.integer_types, Decimal, DN,
                               Principal)):
             return six.text_type(val).encode('utf-8')

From e8d1d1be3320fc28fa25bfa99b5253511cd44209 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 10 Feb 2017 17:36:19 +0100
Subject: [PATCH 4/9] py3: softhsm key_id must be bytes

softhsm works with bytes, so key_id must be byte otherwise we get errors
from bytes and string comparison

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/dnskeysyncinstance.py |  5 +----
 ipaserver/p11helper.py                  | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index 3849626e5a..f43f90cd9e 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -7,7 +7,6 @@
 import os
 import pwd
 import grp
-import random
 import shutil
 import stat
 
@@ -282,9 +281,7 @@ def __setup_replica_keys(self):
             key_id = None
             while True:
                 # check if key with this ID exist in softHSM
-                # id is 16 Bytes long
-                key_id = "".join(chr(random.randint(0, 255))
-                                 for _ in range(0, 16))
+                key_id = _ipap11helper.gen_key_id()
                 replica_pubkey_dn = DN(('ipk11UniqueId', 'autogenerate'), dn_base)
 
 
diff --git a/ipaserver/p11helper.py b/ipaserver/p11helper.py
index 37abf72792..f0c2540752 100644
--- a/ipaserver/p11helper.py
+++ b/ipaserver/p11helper.py
@@ -5,6 +5,7 @@
 import random
 import ctypes.util
 import binascii
+import struct
 
 import six
 from cryptography.hazmat.backends import default_backend
@@ -1824,6 +1825,18 @@ def get_attribute(self, key_object, attr):
 MECH_AES_KEY_WRAP_PAD = CKM_AES_KEY_WRAP_PAD
 
 
+def gen_key_id(key_id_len=16):
+    """
+    Generate random softhsm KEY_ID
+    :param key_id_len: this should be 16
+    :return: random softhsm KEY_ID in bytes representation
+    """
+    return struct.pack(
+        "B" * key_id_len,  # key_id must be bytes
+        *(random.randint(0, 255) for _ in range(key_id_len))
+    )
+
+
 def generate_master_key(p11, keylabel=u"dnssec-master", key_length=16,
                         disable_old_keys=True):
     assert isinstance(p11, P11_Helper)
@@ -1832,7 +1845,7 @@ def generate_master_key(p11, keylabel=u"dnssec-master", key_length=16,
     while True:
         # check if key with this ID exist in LDAP or softHSM
         # id is 16 Bytes long
-        key_id = "".join(chr(random.randint(0, 255)) for _ in range(0, 16))
+        key_id = gen_key_id()
         keys = p11.find_keys(KEY_CLASS_SECRET_KEY,
                              label=keylabel,
                              id=key_id)

From 17d101d4c2d435055f47cf6037bb42e440659e7a Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 10 Feb 2017 19:01:12 +0100
Subject: [PATCH 5/9] py3: LDAP updates: use only bytes/raw values

Functions mix unicode and bytes, use only bytes.

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/ldapupdate.py   | 29 ++++++++++++++++++++---------
 ipaserver/install/schemaupdate.py |  3 ++-
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index c6ab3e2afc..f52328fcaa 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -609,14 +609,15 @@ def _create_default_entry(self, dn, default):
             attr = item['attr']
             value = item['value']
 
-            e = entry.get(attr)
+            e = entry.raw.get(attr)
             if e:
                 # multi-valued attribute
                 e = list(e)
                 e.append(value)
             else:
                 e = [value]
-            entry[attr] = e
+
+            entry.raw[attr] = e
         entry.reset_modlist()
 
         return entry
@@ -650,6 +651,16 @@ def _apply_update_disposition(self, updates, entry):
             attr = update['attr']
             update_value = update['value']
 
+            # do not mix comparison of bytes and unicode, everything in this
+            # function should be compared as bytes
+            if isinstance(update_value, (list, tuple)):
+                update_value = [
+                    v.encode('utf-8') if isinstance(v, unicode) else v
+                    for v in update_value
+                ]
+            elif isinstance(update_value, unicode):
+                update_value = update_value.encode('utf-8')
+
             entry_values = entry.raw.get(attr, [])
             if action == 'remove':
                 self.debug("remove: '%s' from %s, current value %s", safe_output(attr, update_value), attr, safe_output(attr,entry_values))
@@ -660,7 +671,7 @@ def _apply_update_disposition(self, updates, entry):
                         "remove: '%s' not in %s",
                         safe_output(attr, update_value), attr)
                 else:
-                    entry[attr] = entry_values
+                    entry.raw[attr] = entry_values
                     self.debug('remove: updated value %s', safe_output(
                         attr, entry_values))
             elif action == 'add':
@@ -672,7 +683,7 @@ def _apply_update_disposition(self, updates, entry):
                     pass
                 entry_values.append(update_value)
                 self.debug('add: updated value %s', safe_output(attr, entry_values))
-                entry[attr] = entry_values
+                entry.raw[attr] = entry_values
             elif action == 'addifnew':
                 self.debug("addifnew: '%s' to %s, current value %s", safe_output(attr, update_value), attr, safe_output(attr, entry_values))
                 # Only add the attribute if it doesn't exist. Only works
@@ -680,7 +691,7 @@ def _apply_update_disposition(self, updates, entry):
                 if entry.get('objectclass') and len(entry_values) == 0:
                     entry_values.append(update_value)
                     self.debug('addifnew: set %s to %s', attr, safe_output(attr, entry_values))
-                    entry[attr] = entry_values
+                    entry.raw[attr] = entry_values
             elif action == 'addifexist':
                 self.debug("addifexist: '%s' to %s, current value %s", safe_output(attr, update_value), attr, safe_output(attr, entry_values))
                 # Only add the attribute if the entry doesn't exist. We
@@ -688,7 +699,7 @@ def _apply_update_disposition(self, updates, entry):
                 if entry.get('objectclass'):
                     entry_values.append(update_value)
                     self.debug('addifexist: set %s to %s', attr, safe_output(attr, entry_values))
-                    entry[attr] = entry_values
+                    entry.raw[attr] = entry_values
             elif action == 'only':
                 self.debug("only: set %s to '%s', current value %s", attr, safe_output(attr, update_value), safe_output(attr, entry_values))
                 if only.get(attr):
@@ -696,7 +707,7 @@ def _apply_update_disposition(self, updates, entry):
                 else:
                     entry_values = [update_value]
                     only[attr] = True
-                entry[attr] = entry_values
+                entry.raw[attr] = entry_values
                 self.debug('only: updated value %s', safe_output(attr, entry_values))
             elif action == 'onlyifexist':
                 self.debug("onlyifexist: '%s' to %s, current value %s", safe_output(attr, update_value), attr, safe_output(attr, entry_values))
@@ -709,7 +720,7 @@ def _apply_update_disposition(self, updates, entry):
                         entry_values = [update_value]
                         only[attr] = True
                     self.debug('onlyifexist: set %s to %s', attr, safe_output(attr, entry_values))
-                    entry[attr] = entry_values
+                    entry.raw[attr] = entry_values
             elif action == 'deleteentry':
                 # skip this update type, it occurs in  __delete_entries()
                 return None
@@ -724,7 +735,7 @@ def _apply_update_disposition(self, updates, entry):
                 else:
                     entry_values.append(new)
                     self.debug('replace: updated value %s', safe_output(attr, entry_values))
-                    entry[attr] = entry_values
+                    entry.raw[attr] = entry_values
 
         return entry
 
diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index 468a83477b..59b65f580c 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -143,7 +143,6 @@ def update_schema(schema_files, ldapi=False, dm_password=None,):
                         # Note: An add will automatically replace any existing
                         # schema with the same OID. So, we only add.
                         value = add_x_origin(new_obj)
-                        new_elements.append(value)
 
                         if old_obj:
                             old_attr = old_entries_by_oid.get(oid)
@@ -152,6 +151,8 @@ def update_schema(schema_files, ldapi=False, dm_password=None,):
                         else:
                             log.debug('Add: %s', value)
 
+                        new_elements.append(value.encode('utf-8'))
+
                 modified = modified or new_elements
                 schema_entry[attrname].extend(new_elements)
                 # we need to iterate schema updates, due to dependencies (SUP)

From f44231ca19b8f6eda501f12ca5de13ad08a4981e Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 10 Feb 2017 19:02:35 +0100
Subject: [PATCH 6/9] py3: schemaupdate: fix BytesWarning

str() was called on bytes

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/schemaupdate.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index 59b65f580c..cecde30443 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -119,7 +119,7 @@ def update_schema(schema_files, ldapi=False, dm_password=None,):
 
     # The exact representation the DS gives us for each OID
     # (for debug logging)
-    old_entries_by_oid = {cls(str(attr)).oid: str(attr)
+    old_entries_by_oid = {cls(attr.decode('utf-8')).oid: attr.decode('utf-8')
                           for (attrname, cls) in SCHEMA_ELEMENT_CLASSES
                           for attr in schema_entry[attrname]}
 

From 4ec96c917587d9319c7ca9aa449595a1cf75d7c3 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 13 Feb 2017 17:14:51 +0100
Subject: [PATCH 7/9] py3: cainstance: fix BytesWarning

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/cainstance.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 0b313691c3..28a702dba5 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1638,11 +1638,16 @@ def repair_profile_caIPAserviceCert():
             return
 
     indicators = [
-        "policyset.serverCertSet.1.default.params.name="
-            "CN=$request.req_subject_name.cn$, OU=pki-ipa, O=IPA ",
-        "policyset.serverCertSet.9.default.params.crlDistPointsPointName_0="
-            "https://ipa.example.com/ipa/crl/MasterCRL.bin";,
-        ]
+        (
+            b"policyset.serverCertSet.1.default.params.name="
+            b"CN=$request.req_subject_name.cn$, OU=pki-ipa, O=IPA "
+        ),
+        (
+            b"policyset.serverCertSet.9.default.params."
+            b"crlDistPointsPointName_0="
+            b"https://ipa.example.com/ipa/crl/MasterCRL.bin";
+        ),
+    ]
     need_repair = all(l in cur_config for l in indicators)
 
     if need_repair:

From eb382de7b9327bf801b546fcb6fb90802147417c Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 13 Feb 2017 17:33:21 +0100
Subject: [PATCH 8/9] py3: urlfetch: use "file://" prefix with filenames

with py3 urlopen used internally with pyldap doesn't work with raw
filepaths without specifying "file://" prefix. This works on both
py2/py3

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/schemaupdate.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index cecde30443..935e576454 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -125,7 +125,8 @@ def update_schema(schema_files, ldapi=False, dm_password=None,):
 
     for filename in schema_files:
         log.debug('Processing schema LDIF file %s', filename)
-        _dn, new_schema = ldap.schema.subentry.urlfetch(filename)
+        url = "file://{}".format(filename)
+        _dn, new_schema = ldap.schema.subentry.urlfetch(url)
 
         for attrname, cls in SCHEMA_ELEMENT_CLASSES:
             for oids_set in _get_oid_dependency_order(new_schema, cls):

From 6c9c5b8e62375e2e73f89824e0c8317fa971ecab Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 13 Feb 2017 17:34:19 +0100
Subject: [PATCH 9/9] py3: update_mod_nss_cipher_suite: ordering doesn't work
 with None

Py3 doesn't support ordering with None value

https://fedorahosted.org/freeipa/ticket/4985
---
 ipaserver/install/server/upgrade.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index e8ceafa310..3e2abefc21 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1438,7 +1438,7 @@ def update_mod_nss_cipher_suite(http):
     root_logger.info('[Updating mod_nss cipher suite]')
 
     revision = sysupgrade.get_upgrade_state('nss.conf', 'cipher_suite_updated')
-    if revision >= httpinstance.NSS_CIPHER_REVISION:
+    if revision and revision >= httpinstance.NSS_CIPHER_REVISION:
         root_logger.debug("Cipher suite already updated")
         return
 
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to