URL: https://github.com/freeipa/freeipa/pull/853 Author: stlaz Title: #853: x509,certdb: handle certificates as bytes Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/853/head:pr853 git checkout pr853
From 0e5e2562615389183157458dce24233f23dc1746 Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka <slazn...@redhat.com> Date: Mon, 5 Jun 2017 16:12:34 +0200 Subject: [PATCH 1/2] x509,certdb: handle certificates as bytes Certificates, both in PEM and DER format, should be handled as bytes in Python 3. https://pagure.io/freeipa/issue/4985 --- ipalib/x509.py | 16 +++++++------- ipapython/certdb.py | 44 ++++++++++++++++++++++++++------------- ipaserver/install/cainstance.py | 4 ++-- ipaserver/install/installutils.py | 4 ++-- ipatests/test_xmlrpc/testcert.py | 2 +- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/ipalib/x509.py b/ipalib/x509.py index 4d866a64c5..60835508ce 100644 --- a/ipalib/x509.py +++ b/ipalib/x509.py @@ -235,12 +235,10 @@ def make_pem(data): Convert a raw base64-encoded blob into something that looks like a PE file with lines split to 64 characters and proper headers. """ - if isinstance(data, bytes): - data = data.decode('ascii') - pemcert = '\r\n'.join([data[x:x+64] for x in range(0, len(data), 64)]) - return '-----BEGIN CERTIFICATE-----\n' + \ - pemcert + \ - '\n-----END CERTIFICATE-----' + pemcert = b'\r\n'.join([data[x:x+64] for x in range(0, len(data), 64)]) + return (b'-----BEGIN CERTIFICATE-----\n' + pemcert + + b'\n-----END CERTIFICATE-----') + def normalize_certificate(rawcert): """ @@ -299,7 +297,7 @@ def write_certificate(rawcert, filename): dercert = normalize_certificate(rawcert) try: - fp = open(filename, 'w') + fp = open(filename, 'wb') fp.write(make_pem(base64.b64encode(dercert))) fp.close() except (IOError, OSError) as e: @@ -315,11 +313,11 @@ def write_certificate_list(rawcerts, filename): dercerts = [normalize_certificate(rawcert) for rawcert in rawcerts] try: - with open(filename, 'w') as f: + with open(filename, 'wb') as f: for cert in dercerts: cert = base64.b64encode(cert) cert = make_pem(cert) - f.write(cert + '\n') + f.write(cert + b'\n') except (IOError, OSError) as e: raise errors.FileError(reason=str(e)) diff --git a/ipapython/certdb.py b/ipapython/certdb.py index 8c53821916..c0d35ba449 100644 --- a/ipapython/certdb.py +++ b/ipapython/certdb.py @@ -183,7 +183,7 @@ def unparse_trust_flags(trust_flags): def verify_kdc_cert_validity(kdc_cert, ca_certs, realm): pem_kdc_cert = kdc_cert.public_bytes(serialization.Encoding.PEM) - pem_ca_certs = '\n'.join( + pem_ca_certs = b'\n'.join( cert.public_bytes(serialization.Encoding.PEM) for cert in ca_certs) with NamedTemporaryFile() as kdc_file, NamedTemporaryFile() as ca_file: @@ -442,8 +442,12 @@ def import_files(self, files, import_keys=False, key_password=None, "Failed to open %s: %s" % (filename, e.strerror)) # Try to parse the file as PEM file - matches = list(re.finditer( - r'-----BEGIN (.+?)-----(.*?)-----END \1-----', data, re.DOTALL)) + matches = list( + re.finditer( + br'-----BEGIN (.+?)-----(.*?)-----END \1-----', + data, re.DOTALL + ) + ) if matches: loaded = False for match in matches: @@ -451,12 +455,12 @@ def import_files(self, files, import_keys=False, key_password=None, label = match.group(1) line = len(data[:match.start() + 1].splitlines()) - if label in ('CERTIFICATE', 'X509 CERTIFICATE', - 'X.509 CERTIFICATE'): + if label in (b'CERTIFICATE', b'X509 CERTIFICATE', + b'X.509 CERTIFICATE'): try: x509.load_certificate(match.group(2)) except ValueError as e: - if label != 'CERTIFICATE': + if label != b'CERTIFICATE': root_logger.warning( "Skipping certificate in %s at line %s: %s", filename, line, e) @@ -466,11 +470,12 @@ def import_files(self, files, import_keys=False, key_password=None, loaded = True continue - if label in ('PKCS7', 'PKCS #7 SIGNED DATA', 'CERTIFICATE'): + if label in (b'PKCS7', b'PKCS #7 SIGNED DATA', + b'CERTIFICATE'): try: certs = x509.pkcs7_to_pems(body) except ipautil.CalledProcessError as e: - if label == 'CERTIFICATE': + if label == b'CERTIFICATE': root_logger.warning( "Skipping certificate in %s at line %s: %s", filename, line, e) @@ -484,9 +489,9 @@ def import_files(self, files, import_keys=False, key_password=None, loaded = True continue - if label in ('PRIVATE KEY', 'ENCRYPTED PRIVATE KEY', - 'RSA PRIVATE KEY', 'DSA PRIVATE KEY', - 'EC PRIVATE KEY'): + if label in (b'PRIVATE KEY', b'ENCRYPTED PRIVATE KEY', + b'RSA PRIVATE KEY', b'DSA PRIVATE KEY', + b'EC PRIVATE KEY'): if not import_keys: continue @@ -500,8 +505,8 @@ def import_files(self, files, import_keys=False, key_password=None, '-topk8', '-passout', 'file:' + self.pwd_file, ] - if ((label != 'PRIVATE KEY' and key_password) or - label == 'ENCRYPTED PRIVATE KEY'): + if ((label != b'PRIVATE KEY' and key_password) or + label == b'ENCRYPTED PRIVATE KEY'): key_pwdfile = ipautil.write_tmp_file(key_password) args += [ '-passin', 'file:' + key_pwdfile.name, @@ -614,6 +619,14 @@ def trust_root_cert(self, root_nickname, trust_flags): "Setting trust on %s failed" % root_nickname) def get_cert(self, nickname, pem=False): + """ + :param nickname: nickname of the certificate in the NSS database + :param pem: returns DER-encoded certificate if False, otherwise it + returns a PEM-encoded certificate + :returns: string in Python2 + bytes in Python3 + """ + args = ['-L', '-n', nickname, '-a'] try: result = self.run_certutil(args, capture_output=True) @@ -624,6 +637,9 @@ def get_cert(self, nickname, pem=False): cert, _start = find_cert_from_txt(cert, start=0) cert = x509.strip_header(cert) cert = base64.b64decode(cert) + else: + # encode the output string to bytes + cert = cert.encode('ascii') return cert def has_nickname(self, nickname): @@ -638,7 +654,7 @@ def has_nickname(self, nickname): def export_pem_cert(self, nickname, location): """Export the given cert to PEM file in the given location""" cert = self.get_cert(nickname, pem=True) - with open(location, "w+") as fd: + with open(location, "w+b") as fd: fd.write(cert) os.chmod(location, 0o444) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 28a702dba5..8a6de77e21 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -805,10 +805,10 @@ def __export_ca_chain(self): for path in [paths.IPA_CA_CRT, paths.KDC_CA_BUNDLE_PEM, paths.CA_BUNDLE_PEM]: - with open(path, 'w') as ipaca_pem: + with open(path, 'wb') as ipaca_pem: for cert in certlist: ipaca_pem.write(cert) - ipaca_pem.write('\n') + ipaca_pem.write(b'\n') def __request_ra_certificate(self): # create a temp file storing the pwd diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index 04bf4fcd20..e679d4e590 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -1217,11 +1217,11 @@ def load_external_cert(files, ca_subject): (subject, ", ".join(files), e)) cert_file = tempfile.NamedTemporaryFile() - cert_file.write(ca_cert_chain[0] + '\n') + cert_file.write(ca_cert_chain[0] + b'\n') cert_file.flush() ca_file = tempfile.NamedTemporaryFile() - ca_file.write('\n'.join(ca_cert_chain[1:]) + '\n') + ca_file.write(b'\n'.join(ca_cert_chain[1:]) + b'\n') ca_file.flush() return cert_file, ca_file diff --git a/ipatests/test_xmlrpc/testcert.py b/ipatests/test_xmlrpc/testcert.py index c27ea95b05..a01f765cdd 100644 --- a/ipatests/test_xmlrpc/testcert.py +++ b/ipatests/test_xmlrpc/testcert.py @@ -96,4 +96,4 @@ def makecert(reqdir, subject, principal): csr = unicode(generate_csr(reqdir, pwname, str(subject))) res = api.Command['cert_request'](csr, principal=principal, add=True) - return x509.make_pem(res['result']['certificate']) + return x509.make_pem(res['result']['certificate'].encode('ascii')) From 58faef9f1c49c0ce0977fa0158160ccdf01643da Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka <slazn...@redhat.com> Date: Wed, 7 Jun 2017 10:17:37 +0200 Subject: [PATCH 2/2] tests fixup --- ipatests/test_xmlrpc/testcert.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/testcert.py b/ipatests/test_xmlrpc/testcert.py index a01f765cdd..6e5c738753 100644 --- a/ipatests/test_xmlrpc/testcert.py +++ b/ipatests/test_xmlrpc/testcert.py @@ -96,4 +96,10 @@ def makecert(reqdir, subject, principal): csr = unicode(generate_csr(reqdir, pwname, str(subject))) res = api.Command['cert_request'](csr, principal=principal, add=True) - return x509.make_pem(res['result']['certificate'].encode('ascii')) + cert = res['result']['certificate'] + if six.PY3: + # in Python 3, make_pem expects a bytes instance + # in Python 2, we need to pass unicode so that the PEM cert is handled + # properly in the Bytes._covnert_scalar() method + cert = cert.encode('ascii') + return x509.make_pem(cert)
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org