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