URL: https://github.com/freeipa/freeipa/pull/382 Author: mbasti-rh Title: #382: [WIP] Py3 ipa-server-install fixes Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/382/head:pr382 git checkout pr382
From 0ba8877d4f0a6e96d4e338a88f8638d00ad980b1 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 9 Jan 2017 11:53:59 +0100 Subject: [PATCH 01/10] py3: create_cert_db: write to file in a compatible way Py3 expect bytes to be writed using os.write. Instead of that using io module is more pythonic. https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/httpinstance.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py index bacd5fc..ded0553 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -19,6 +19,7 @@ from __future__ import print_function +import io import os import os.path import pwd @@ -314,9 +315,8 @@ def create_cert_db(self): # Create the password file for this db password = ipautil.ipa_generate_password() - f = os.open(pwd_file, os.O_CREAT | os.O_RDWR) - os.write(f, password) - os.close(f) + with io.open(pwd_file, 'w') as f: + f.write(password) ipautil.run([paths.CERTUTIL, "-d", database, "-f", pwd_file, "-N"]) From f97b56d35539dbc091d20282e01b7a804c6f8732 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 10 Jan 2017 13:45:11 +0100 Subject: [PATCH 02/10] py3: service.py: replace mkstemp by NamedTemporaryFile NamedTemporaryfile can be used in more pythonic way and file can be opened in textual mode that is required with PY3 https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/ipautil.py | 2 +- ipaserver/install/service.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index e3e4611..34d10ef 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -852,7 +852,7 @@ def ipa_generate_password(entropy_bits=256, uppercase=1, lowercase=1, digits=1, rnd = random.SystemRandom() todo_entropy = entropy_bits - password = '' + password = u'' # Generate required character classes: # The order of generated characters is fixed to comply with check in # NSS function sftk_newPinCheck() in nss/lib/softoken/fipstokn.c. diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py index 6451f92..c96cd8b 100644 --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py @@ -208,10 +208,11 @@ def _ldap_mod(self, ldif, sub_dict=None, raise_on_err=True, args += ["-H", ldap_uri] if dm_password: - [pw_fd, pw_name] = tempfile.mkstemp() - os.write(pw_fd, dm_password) - os.close(pw_fd) - auth_parms = ["-x", "-D", "cn=Directory Manager", "-y", pw_name] + with tempfile.NamedTemporaryFile( + mode='w', delete=False) as pw_file: + pw_name = pw_file.name + pw_file.write(dm_password) + auth_parms = ["-x", "-D", "cn=Directory Manager", "-y", pw_name] # Use GSSAPI auth when not using DM password or not being root elif os.getegid() != 0: auth_parms = ["-Y", "GSSAPI"] From 381a6570f75a581d01e8ed57d701654ae36d388e Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 9 Jan 2017 12:42:23 +0100 Subject: [PATCH 03/10] py3: open temporary ldif file in text mode ldif parser uses file in text mode, so we have to open it in text mode in py3 Also values passed to parser should be bytes https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/dsinstance.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index 89315b6..2721d88 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -582,14 +582,15 @@ def __update_dse_ldif(self): 'dse.ldif' ) - with tempfile.NamedTemporaryFile(delete=False) as new_dse_ldif: + with tempfile.NamedTemporaryFile( + mode='w', delete=False) as new_dse_ldif: temp_filename = new_dse_ldif.name with open(dse_filename, "r") as input_file: parser = installutils.ModifyLDIF(input_file, new_dse_ldif) parser.replace_value( 'cn=config,cn=ldbm database,cn=plugins,cn=config', 'nsslapd-db-locks', - ['50000'] + [b'50000'] ) if self.config_ldif: # parse modifications from ldif file supplied by the admin From cbc2abe019f87a0bf69e63704fe06376e9b4c556 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 9 Jan 2017 19:01:29 +0100 Subject: [PATCH 04/10] py3: ldap modlist must have keys as string, not bytes https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/ipaldap.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index daee068..11448f0 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -886,7 +886,8 @@ def encode(self, val): elif isinstance(val, tuple): return tuple(self.encode(m) for m in val) elif isinstance(val, dict): - dct = dict((self.encode(k), self.encode(v)) for k, v in val.items()) + # key in dict must be str not bytes + dct = dict((k, self.encode(v)) for k, v in val.items()) return dct elif isinstance(val, datetime.datetime): return val.strftime(LDAP_GENERALIZED_TIME_FORMAT) From 51578def5ec74fb76993f2a5f510d1d70d33a3c1 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 9 Jan 2017 19:26:04 +0100 Subject: [PATCH 05/10] py3: ipautil: open tempfiles in text mode Code in ipautlis works with text, so tempfiles should be open in textmode otherwise TypeErrors are raised https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/ipautil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 34d10ef..f2b3d74 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -1020,7 +1020,7 @@ def config_replace_variables(filepath, replacevars=dict(), appendvars=dict()): orig_stat = os.stat(filepath) old_values = dict() temp_filename = None - with tempfile.NamedTemporaryFile(delete=False) as new_config: + with tempfile.NamedTemporaryFile(mode="w", delete=False) as new_config: temp_filename = new_config.name with open(filepath, 'r') as f: for line in f: @@ -1106,7 +1106,7 @@ def add_options(config, replacevars, appendvars, oldvars): orig_stat = os.stat(filepath) old_values = dict() temp_filename = None - with tempfile.NamedTemporaryFile(delete=False) as new_config: + with tempfile.NamedTemporaryFile(mode='w', delete=False) as new_config: temp_filename = new_config.name with open(filepath, 'r') as f: in_section = False From 45bdfebb22d44b76fd46efbed9f8d329d43e41b4 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 9 Jan 2017 19:28:57 +0100 Subject: [PATCH 06/10] py3: CA/KRA: config parser requires string basedn is DN object it has to be converted to string before it can be used with config parser https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/cainstance.py | 3 ++- ipaserver/install/krainstance.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index c7e81f0..e1275c0 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -35,6 +35,7 @@ import shlex import pipes +import six # pylint: disable=import-error from six.moves.configparser import ConfigParser, RawConfigParser # pylint: enable=import-error @@ -499,7 +500,7 @@ def __spawn_instance(self): # Directory server config.set("CA", "pki_ds_ldap_port", "389") config.set("CA", "pki_ds_password", self.dm_password) - config.set("CA", "pki_ds_base_dn", self.basedn) + config.set("CA", "pki_ds_base_dn", six.text_type(self.basedn)) config.set("CA", "pki_ds_database", "ipaca") if self.use_ldaps: diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py index 554811c..dde0f33 100644 --- a/ipaserver/install/krainstance.py +++ b/ipaserver/install/krainstance.py @@ -22,6 +22,7 @@ import shutil import tempfile +import six # pylint: disable=import-error from six.moves.configparser import ConfigParser # pylint: enable=import-error @@ -187,7 +188,7 @@ def __spawn_instance(self): # Directory server config.set("KRA", "pki_ds_ldap_port", "389") config.set("KRA", "pki_ds_password", self.dm_password) - config.set("KRA", "pki_ds_base_dn", self.basedn) + config.set("KRA", "pki_ds_base_dn", six.text_type(self.basedn)) config.set("KRA", "pki_ds_database", "ipaca") config.set("KRA", "pki_ds_create_new_db", "False") From 19cbc796616ccdadd085f1bb5e0a8152c7839b88 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 10 Jan 2017 13:33:41 +0100 Subject: [PATCH 07/10] py3: write CA/KRA config into file opened in text mode config parser writes data as text so CA/KRA should be opened in textual mode otherwise type errors are raised from installer https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/cainstance.py | 2 +- ipaserver/install/krainstance.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index e1275c0..ff51f41 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -597,7 +597,7 @@ def __spawn_instance(self): config.set("Tomcat", "pki_ajp_host", "::1") # Generate configuration file - with open(cfg_file, "wb") as f: + with open(cfg_file, "w") as f: config.write(f) self.backup_state('installed', True) diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py index dde0f33..a32ed14 100644 --- a/ipaserver/install/krainstance.py +++ b/ipaserver/install/krainstance.py @@ -260,7 +260,7 @@ def __spawn_instance(self): admin_path.write(cert) # Generate configuration file - with open(cfg_file, "wb") as f: + with open(cfg_file, "w") as f: config.write(f) try: From dcfbb8d5de5b35231a84b8dcb64d00ebec0c0df1 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 10 Jan 2017 16:44:46 +0100 Subject: [PATCH 08/10] py3: cainstance: replace mkstemp with NamedTemporaryFile With Python3 files must be opened in textual mode to write text, and best practise is to use fileobject instead fo os.write() and manual encodig https://fedorahosted.org/freeipa/ticket/4985 --- ipaserver/install/cainstance.py | 49 +++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index ff51f41..734eed4 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -655,13 +655,12 @@ def import_ra_cert(self, rafile): Used when setting up replication """ # Add the new RA cert to the database in /etc/httpd/alias - (agent_fd, agent_name) = tempfile.mkstemp() - os.write(agent_fd, self.dm_password) - os.close(agent_fd) - try: - import_pkcs12(rafile, agent_name, self.ra_agent_db, self.ra_agent_pwd) - finally: - os.remove(agent_name) + with tempfile.NamedTemporaryFile(mode="w") as agent_file: + agent_file.write(self.dm_password) + agent_file.flush() + + import_pkcs12( + rafile, agent_file.name, self.ra_agent_db, self.ra_agent_pwd) self.configure_agent_renewal() @@ -757,10 +756,9 @@ def __import_ca_chain(self): ca_dn = DN(('CN','Certificate Authority'), self.subject_base) for cert in certlist: - try: - chain_fd, chain_name = tempfile.mkstemp() - os.write(chain_fd, cert) - os.close(chain_fd) + with tempfile.NamedTemporaryFile(mode="w") as chain_file: + chain_file.write(cert) + chain_file.flush() (_rdn, subject_dn) = certs.get_cert_nickname(cert) if subject_dn == ca_dn: nick = get_ca_nickname(self.realm) @@ -770,10 +768,8 @@ def __import_ca_chain(self): trust_flags = ',,' self.__run_certutil( ['-A', '-t', trust_flags, '-n', nick, '-a', - '-i', chain_name] + '-i', chain_file.name] ) - finally: - os.remove(chain_name) # Restore NSS trust flags of all previously existing certificates for nick, trust_flags in cert_backup_list: @@ -781,13 +777,15 @@ def __import_ca_chain(self): def __request_ra_certificate(self): # create a temp file storing the pwd - (agent_fd, agent_pwdfile) = tempfile.mkstemp(dir=paths.VAR_LIB_IPA) - os.write(agent_fd, self.admin_password) - os.close(agent_fd) + agent_file = tempfile.NamedTemporaryFile( + mode="w", dir=paths.VAR_LIB_IPA, delete=False) + agent_file.write(self.admin_password) + agent_file.close() # create a temp pem file storing the CA chain - (chain_fd, chain_file) = tempfile.mkstemp(dir=paths.VAR_LIB_IPA) - os.close(chain_fd) + chain_file = tempfile.NamedTemporaryFile( + mode="w", dir=paths.VAR_LIB_IPA, delete=False) + chain_file.close() chain = self.__get_ca_chain() data = base64.b64decode(chain) @@ -797,17 +795,17 @@ def __request_ra_certificate(self): "-inform", "DER", "-print_certs", - "-out", chain_file, + "-out", chain_file.name, ], stdin=data, capture_output=False) agent_args = [paths.DOGTAG_IPA_CA_RENEW_AGENT_SUBMIT, "--dbdir", self.agent_db, "--nickname", "ipa-ca-agent", - "--cafile", chain_file, + "--cafile", chain_file.name, "--ee-url", 'http://%s:8080/ca/ee/ca/' % self.fqdn, "--agent-url", 'https://%s:8443/ca/agent/ca/' % self.fqdn, - "--sslpinfile", agent_pwdfile] + "--sslpinfile", agent_file.name] helper = " ".join(agent_args) # configure certmonger renew agent to use temporary agent cert @@ -840,8 +838,11 @@ def __request_ra_certificate(self): certmonger.modify_ca_helper( ipalib.constants.RENEWAL_CA_NAME, old_helper) # remove the pwdfile - os.remove(agent_pwdfile) - os.remove(chain_file) + for f in (agent_file, chain_file): + try: + os.remove(f.name) + except IOError: + pass def __setup_sign_profile(self): # Tell the profile to automatically issue certs for RAs From d8959ba9955cb8a339469731ac1fe545e2f15dc4 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 10 Jan 2017 18:21:13 +0100 Subject: [PATCH 09/10] py3: _httplib_request: don't convert string to bytes There is no need to encode hostname to bytes. UTF-8 characters must be encoded in different format in URL anyway and it causes only error in Py3. String must be unicode to support Py2. https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/dogtag.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py index eb1f73e..37e7a58 100644 --- a/ipapython/dogtag.py +++ b/ipapython/dogtag.py @@ -188,9 +188,7 @@ def _httplib_request( Perform a HTTP(s) request. """ - if isinstance(host, unicode): - host = host.encode('utf-8') - uri = '%s://%s%s' % (protocol, ipautil.format_netloc(host, port), path) + uri = u'%s://%s%s' % (protocol, ipautil.format_netloc(host, port), path) root_logger.debug('request %s %s', method, uri) root_logger.debug('request body %r', request_body) From 2dc1ce3eccb95a4a3ce6dc5e4b2a69006b95c462 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 10 Jan 2017 18:24:16 +0100 Subject: [PATCH 10/10] py3: HTTPResponse has no 'dict' attribute in 'msg' There is no 'dict' attribute in 'msg', but 'msg' attribute is dict-like object in both py2/3, so it can be used instead. https://fedorahosted.org/freeipa/ticket/4985 --- ipapython/dogtag.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py index 37e7a58..c6a8346 100644 --- a/ipapython/dogtag.py +++ b/ipapython/dogtag.py @@ -205,7 +205,7 @@ def _httplib_request( res = conn.getresponse() http_status = res.status - http_headers = res.msg.dict + http_headers = res.msg http_body = res.read() conn.close() except Exception as e:
-- 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
