URL: https://github.com/freeipa/freeipa/pull/428 Author: MartinBasti Title: #428: [Py3] ipa-server-install Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/428/head:pr428 git checkout pr428
From 4d367dfa6e02858132f62e5695714939310c1637 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 27 Jan 2017 17:00:42 +0100 Subject: [PATCH 01/11] py3: modify_s: attribute name must be str not bytes https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/ipaldap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 37d23d7..3df82b3 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -727,7 +727,7 @@ def modify_s(self, dn, modlist): # FIXME: for backwards compatibility only assert isinstance(dn, DN) dn = str(dn) - modlist = [(a, self.encode(b), self.encode(c)) for a, b, c in modlist] + modlist = [(a, b, self.encode(c)) for a, b, c in modlist] return self.conn.modify_s(dn, modlist) @property From 9a045ddb67760192766923cd5a597227d5a4edda Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 27 Jan 2017 17:33:44 +0100 Subject: [PATCH 02/11] py3: configparser: use raw keyword configparser.get() changed in python3 and `raw` is now a keyword attribute. Also it must be set to True, otherwise InterpolationSyntaxError is raised ''' InterpolationSyntaxError: '%' must be followed by '%' or '(', found: '%2fvar%2frun%2fslapd-EXAMPLE-COM.socket' ''' https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/secrets/kem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/secrets/kem.py b/ipaserver/secrets/kem.py index 143caaf..3577975 100644 --- a/ipaserver/secrets/kem.py +++ b/ipaserver/secrets/kem.py @@ -181,7 +181,7 @@ def __init__(self, config=None, ipaconf=paths.IPA_DEFAULT_CONF): self.realm = conf.get('global', 'realm') self.ldap_uri = config.get('ldap_uri', None) if self.ldap_uri is None: - self.ldap_uri = conf.get('global', 'ldap_uri', None) + self.ldap_uri = conf.get('global', 'ldap_uri', raw=True) self._server_keys = None def find_key(self, kid, usage): From 7dcd8723ef638c23b7d22a0f36cf242cc8aef71c Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 27 Jan 2017 18:10:37 +0100 Subject: [PATCH 03/11] py3: custodia: basedn must be unicode basedn in custodia related modules has type bytes, that causes issues in Py3 when strings were concatenated with bytes ``` malformed RDN string = "cn=custodia,cn=ipa,cn=etc,b'dc=example,dc=com'" ``` https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/secrets/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/secrets/common.py b/ipaserver/secrets/common.py index 2b906b6..f3dc320 100644 --- a/ipaserver/secrets/common.py +++ b/ipaserver/secrets/common.py @@ -23,7 +23,7 @@ def basedn(self): if self._basedn is None: conn = self.connect() r = conn.search_s('', ldap.SCOPE_BASE) - self._basedn = r[0][1]['defaultnamingcontext'][0] + self._basedn = r[0][1]['defaultnamingcontext'][0].decode('utf-8') return self._basedn def connect(self): From b07246ffe79a5dd00a9ab02c7f6ae23260e4cecd Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 18:11:42 +0100 Subject: [PATCH 04/11] py3: kem.py: user bytes with ldap values python ldap requires bytes as values https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/secrets/kem.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ipaserver/secrets/kem.py b/ipaserver/secrets/kem.py index 3577975..5d784b7 100644 --- a/ipaserver/secrets/kem.py +++ b/ipaserver/secrets/kem.py @@ -130,13 +130,13 @@ def set_key(self, usage, principal, key): service_rdn = ('cn', servicename) if servicename != 'host' else DN() dn = str(DN(('cn', name), service_rdn, self.keysbase)) try: - mods = [('objectClass', ['nsContainer', - 'ipaKeyPolicy', - 'ipaPublicKeyObject', - 'groupOfPrincipals']), - ('cn', name), - ('ipaKeyUsage', RFC5280_USAGE_MAP[usage]), - ('memberPrincipal', principal), + mods = [('objectClass', [b'nsContainer', + b'ipaKeyPolicy', + b'ipaPublicKeyObject', + b'groupOfPrincipals']), + ('cn', name.encode('utf-8')), + ('ipaKeyUsage', RFC5280_USAGE_MAP[usage].encode('utf-8')), + ('memberPrincipal', principal.encode('utf-8')), ('ipaPublicKey', public_key)] conn.add_s(dn, mods) except Exception: # pylint: disable=broad-except From f3185832501737acfc3ed349760dc48680def8c4 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 18:14:33 +0100 Subject: [PATCH 05/11] custodia: kem.set_keys: replace too-broad exception Exception is too brod and may hide various issues that show up later. If the code expects that entry may exist, then ldap.ALREADY_EXISTS exception should be used --- ipaserver/secrets/kem.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ipaserver/secrets/kem.py b/ipaserver/secrets/kem.py index 5d784b7..28fb4d3 100644 --- a/ipaserver/secrets/kem.py +++ b/ipaserver/secrets/kem.py @@ -139,8 +139,7 @@ def set_key(self, usage, principal, key): ('memberPrincipal', principal.encode('utf-8')), ('ipaPublicKey', public_key)] conn.add_s(dn, mods) - except Exception: # pylint: disable=broad-except - # This may fail if the entry already exists + except ldap.ALREADY_EXISTS: mods = [(ldap.MOD_REPLACE, 'ipaPublicKey', public_key)] conn.modify_s(dn, mods) From a651d0f423a21cce11e5f611208f54aa34d4176d Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 18:36:24 +0100 Subject: [PATCH 06/11] py3: upgradeinstance: open dse.ldif in textual mode ldap ldif parser requires to have input file opened in textual mode https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/upgradeinstance.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py index 8e28436..b244898 100644 --- a/ipaserver/install/upgradeinstance.py +++ b/ipaserver/install/upgradeinstance.py @@ -124,7 +124,7 @@ def create_instance(self): def __save_config(self): shutil.copy2(self.filename, self.savefilename) - with open(self.filename, "rb") as in_file: + with open(self.filename, "r") as in_file: parser = GetEntryFromLDIF(in_file, entries_dn=["cn=config"]) parser.parse() try: @@ -156,8 +156,8 @@ def __save_config(self): def __enable_ds_global_write_lock(self): ldif_outfile = "%s.modified.out" % self.filename - with open(ldif_outfile, "wb") as out_file: - with open(self.filename, "rb") as in_file: + with open(ldif_outfile, "w") as out_file: + with open(self.filename, "r") as in_file: parser = installutils.ModifyLDIF(in_file, out_file) parser.replace_value( @@ -172,8 +172,8 @@ def __restore_config(self): global_lock = self.restore_state('nsslapd-global-backend-lock') ldif_outfile = "%s.modified.out" % self.filename - with open(ldif_outfile, "wb") as out_file: - with open(self.filename, "rb") as in_file: + with open(ldif_outfile, "w") as out_file: + with open(self.filename, "r") as in_file: parser = installutils.ModifyLDIF(in_file, out_file) if port is not None: @@ -194,8 +194,8 @@ def __restore_config(self): def __disable_listeners(self): ldif_outfile = "%s.modified.out" % self.filename - with open(ldif_outfile, "wb") as out_file: - with open(self.filename, "rb") as in_file: + with open(ldif_outfile, "w") as out_file: + with open(self.filename, "r") as in_file: parser = installutils.ModifyLDIF(in_file, out_file) parser.replace_value("cn=config", "nsslapd-port", ["0"]) parser.replace_value("cn=config", "nsslapd-security", ["off"]) From 970d893c0fefee32cf61a67170789789def63400 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 18:53:47 +0100 Subject: [PATCH 07/11] py3: upgradeinstance: decode data before storing them as backup... ...and vice versa backup requires string not bytes, but ldap provide bytes thus data must be decoded and encoded from restore https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/upgradeinstance.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py index b244898..8e6c87d 100644 --- a/ipaserver/install/upgradeinstance.py +++ b/ipaserver/install/upgradeinstance.py @@ -134,21 +134,22 @@ def __save_config(self): self.filename) try: - port = config_entry['nsslapd-port'][0] + port = config_entry['nsslapd-port'][0].decode('utf-8') except KeyError: pass else: self.backup_state('nsslapd-port', port) try: - security = config_entry['nsslapd-security'][0] + security = config_entry['nsslapd-security'][0].decode('utf-8') except KeyError: pass else: self.backup_state('nsslapd-security', security) try: - global_lock = config_entry['nsslapd-global-backend-lock'][0] + global_lock = config_entry[ + 'nsslapd-global-backend-lock'][0].decode('utf-8') except KeyError: pass else: @@ -177,16 +178,17 @@ def __restore_config(self): parser = installutils.ModifyLDIF(in_file, out_file) if port is not None: - parser.replace_value("cn=config", "nsslapd-port", [port]) + parser.replace_value( + "cn=config", "nsslapd-port", [port.encode('utf-8')]) if security is not None: parser.replace_value("cn=config", "nsslapd-security", - [security]) + [security.encode('utf-8')]) # disable global lock by default parser.remove_value("cn=config", "nsslapd-global-backend-lock") if global_lock is not None: parser.add_value("cn=config", "nsslapd-global-backend-lock", - [global_lock]) + [global_lock.encode('utf-8')]) parser.parse() From 41e271789e4a93db4f5235b17c9c85702b8406ac Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 19:23:39 +0100 Subject: [PATCH 08/11] py3: upgradeinstance: use bytes literals with LDIF operations python ldif support only bytes as values, literals must be bytes https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/upgradeinstance.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py index 8e6c87d..fca4226 100644 --- a/ipaserver/install/upgradeinstance.py +++ b/ipaserver/install/upgradeinstance.py @@ -162,7 +162,7 @@ def __enable_ds_global_write_lock(self): parser = installutils.ModifyLDIF(in_file, out_file) parser.replace_value( - "cn=config", "nsslapd-global-backend-lock", ["on"]) + "cn=config", "nsslapd-global-backend-lock", [b"on"]) parser.parse() shutil.copy2(ldif_outfile, self.filename) @@ -199,8 +199,8 @@ def __disable_listeners(self): with open(ldif_outfile, "w") as out_file: with open(self.filename, "r") as in_file: parser = installutils.ModifyLDIF(in_file, out_file) - parser.replace_value("cn=config", "nsslapd-port", ["0"]) - parser.replace_value("cn=config", "nsslapd-security", ["off"]) + parser.replace_value("cn=config", "nsslapd-port", [b"0"]) + parser.replace_value("cn=config", "nsslapd-security", [b"off"]) parser.remove_value("cn=config", "nsslapd-ldapientrysearchbase") parser.parse() From 56d35089313f3a1a56e65eb0757c49b7b918495d Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 19:59:31 +0100 Subject: [PATCH 09/11] py3: create DNS zonefile: use textual mode Also code was rewritten to use NamedTemporaryFile with context https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/bindinstance.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index e24249a..10a37b9 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -668,10 +668,13 @@ def create_file_with_system_records(self): system_records.get_base_records() ) ) - [fd, name] = tempfile.mkstemp(".db","ipa.system.records.") - os.write(fd, text) - os.close(fd) - print("Please add records in this file to your DNS system:", name) + with tempfile.NamedTemporaryFile( + mode="w", prefix="ipa.system.records.", + suffix=".db", delete=False + ) as f: + f.write(text) + print("Please add records in this file to your DNS system:", + f.name) def create_instance(self): From 6401eb82de00b65476d674240ac2c954484d8d4a Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 31 Jan 2017 20:27:11 +0100 Subject: [PATCH 10/11] py3: change_admin_password: use textual mode Convert function to NamedTemporaryFile with textual mode, because passwords are text. Using `with` and NamedTemporaryFile gives more security agains leaking password from tempfiles. https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/dsinstance.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index ceb7bf3..8e979a7 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -951,21 +951,19 @@ def add_hbac(self): def change_admin_password(self, password): root_logger.debug("Changing admin password") - dmpwdfile = "" - admpwdfile = "" - try: - (dmpwdfd, dmpwdfile) = tempfile.mkstemp(dir=paths.VAR_LIB_IPA) - os.write(dmpwdfd, self.dm_password) - os.close(dmpwdfd) + dir_ipa = paths.VAR_LIB_IPA + with tempfile.NamedTemporaryFile("w", dir=dir_ipa) as dmpwdfile, \ + tempfile.NamedTemporaryFile("w", dir=dir_ipa) as admpwdfile: + dmpwdfile.write(self.dm_password) + dmpwdfile.flush() - (admpwdfd, admpwdfile) = tempfile.mkstemp(dir=paths.VAR_LIB_IPA) - os.write(admpwdfd, password) - os.close(admpwdfd) + admpwdfile.write(password) + admpwdfile.flush() args = [paths.LDAPPASSWD, "-h", self.fqdn, "-ZZ", "-x", "-D", str(DN(('cn', 'Directory Manager'))), - "-y", dmpwdfile, "-T", admpwdfile, + "-y", dmpwdfile.name, "-T", admpwdfile.name, str(DN(('uid', 'admin'), ('cn', 'users'), ('cn', 'accounts'), self.suffix))] try: env = {'LDAPTLS_CACERTDIR': os.path.dirname(paths.IPA_CA_CRT), @@ -976,12 +974,6 @@ def change_admin_password(self, password): print("Unable to set admin password", e) root_logger.debug("Unable to set admin password %s" % e) - finally: - if os.path.isfile(dmpwdfile): - os.remove(dmpwdfile) - if os.path.isfile(admpwdfile): - os.remove(admpwdfile) - def uninstall(self): if self.is_configured(): self.print_msg("Unconfiguring directory server") From 52db988d87a3d2c866f7a01bbac73ca8591fb988 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 7 Feb 2017 19:24:46 +0100 Subject: [PATCH 11/11] py3: ipa_generate_password: do not compare None and Int The one cannot compare None and Int in Py3 """ unorderable types: NoneType() > int() """ Continue when class is disabled with None value https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/ipautil.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index c810adc..60b4a37 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -860,6 +860,8 @@ def ipa_generate_password(entropy_bits=256, uppercase=1, lowercase=1, digits=1, for charclass_name in ['digits', 'uppercase', 'lowercase', 'special']: charclass = pwd_charsets[charclass_name] todo_characters = req_classes[charclass_name] + if todo_characters is None: + continue while todo_characters > 0: password += rnd.choice(charclass['chars']) todo_entropy -= charclass['entropy']
-- 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
