URL: https://github.com/freeipa/freeipa/pull/1867 Author: tiran Title: #1867: Load certificate files as binary data Action: opened
PR body: """ In Python 3, cryptography requires certificate data to be binary. Even PEM encoded files are treated as binary content. certmap-match and cert-find were loading certificates as text files. A new BinaryFile type loads files as binary content. Fixes: https://pagure.io/freeipa/issue/7520 Signed-off-by: Christian Heimes <chei...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1867/head:pr1867 git checkout pr1867
From 505d4749cd15b32c3d1aa1bfe4020ab7d33ba40d Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Fri, 27 Apr 2018 12:29:17 +0200 Subject: [PATCH] Load certificate files as binary data In Python 3, cryptography requires certificate data to be binary. Even PEM encoded files are treated as binary content. certmap-match and cert-find were loading certificates as text files. A new BinaryFile type loads files as binary content. Fixes: https://pagure.io/freeipa/issue/7520 Signed-off-by: Christian Heimes <chei...@redhat.com> --- ipaclient/plugins/cert.py | 4 ++-- ipaclient/plugins/certmap.py | 4 ++-- ipalib/cli.py | 19 ++++++++++++------- ipalib/parameters.py | 16 ++++++++++++++-- ipatests/test_cmdline/test_cli.py | 12 ++++++++++++ 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/ipaclient/plugins/cert.py b/ipaclient/plugins/cert.py index a8d1e2127c..6d453d0d6e 100644 --- a/ipaclient/plugins/cert.py +++ b/ipaclient/plugins/cert.py @@ -27,7 +27,7 @@ from ipalib import errors from ipalib import x509 from ipalib import util -from ipalib.parameters import File, Flag, Str +from ipalib.parameters import BinaryFile, File, Flag, Str from ipalib.plugable import Registry from ipalib.text import _ @@ -196,7 +196,7 @@ class cert_remove_hold(MethodOverride): @register(override=True, no_fail=True) class cert_find(MethodOverride): takes_options = ( - File( + BinaryFile( 'file?', label=_("Input filename"), doc=_('File to load the certificate from.'), diff --git a/ipaclient/plugins/certmap.py b/ipaclient/plugins/certmap.py index 981ba292f6..31db991971 100644 --- a/ipaclient/plugins/certmap.py +++ b/ipaclient/plugins/certmap.py @@ -4,7 +4,7 @@ from ipaclient.frontend import MethodOverride from ipalib import errors, x509 -from ipalib.parameters import File +from ipalib.parameters import BinaryFile from ipalib.plugable import Registry from ipalib.text import _ @@ -14,7 +14,7 @@ @register(override=True, no_fail=True) class certmap_match(MethodOverride): takes_args = ( - File( + BinaryFile( 'file?', label=_("Input file"), doc=_("File to load the certificate from"), diff --git a/ipalib/cli.py b/ipalib/cli.py index 3fde581974..8d89c8f518 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -56,7 +56,7 @@ NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError, PromptFailed) from ipalib.constants import CLI_TAB, LDAP_GENERALIZED_TIME_FORMAT -from ipalib.parameters import File, Str, Enum, Any, Flag +from ipalib.parameters import File, BinaryFile, Str, Enum, Any, Flag from ipalib.text import _ from ipalib import api # pylint: disable=unused-import from ipapython.dnsutil import DNSName @@ -1333,7 +1333,7 @@ def load_files(self, cmd, kw): 3) the webUI will use a different way of loading files """ for p in cmd.params(): - if isinstance(p, File): + if isinstance(p, (File, BinaryFile)): # FIXME: this only reads the first file raw = None if p.name in kw: @@ -1342,9 +1342,8 @@ def load_files(self, cmd, kw): else: fname = kw[p.name] try: - f = open(fname, 'r') - raw = f.read() - f.close() + with open(fname, p.open_mode) as f: + raw = f.read() except IOError as e: raise ValidationError( name=to_cli(p.cli_name), @@ -1352,14 +1351,20 @@ def load_files(self, cmd, kw): ) elif p.stdin_if_missing: try: - raw = sys.stdin.read() + if six.PY3 and p.type is bytes: + raw = sys.stdin.buffer.read() + else: + raw = sys.stdin.read() except IOError as e: raise ValidationError( name=to_cli(p.cli_name), error=e.args[1] ) if raw: - kw[p.name] = self.Backend.textui.decode(raw) + if p.type is bytes: + kw[p.name] = raw + else: + kw[p.name] = self.Backend.textui.decode(raw) elif p.required: raise ValidationError( name=to_cli(p.cli_name), error=_('No file to read') diff --git a/ipalib/parameters.py b/ipalib/parameters.py index 902ae3401b..62d555129b 100644 --- a/ipalib/parameters.py +++ b/ipalib/parameters.py @@ -1753,17 +1753,29 @@ def _validate_scalar(self, value, index=None): class File(Str): - """ - File parameter type. + """Text file parameter type. Accepts file names and loads their content into the parameter value. """ + open_mode = 'r' + kwargs = Data.kwargs + ( + # valid for CLI, other backends (e.g. webUI) can ignore this + ('stdin_if_missing', bool, False), + ('noextrawhitespace', bool, False), + ) + + +class BinaryFile(Bytes): + """Binary file parameter type + """ + open_mode = 'rb' kwargs = Data.kwargs + ( # valid for CLI, other backends (e.g. webUI) can ignore this ('stdin_if_missing', bool, False), ('noextrawhitespace', bool, False), ) + class DateTime(Param): """ DateTime parameter type. diff --git a/ipatests/test_cmdline/test_cli.py b/ipatests/test_cmdline/test_cli.py index e1305b9beb..2b9d859d4e 100644 --- a/ipatests/test_cmdline/test_cli.py +++ b/ipatests/test_cmdline/test_cli.py @@ -3,12 +3,14 @@ import shlex import subprocess import sys +import tempfile import unittest import six from six import StringIO from ipatests import util +from ipatests.test_ipalib.test_x509 import goodcert_headers from ipalib import api, errors import pytest @@ -312,6 +314,16 @@ def test_without_options(): if not adtrust_is_enabled: mockldap.del_entry(adtrust_dn) + def test_certfind(self): + with tempfile.NamedTemporaryFile() as f: + f.write(goodcert_headers) + f.flush() + self.check_command( + 'cert_find --file={}'.format(f.name), + 'cert_find', + file=goodcert_headers + ) + def test_cli_fsencoding(): # https://pagure.io/freeipa/issue/5887
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
