On 11/25/2015 11:04 AM, Jan Cholasta wrote:
> On 24.11.2015 17:21, Petr Viktorin wrote:
>> On 11/23/2015 10:50 AM, Jan Cholasta wrote:
>>> On 23.11.2015 07:43, Jan Cholasta wrote:
>>>> On 19.11.2015 00:55, Petr Viktorin wrote:
>>>>> On 11/03/2015 02:39 PM, Petr Viktorin wrote:
>>>>>> Hello,
>>>>>> Python 3's strings are Unicode, so data coming to or leaving a Python
>>>>>> program needs to be decoded/encoded if it's to be handled as a
>>>>>> string.
>>>>>> One of the boundaries where encoding is necessary is external
>>>>>> programs,
>>>>>> specifically, ipautil.run.
>>>>>> Unfortunately there's no one set of options that would work for all
>>>>>> run() invocations, so I went through them all and specified the
>>>>>> stdin/stdout/stderr encoding where necessary. I've also updated the
>>>>>> call
>>>>>> sites to make it clearer if the return values are used or not.
>>>>>> If an encoding is not set, run() will accept/return bytes. (This is a
>>>>>> fail-safe setting, since it can't raise errors, and using bytes where
>>>>>> strings are expected generally fails loudly in py3.)
>>>>>>
>>>>>> Note that the changes are not effective under Python 2.
>>>>>
>>>>> ping,
>>>>> Could someone look at this patch?
>>>>
>>>> Looking.
>>>
>>> 1) I think a better approach would be to use str for stdin/stdout/stderr
>>> by default in both Python 2 and 3, i.e. do nothing in Python 2 and
>>> encode/decode using locale.getpreferredencoding() in Python 3. This is
>>> consistent with sys.std{in,out,err} in both Python 2 and 3. Using bytes
>>> or different encoding should be opt-in.
>>>
>>> Note that different encoding should be used only if the LC_ALL or LANG
>>> variables are overriden in the command's environment.
>>
>> That would assume the output of *everything* run via ipautil.run can be
>> decoded using the locale encoding. Any stray invalid byte would make IPA
>> crash, even in cases where we don't care about the output. IPA calls too
>> many weird tools to assume they all output text.
> 
> Such a stray invalid byte may bubble through our code and cause havoc
> somewhere else, which is much harder to troubleshoot than a crash in
> ipautil.run(), where you can see that it was a misbehaving command that
> caused the crash and exactly what command it was.

The invalid byte can't bubble through code, because if you don't specify
an encoding you get bytes back. You'll get a crash when you try using it
as a string.

locale.getpreferredencoding() is not used consistently in all software,
especially if it's not written in Python. For instance, wget won't
magically re-encode the data it fetches for us. It's better to
explicitly specify the encoding every time.

> If we don't care about output somewhere, we should not capture it there
> at all.

Then people need to remember to put "capture_output=True" everywhere
(except that also disables logging of the output, so we'd need an
additional option).

>>> 3) I think the "surrogateescape" error handler would be better for
>>> non-strict decoding, as the text can be encoded back to bytes without
>>> loss of information.
>>
>> Non-strict decoding is meant for logging, so we want backslash escapes
>> rather than invalid surrogates. If you need round-tripping then you
>> should use bytes.
> 
> I see. There is no guarantee that non-strict output will be used only
> for logging though.
> 
> I think it might be better to return raw output and let the caller use a
> decode-for-logging utility function instead. The logger already escapes
> invalid bytes itself, so the function would have to be used only when
> raising exceptions.

It's also used for logging at a higher level (info-critical).
But this makes sense; changed.

-- 
Petr Viktorin

From 778db1a57fe4bee8b111f7b5c0faee50560e9aa6 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 27 Oct 2015 17:54:54 +0100
Subject: [PATCH] Handle encoding for ipautil.run

For Python 2, nothing is changed, and all data discussed below is
bytestrings (str).

Parts of the "argv" for run() may be strings or bytes, and may even be
mixed.

Stdin must be bytes (or None) by default.
If stdin_encoding is given, it may be a string, and is encoded appropriately.

Stdout and stderr are returned as bytes, unless specified otherwise.
This means there will be no decoding errors in the (very common) case
where these are ignored.
For the logs, these output streams are encoded with 'backslashreplace',
so errors can't be raised, and also binary garbage won't show up in logs.

If the stdout_encoding and/or stderr_encoding arguments are given,
the corresponding stream is decoded. This may raise an exception
if the data can't be encoded.
To suppress errors, pass {stdout,stderr}_strict=False. This will cause
invalid characters to be replaced with backslash sequences. It's appropriate
when the stream is copied to a log file, NOT when the data is further
manipulated.
---
 .../certmonger/dogtag-ipa-ca-renew-agent-submit    |  9 ++-
 install/certmonger/ipa-server-guard                | 11 +++-
 install/oddjob/com.redhat.idm.trust-fetch-domains  |  5 +-
 install/tools/ipa-adtrust-install                  |  3 +-
 install/tools/ipa-replica-conncheck                | 17 ++++--
 ipa-client/ipa-install/ipa-client-install          | 35 ++++++++----
 ipalib/plugins/pwpolicy.py                         |  3 +-
 ipaplatform/base/services.py                       | 32 ++++++-----
 ipaplatform/redhat/services.py                     |  3 +-
 ipaplatform/redhat/tasks.py                        | 11 +++-
 ipapython/certdb.py                                | 14 +++--
 ipapython/dnssec/bindmgr.py                        | 14 ++++-
 ipapython/dnssec/odsmgr.py                         |  3 +-
 ipapython/ipautil.py                               | 65 ++++++++++++++++++----
 ipapython/kernel_keyring.py                        | 44 +++++++++++----
 ipaserver/dcerpc.py                                | 15 ++---
 ipaserver/install/cainstance.py                    | 24 +++++---
 ipaserver/install/certs.py                         |  2 +-
 ipaserver/install/httpinstance.py                  |  8 ++-
 ipaserver/install/ipa_backup.py                    | 38 ++++++++++---
 ipaserver/install/ipa_restore.py                   | 32 +++++++++--
 ipaserver/install/krbinstance.py                   | 10 +++-
 ipaserver/install/opendnssecinstance.py            |  7 ++-
 ipaserver/install/replication.py                   |  2 +-
 ipaserver/install/server/install.py                |  9 ++-
 ipatests/test_ipapython/test_ipautil.py            | 35 ++++++++++++
 ipatests/test_ipapython/test_keyring.py            | 17 +++---
 ipatests/test_xmlrpc/test_host_plugin.py           |  2 +-
 28 files changed, 349 insertions(+), 121 deletions(-)

diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 44993b038a38da60a25843147e86b64deda874e1..06a2450cfef74f173d1cc359c6d2772860e18cfc 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -156,8 +156,13 @@ def request_cert():
     args = [path] + sys.argv[1:]
     if os.environ.get('CERTMONGER_CA_PROFILE') == 'caCACert':
         args += ['-N', '-O', 'bypassCAnotafter=true']
-    stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ)
-    sys.stderr.write(stderr)
+    stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ,
+                                     stdout_encoding='ascii')
+    if six.PY2:
+        sys.stderr.write(stderr)
+    else:
+        # Write bytes directly
+        sys.stderr.buffer.write(stderr)  #pylint: disable=no-member
     sys.stderr.flush()
 
     syslog.syslog(syslog.LOG_NOTICE, "dogtag-ipa-renew-agent returned %d" % rc)
diff --git a/install/certmonger/ipa-server-guard b/install/certmonger/ipa-server-guard
index 7ce3e43fce16ce9974e5db10af7cf851c0411943..a0a2e2b3c5b8d118418037d31728f4f28d45f325 100755
--- a/install/certmonger/ipa-server-guard
+++ b/install/certmonger/ipa-server-guard
@@ -30,6 +30,8 @@ import sys
 import syslog
 import traceback
 
+import six
+
 from ipapython import ipautil
 from ipaserver.install import certs
 
@@ -41,9 +43,14 @@ def main():
     with certs.renewal_lock:
         stdout, stderr, rc = ipautil.run(sys.argv[1:], raiseonerr=False,
                                          env=os.environ)
-        sys.stdout.write(stdout)
+        if six.PY2:
+            sys.stdout.write(stdout)
+            sys.stderr.write(stderr)
+        else:
+            # Write bytes directly
+            sys.stdout.buffer.write(stdout)  #pylint: disable=no-member
+            sys.stderr.buffer.write(stderr)  #pylint: disable=no-member
         sys.stdout.flush()
-        sys.stderr.write(stderr)
         sys.stderr.flush()
 
     return rc
diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains b/install/oddjob/com.redhat.idm.trust-fetch-domains
index 019545b9340ac95f4e528618875df3dcd8170feb..ea82e086ef5bade9be3b9f30ae50504c4fcd5db7 100755
--- a/install/oddjob/com.redhat.idm.trust-fetch-domains
+++ b/install/oddjob/com.redhat.idm.trust-fetch-domains
@@ -26,9 +26,8 @@ def retrieve_keytab(api, ccache_name, oneway_keytab_name, oneway_principal):
     if os.path.isfile(oneway_keytab_name):
         os.unlink(oneway_keytab_name)
 
-    (stdout, stderr, retcode) = ipautil.run(getkeytab_args,
-                                            env={'KRB5CCNAME': ccache_name, 'LANG': 'C'},
-                                            raiseonerr=False)
+    ipautil.run(getkeytab_args, env={'KRB5CCNAME': ccache_name, 'LANG': 'C'},
+                raiseonerr=False)
     # Make sure SSSD is able to read the keytab
     try:
         sssd = pwd.getpwnam('sssd')
diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index ff69d69e2c11ce08b8b648a5a78777c472da2ac9..05be43b0894ef0457edfa4161e06fc0e01cd8b0e 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -200,7 +200,8 @@ def set_and_check_netbios_name(netbios_name, unattended):
 
 def ensure_admin_kinit(admin_name, admin_password):
     try:
-        ipautil.run(['kinit', admin_name], stdin=admin_password+'\n')
+        ipautil.run(['kinit', admin_name], stdin=admin_password+'\n',
+                    stdin_encoding='ascii')
     except ipautil.CalledProcessError as e:
         print("There was error to automatically re-kinit your admin user ticket.")
         return False
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index a67837c545a9d3a4b328925483d65592ebe94ed1..a5ed73d0cd09c2cf7b38cb04e9613365aa43b473 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -38,8 +38,11 @@ import threading
 import errno
 from socket import SOCK_STREAM, SOCK_DGRAM
 import distutils.spawn
+import locale
+
 from ipaplatform.paths import paths
 import gssapi
+import six
 
 CONNECT_TIMEOUT = 5
 RESPONDERS = [ ]
@@ -402,20 +405,24 @@ def main():
                         sys.exit("Principal password required")
 
 
-                stderr=''
-                (stdout, stderr, returncode) = ipautil.run([paths.KINIT, principal],
+                (_stdout, stderr, returncode) = ipautil.run([paths.KINIT, principal],
                      env={'KRB5_CONFIG':KRB5_CONFIG, 'KRB5CCNAME':CCACHE_FILE},
-                    stdin=password, raiseonerr=False)
+                    stdin=password, raiseonerr=False, stdin_encoding='ascii')
                 if returncode != 0:
+                    if six.PY3:
+                        stderr = stderr.decode(locale.getpreferredencoding(),
+                                               errors='replace')
                     raise RuntimeError("Cannot acquire Kerberos ticket: %s" % stderr)
 
                 # Verify kinit was actually successful
-                stderr=''
-                (stdout, stderr, returncode) = ipautil.run([paths.BIN_KVNO,
+                (_stdout, stderr, returncode) = ipautil.run([paths.BIN_KVNO,
                      'host/%s' % options.master],
                     env={'KRB5_CONFIG':KRB5_CONFIG, 'KRB5CCNAME':CCACHE_FILE},
                     raiseonerr=False)
                 if returncode != 0:
+                    if six.PY3:
+                        stderr = stderr.decode(locale.getpreferredencoding(),
+                                               errors='replace')
                     raise RuntimeError("Could not get ticket for master server: %s" % stderr)
 
             user = principal.partition('@')[0]
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 05a550b11e74db84e46a126798c4db728226865c..d2a443b57ad6b7f223e3fad5d10b746193a221de 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -34,9 +34,11 @@ try:
     import shutil
     import dns
     import gssapi
+    import locale
 
     import nss.nss as nss
     import SSSDConfig
+    import six
     from six.moves.urllib.parse import urlparse, urlunparse
 
     from ipapython.ipa_log_manager import standard_logging_setup, root_logger
@@ -602,8 +604,12 @@ def uninstall(options, env):
         if options.debug:
             join_args.append("-d")
             env['XMLRPC_TRACE_CURL'] = 'yes'
-        (stdout, stderr, returncode) = run(join_args, raiseonerr=False, env=env)
+        _stdout, stderr, returncode = run(
+            join_args, raiseonerr=False, env=env)
         if returncode != 0:
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             root_logger.error("Unenrolling host failed: %s", stderr)
 
     if os.path.exists(paths.IPA_DEFAULT_CONF):
@@ -1437,7 +1443,7 @@ def configure_sshd_config(fstore, options):
                 args.append('-o')
                 args.append('%s=%s' % item)
 
-            (stdout, stderr, retcode) = ipautil.run(args, raiseonerr=False)
+            _stdout, _stderr, retcode = ipautil.run(args, raiseonerr=False)
             if retcode == 0:
                 authorized_keys_changes = candidate
                 break
@@ -1478,6 +1484,9 @@ def configure_automount(options):
     except Exception as e:
         root_logger.error('Automount configuration failed: %s', str(e))
     else:
+        if six.PY3:
+            stdout = stdout.decode(locale.getpreferredencoding(),
+                                   errors='replace')
         root_logger.info(stdout)
 
 
@@ -1490,7 +1499,8 @@ def configure_nisdomain(options, domain):
     # First backup the old NIS domain name
     if os.path.exists(paths.BIN_NISDOMAINNAME):
         try:
-            nis_domain_name, _, _ = ipautil.run([paths.BIN_NISDOMAINNAME])
+            nis_domain_name, _, _ = ipautil.run([paths.BIN_NISDOMAINNAME],
+                                                stdout_encoding='ascii')
         except CalledProcessError as e:
             pass
 
@@ -1529,8 +1539,9 @@ def unconfigure_nisdomain():
 
 
 def get_iface_from_ip(ip_addr):
-    ipresult = ipautil.run([paths.IP, '-oneline', 'address', 'show'])
-    for line in ipresult[0].split('\n'):
+    sout, _serr, _rc = ipautil.run([paths.IP, '-oneline', 'address', 'show'],
+                                   stdout_encoding='ascii')
+    for line in sout.split('\n'):
         fields = line.split()
         if len(fields) < 6:
             continue
@@ -1547,8 +1558,8 @@ def get_local_ipaddresses(iface=None):
     args = [paths.IP, '-oneline', 'address', 'show']
     if iface:
         args += ['dev', iface]
-    ipresult = ipautil.run(args)
-    lines = ipresult[0].split('\n')
+    sout, _serr, _rc = ipautil.run(args, stdout_encoding='ascii')
+    lines = sout.split('\n')
     ips = []
     for line in lines:
         fields = line.split()
@@ -1919,7 +1930,8 @@ def get_ca_certs_from_http(url, warn=True):
     root_logger.debug("trying to retrieve CA cert via HTTP from %s", url)
     try:
 
-        stdout, stderr, rc = run([paths.BIN_WGET, "-O", "-", url])
+        stdout, _stderr, _rc = run([paths.BIN_WGET, "-O", "-", url],
+                                   stdout_encoding='ascii')
     except CalledProcessError as e:
         raise errors.NoCertificateError(entry=url)
 
@@ -2668,7 +2680,9 @@ def install(options, env, fstore, statestore):
                 return CLIENT_INSTALL_ERROR
 
             # Now join the domain
-            (stdout, stderr, returncode) = run(join_args, raiseonerr=False, env=env, nolog=nolog)
+            _stdout, stderr, returncode = run(
+                join_args, raiseonerr=False, env=env, nolog=nolog,
+                stderr_encoding=locale.getpreferredencoding())
 
             if returncode != 0:
                 root_logger.error("Joining realm failed: %s", stderr)
@@ -2691,8 +2705,7 @@ def install(options, env, fstore, statestore):
                 subject_base = DN(subject_base)
 
             if options.principal is not None:
-                stderr, stdout, returncode = run(
-                        ["kdestroy"], raiseonerr=False, env=env)
+                run(["kdestroy"], raiseonerr=False, env=env)
 
             # Obtain the TGT. We do it with the temporary krb5.conf, so that
             # only the KDC we're installing under is contacted.
diff --git a/ipalib/plugins/pwpolicy.py b/ipalib/plugins/pwpolicy.py
index 5e98d5469eac0a3f0094d0485dc5d0aaee299438..4d4d32a0615208d9d4a1a285e53d446315263de5 100644
--- a/ipalib/plugins/pwpolicy.py
+++ b/ipalib/plugins/pwpolicy.py
@@ -279,7 +279,8 @@ class pwpolicy(LDAPObject):
     has_lockout = False
     lockout_params = ()
 
-    (stdout, stderr, rc) = run(['klist', '-V'], raiseonerr=False)
+    stdout, _stderr, rc = run(['klist', '-V'], raiseonerr=False,
+                              stdout_encoding='ascii')
     if rc == 0:
         verstr = stdout.split()[-1]
         ver = version.LooseVersion(verstr)
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index da2f1011e34431664cd5c730668ae483b7bd0a1d..e2e3b351a62ca8291771bd29856acc5ce19d5368 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -27,6 +27,7 @@ import os
 import json
 import time
 import collections
+import locale
 
 import six
 
@@ -320,9 +321,8 @@ class SystemdService(PlatformService):
 
         while True:
             try:
-                (sout, serr, rcode) = ipautil.run(
+                sout, _serr, rcode = ipautil.run(
                     [paths.SYSTEMCTL, "is-active", instance],
-                    capture_output=True
                 )
             except ipautil.CalledProcessError as e:
                 if e.returncode == 3 and 'activating' in str(e.output):
@@ -331,6 +331,9 @@ class SystemdService(PlatformService):
                 return False
             else:
                 # activating
+                if six.PY3:
+                    sout = sout.decode(locale.getpreferredencoding(),
+                                       errors='replace')
                 if rcode == 3 and 'activating' in str(sout):
                     time.sleep(SERVICE_POLL_INTERVAL)
                     continue
@@ -342,12 +345,14 @@ class SystemdService(PlatformService):
 
     def is_installed(self):
         try:
-            (sout, serr, rcode) = ipautil.run([paths.SYSTEMCTL,
-                                               "list-unit-files",
-                                               "--full"])
+            sout, _serr, rcode = ipautil.run(
+                [paths.SYSTEMCTL, "list-unit-files", "--full"])
             if rcode != 0:
                 return False
             else:
+                if six.PY3:
+                    sout = sout.decode(locale.getpreferredencoding(),
+                                       errors='replace')
                 svar = self.parse_variables(sout)
                 if not self.service_instance("") in svar:
                     # systemd doesn't show the service
@@ -360,10 +365,9 @@ class SystemdService(PlatformService):
     def is_enabled(self, instance_name=""):
         enabled = True
         try:
-            (sout, serr, rcode) = ipautil.run(
-                                      [paths.SYSTEMCTL,
-                                       "is-enabled",
-                                        self.service_instance(instance_name)])
+            _sout, _serr, rcode = ipautil.run(
+                [paths.SYSTEMCTL, "is-enabled",
+                 self.service_instance(instance_name)])
 
             if rcode != 0:
                 enabled = False
@@ -375,11 +379,13 @@ class SystemdService(PlatformService):
     def is_masked(self, instance_name=""):
         masked = False
         try:
-            (sout, serr, rcode) = ipautil.run(
-                                      [paths.SYSTEMCTL,
-                                       "is-enabled",
-                                        self.service_instance(instance_name)])
+            sout, _serr, rcode = ipautil.run(
+                [paths.SYSTEMCTL, "is-enabled",
+                 self.service_instance(instance_name)])
 
+            if six.PY3:
+                sout = sout.decode(locale.getpreferredencoding(),
+                                   errors='replace')
             if rcode == 1 and sout == 'masked':
                 masked = True
 
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 0902215a56191032a1a65d0c2d05ddd5b7dab67f..ac4645109676bea5714c944985bd5f1adcff1816 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -220,7 +220,8 @@ class RedHatCAService(RedHatService):
                     url
                 ]
 
-                stdout, stderr, returncode = ipautil.run(args)
+                stdout, _stderr, _returncode = ipautil.run(
+                    args, stdout_encoding='utf-8')
 
                 status = dogtag._parse_ca_status(stdout)
                 # end of workaround
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..70e2fa6db955ef9bb7d061745c6ed7ad0f5ffd77 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,10 +30,12 @@ import stat
 import socket
 import sys
 import base64
-
+import locale
 from subprocess import CalledProcessError
+
 from nss.error import NSPRError
 from pyasn1.error import PyAsn1Error
+import six
 from six.moves import urllib
 
 from ipapython.ipa_log_manager import root_logger, log_mgr
@@ -375,7 +377,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
             if state is None:
                 continue
             try:
-                (stdout, stderr, rc) = ipautil.run([paths.GETSEBOOL, setting])
+                stdout, _stderr, _rc = ipautil.run(
+                    [paths.GETSEBOOL, setting],
+                )
+                if six.PY3:
+                    stdout = stdout.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 original_state = stdout.split()[2]
                 if backup_func is not None:
                     backup_func(setting, original_state)
diff --git a/ipapython/certdb.py b/ipapython/certdb.py
index 704bae528b44a3a3b978d25982a7e75a8e3af0f6..3f687c584102f7844555b79dd2dce0254f0c5370 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -110,7 +110,7 @@ class NSSDatabase(object):
     def run_certutil(self, args, stdin=None):
         new_args = [paths.CERTUTIL, "-d", self.secdir]
         new_args = new_args + args
-        return ipautil.run(new_args, stdin)
+        return ipautil.run(new_args, stdin, stdin_encoding='ascii')
 
     def create_db(self, password_filename):
         """Create cert DB
@@ -177,7 +177,7 @@ class NSSDatabase(object):
             pkcs12_passwd = pkcs12_passwd + '\n'
             args = args + ["-w", paths.DEV_STDIN]
         try:
-            ipautil.run(args, stdin=pkcs12_passwd)
+            ipautil.run(args, stdin=pkcs12_passwd, stdin_encoding='ascii')
         except ipautil.CalledProcessError as e:
             if e.returncode == 17:
                 raise RuntimeError("incorrect password for pkcs#12 file %s" %
@@ -247,7 +247,10 @@ class NSSDatabase(object):
                             '-print_certs',
                         ]
                         try:
-                            stdout, stderr, rc = ipautil.run(args, stdin=body)
+                            stdout, _stderr, rc = ipautil.run(
+                                args, stdin=body,
+                                stdin_encoding='ascii',
+                                stdout_encoding='ascii')
                         except ipautil.CalledProcessError as e:
                             if label == 'CERTIFICATE':
                                 root_logger.warning(
@@ -286,7 +289,10 @@ class NSSDatabase(object):
                                 '-passin', 'file:' + key_pwdfile.name,
                             ]
                         try:
-                            stdout, stderr, rc = ipautil.run(args, stdin=body)
+                            stdout, _stderr, _rc = ipautil.run(
+                                args, stdin=body,
+                                stdin_encoding='ascii',
+                                stdout_encoding='ascii')
                         except ipautil.CalledProcessError as e:
                             root_logger.warning(
                                 "Skipping private key in %s at line %s: %s",
diff --git a/ipapython/dnssec/bindmgr.py b/ipapython/dnssec/bindmgr.py
index 1822dacf2535e7c37062e4d639e01289edcf5074..9eb1630c57c972595d763952b8f9f93cafe55769 100644
--- a/ipapython/dnssec/bindmgr.py
+++ b/ipapython/dnssec/bindmgr.py
@@ -10,6 +10,9 @@ import logging
 import shutil
 import stat
 import subprocess
+import locale
+
+import six
 
 from ipalib import api
 import ipalib.constants
@@ -40,8 +43,11 @@ class BINDMgr(object):
 
     def notify_zone(self, zone):
         cmd = ['rndc', 'sign', zone.to_text()]
-        output = ipautil.run(cmd)[0]
-        self.log.info(output)
+        output, _stderr, _rc = ipautil.run(cmd)
+        if six.PY3:
+            output = output.decode(locale.getpreferredencoding(),
+                                   errors='replace')
+        self.log.info('%s', output)
 
     def dn2zone_name(self, dn):
         """cn=KSK-20140813162153Z-cede9e182fc4af76c4bddbc19123a565,cn=keys,idnsname=test,cn=dns,dc=ipa,dc=example"""
@@ -110,7 +116,9 @@ class BINDMgr(object):
         cmd.append(zone.to_text())
 
         # keys has to be readable by ODS & named
-        basename = ipautil.run(cmd)[0].strip()
+        stdout, _stderr, _rc = ipautil.run(
+            cmd, stdout_encoding=locale.getpreferredencoding())
+        basename = stdout.strip()
         private_fn = "%s/%s.private" % (workdir, basename)
         os.chmod(private_fn, FILE_PERM)
         # this is useful mainly for debugging
diff --git a/ipapython/dnssec/odsmgr.py b/ipapython/dnssec/odsmgr.py
index efbe16cc6ebf050d9cf347ed97b2b2e4b37c8a6e..f81f25b7d86f7d7132423cb018542569dbf43c88 100644
--- a/ipapython/dnssec/odsmgr.py
+++ b/ipapython/dnssec/odsmgr.py
@@ -128,7 +128,8 @@ class ODSMgr(object):
         Raises CalledProcessError if returncode != 0.
         """
         cmd = ['ods-ksmutil'] + params
-        return ipautil.run(cmd)[0]
+        stdout, _stderr, _rc = ipautil.run(cmd, stdout_encoding='ascii')
+        return stdout
 
     def get_ods_zonelist(self):
         stdout = self.ksmutil(['zonelist', 'export'])
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4acdd1a98818bf311a8fef103e7219cc62a28ec1..309326f24a07e25135fb3131d48351fe5c9a71cc 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -37,6 +37,7 @@ import gssapi
 import pwd
 import grp
 from contextlib import contextmanager
+import locale
 
 from dns import resolver, rdatatype
 from dns.exception import DNSException
@@ -154,8 +155,10 @@ class CheckedIPAddress(netaddr.IPAddress):
             elif addr.version == 6:
                 family = 'inet6'
 
-            ipresult = run([paths.IP, '-family', family, '-oneline', 'address', 'show'])
-            lines = ipresult[0].split('\n')
+            stdout, _stderr, _rc = run(
+                [paths.IP, '-family', family, '-oneline', 'address', 'show'],
+                stdout_encoding='ascii')
+            lines = stdout.split('\n')
             for line in lines:
                 fields = line.split()
                 if len(fields) < 4:
@@ -255,11 +258,26 @@ def write_tmp_file(txt):
     return fd
 
 def shell_quote(string):
-    return "'" + string.replace("'", "'\\''") + "'"
+    if isinstance(string, str):
+        return "'" + string.replace("'", "'\\''") + "'"
+    else:
+        return b"'" + string.replace(b"'", b"'\\''") + b"'"
+
+if six.PY3:
+    def _log_arg(s):
+        """Convert string or bytes to a string suitable for logging"""
+        if isinstance(s, bytes):
+            return s.decode(locale.getpreferredencoding(),
+                            errors='replace')
+        else:
+            return s
+else:
+    _log_arg = str
 
 def run(args, stdin=None, raiseonerr=True,
         nolog=(), env=None, capture_output=True, skip_output=False, cwd=None,
-        runas=None, timeout=None, suplementary_groups=[]):
+        runas=None, timeout=None, suplementary_groups=[],
+        stdin_encoding=None, stdout_encoding=None, stderr_encoding=None):
     """
     Execute a command and return stdin, stdout and the process return code.
 
@@ -292,6 +310,11 @@ def run(args, stdin=None, raiseonerr=True,
     :param suplementary_groups: List of group names that will be used as
         suplementary groups for subporcess.
         The option runas must be specified together with this option.
+    :param stdin_encoding: For Python 3, the encoding of the stdin string.
+        If None, bytes are expected. Only effective if stdin is not None.
+    :param stdout_encoding: For Python 3, the encoding with which stdout
+        should be decoded. If None, bytes are returned.
+    :param stderr_encoding: Like stdout_encoding, but for stderr
     """
     assert isinstance(suplementary_groups, list)
     p_in = None
@@ -318,12 +341,15 @@ def run(args, stdin=None, raiseonerr=True,
         p_out = subprocess.PIPE
         p_err = subprocess.PIPE
 
+    if six.PY3 and stdin is not None and stdin_encoding:
+        stdin = stdin.encode(stdin_encoding)
+
     if timeout:
         # If a timeout was provided, use the timeout command
         # to execute the requested command.
         args[0:0] = [paths.BIN_TIMEOUT, str(timeout)]
 
-    arg_string = nolog_replace(' '.join(shell_quote(a) for a in args), nolog)
+    arg_string = nolog_replace(' '.join(_log_arg(a) for a in args), nolog)
     root_logger.debug('Starting external process')
     root_logger.debug('args=%s' % arg_string)
 
@@ -352,7 +378,13 @@ def run(args, stdin=None, raiseonerr=True,
                              close_fds=True, env=env, cwd=cwd,
                              preexec_fn=preexec_fn)
         stdout,stderr = p.communicate(stdin)
-        stdout,stderr = str(stdout), str(stderr)    # Make pylint happy
+        if six.PY2:
+            stdout, stderr = str(stdout), str(stderr)    # Make pylint happy
+        else:
+            if stdout_encoding:
+                stdout = stdout.decode(stdout_encoding)
+            if stderr_encoding:
+                stderr = stderr.decode(stderr_encoding)
     except KeyboardInterrupt:
         root_logger.debug('Process interrupted')
         p.wait()
@@ -374,8 +406,18 @@ def run(args, stdin=None, raiseonerr=True,
     if capture_output and not skip_output:
         stdout = nolog_replace(stdout, nolog)
         stderr = nolog_replace(stderr, nolog)
-        root_logger.debug('stdout=%s' % stdout)
-        root_logger.debug('stderr=%s' % stderr)
+        if six.PY3 and isinstance(stdout, bytes):
+            stdout_logstr = stdout.decode(locale.getpreferredencoding(),
+                                          errors='replace')
+        else:
+            stdout_logstr = stdout
+        if six.PY3 and isinstance(stderr, bytes):
+            stderr_logstr = stderr.decode(locale.getpreferredencoding(),
+                                          errors='replace')
+        else:
+            stderr_logstr = stderr
+        root_logger.debug('stdout=%s' % stdout_logstr)
+        root_logger.debug('stderr=%s' % stderr_logstr)
 
     if p.returncode != 0 and raiseonerr:
         raise CalledProcessError(p.returncode, arg_string, stdout)
@@ -1278,9 +1320,12 @@ def kinit_password(principal, password, ccache_name, config=None,
 
     # this workaround enables us to capture stderr and put it
     # into the raised exception in case of unsuccessful authentication
-    (stdout, stderr, retcode) = run(args, stdin=password, env=env,
-                                    raiseonerr=False)
+    _stdout, stderr, retcode = run(args, stdin=password, env=env,
+                                   raiseonerr=False)
     if retcode:
+        if six.PY3:
+            stderr = stderr.decode(locale.getpreferredencoding(),
+                                   errors='replace')
         raise RuntimeError(stderr)
 
 
diff --git a/ipapython/kernel_keyring.py b/ipapython/kernel_keyring.py
index d30531cabaee5c12376f0821a21a6f63cd60397c..ab7224a878aab178e5c2e72765a08aef79f93447 100644
--- a/ipapython/kernel_keyring.py
+++ b/ipapython/kernel_keyring.py
@@ -18,6 +18,9 @@
 #
 
 import os
+import locale
+
+import six
 
 from ipapython.ipautil import run
 
@@ -44,13 +47,17 @@ def get_real_key(key):
     One cannot request a key based on the description it was created with
     so find the one we're looking for.
     """
-    (stdout, stderr, rc) = run(['keyctl', 'search', KEYRING, KEYTYPE, key], raiseonerr=False)
+    assert isinstance(key, str)
+    stdout, _stderr, rc = run(['keyctl', 'search', KEYRING, KEYTYPE, key],
+                              raiseonerr=False)
     if rc:
         raise ValueError('key %s not found' % key)
     return stdout.rstrip()
 
 def get_persistent_key(key):
-    (stdout, stderr, rc) = run(['keyctl', 'get_persistent', KEYRING, key], raiseonerr=False)
+    assert isinstance(key, str)
+    stdout, _stderr, rc = run(['keyctl', 'get_persistent', KEYRING, key],
+                              raiseonerr=False)
     if rc:
         raise ValueError('persistent key %s not found' % key)
     return stdout.rstrip()
@@ -68,6 +75,7 @@ def has_key(key):
     """
     Returns True/False whether the key exists in the keyring.
     """
+    assert isinstance(key, str)
     try:
         get_real_key(key)
         return True
@@ -80,10 +88,11 @@ def read_key(key):
 
     Use pipe instead of print here to ensure we always get the raw data.
     """
+    assert isinstance(key, str)
     real_key = get_real_key(key)
-    (stdout, stderr, rc) = run(['keyctl', 'pipe', real_key], raiseonerr=False)
+    stdout, stderr, rc = run(['keyctl', 'pipe', real_key], raiseonerr=False)
     if rc:
-        raise ValueError('keyctl pipe failed: %s' % stderr)
+        _raise_error('keyctl pipe failed', stderr)
 
     return stdout
 
@@ -91,11 +100,14 @@ def update_key(key, value):
     """
     Update the keyring data. If they key doesn't exist it is created.
     """
+    assert isinstance(key, str)
+    assert isinstance(value, bytes)
     if has_key(key):
         real_key = get_real_key(key)
-        (stdout, stderr, rc) = run(['keyctl', 'pupdate', real_key], stdin=value, raiseonerr=False)
+        _stdout, stderr, rc = run(['keyctl', 'pupdate', real_key], stdin=value,
+                                  raiseonerr=False)
         if rc:
-            raise ValueError('keyctl pupdate failed: %s' % stderr)
+            _raise_error('keyctl pupdate failed', stderr)
     else:
         add_key(key, value)
 
@@ -103,17 +115,29 @@ def add_key(key, value):
     """
     Add a key to the kernel keyring.
     """
+    assert isinstance(key, str)
+    assert isinstance(value, bytes)
     if has_key(key):
         raise ValueError('key %s already exists' % key)
-    (stdout, stderr, rc) = run(['keyctl', 'padd', KEYTYPE, key, KEYRING], stdin=value, raiseonerr=False)
+    _stdout, stderr, rc = run(['keyctl', 'padd', KEYTYPE, key, KEYRING],
+                              stdin=value, raiseonerr=False)
     if rc:
-        raise ValueError('keyctl padd failed: %s' % stderr)
+        _raise_error('keyctl padd failed', stderr)
 
 def del_key(key):
     """
     Remove a key from the keyring
     """
+    assert isinstance(key, str)
     real_key = get_real_key(key)
-    (stdout, stderr, rc) = run(['keyctl', 'unlink', real_key, KEYRING], raiseonerr=False)
+    _stdout, stderr, rc = run(['keyctl', 'unlink', real_key, KEYRING],
+                               raiseonerr=False)
     if rc:
-        raise ValueError('keyctl unlink failed: %s' % stderr)
+        _raise_error('keyctl unlink failed', stderr)
+
+
+def _raise_error(message, stderr):
+    if six.PY3:
+        stderr = stderr.decode(locale.getpreferredencoding(),
+                               errors='replace')
+    raise ValueError('%s: %s' % (message, stderr))
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 2e412861ebc265a9b07c8634068151181a3e9b9e..7da72da93b99f635b66334ac7c14a0b4669fccfd 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -588,7 +588,7 @@ class DomainValidator(object):
         # Destroy the contents of the ccache
         root_logger.debug('Destroying the contents of the separate ccache')
 
-        (stdout, stderr, returncode) = ipautil.run(
+        ipautil.run(
             [paths.KDESTROY, '-A', '-c', ccache_path],
             env={'KRB5CCNAME': ccache_path},
             raiseonerr=False)
@@ -597,7 +597,7 @@ class DomainValidator(object):
         root_logger.debug('Running kinit from ipa.keytab to obtain HTTP '
                           'service principal with MS-PAC attached.')
 
-        (stdout, stderr, returncode) = ipautil.run(
+        (_stdout, _stderr, returncode) = ipautil.run(
             [paths.KINIT, '-kt', keytab, principal],
             env={'KRB5CCNAME': ccache_path},
             raiseonerr=False)
@@ -605,8 +605,6 @@ class DomainValidator(object):
         if returncode == 0:
             return (ccache_path, principal)
         else:
-            root_logger.debug('Kinit failed, stout: %s, stderr: %s'
-                              % (stdout, stderr))
             return (None, None)
 
     def kinit_as_administrator(self, domain):
@@ -634,7 +632,7 @@ class DomainValidator(object):
         # Destroy the contents of the ccache
         root_logger.debug('Destroying the contents of the separate ccache')
 
-        (stdout, stderr, returncode) = ipautil.run(
+        ipautil.run(
             [paths.KDESTROY, '-A', '-c', ccache_path],
             env={'KRB5CCNAME': ccache_path},
             raiseonerr=False)
@@ -642,17 +640,16 @@ class DomainValidator(object):
         # Destroy the contents of the ccache
         root_logger.debug('Running kinit with credentials of AD administrator')
 
-        (stdout, stderr, returncode) = ipautil.run(
+        (_stdout, _stderr, returncode) = ipautil.run(
             [paths.KINIT, principal],
             env={'KRB5CCNAME': ccache_path},
             stdin=password,
-            raiseonerr=False)
+            raiseonerr=False,
+            stdin_encoding='ascii')
 
         if returncode == 0:
             return (ccache_path, principal)
         else:
-            root_logger.debug('Kinit failed, stout: %s, stderr: %s'
-                              % (stdout, stderr))
             return (None, None)
 
     def search_in_dc(self, domain, filter, attrs, scope, basedn=None,
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 3632a39312c338fe0529e283fdb84145aabe96e2..cb89fc820d537b09555f99b4a4c96ccaaa2ff3e6 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -548,10 +548,11 @@ class CAInstance(DogtagInstance):
             x509.write_certificate(cert.der_data, cert_file.name)
             cert_file.flush()
 
-            cert_chain, stderr, rc = ipautil.run(
+            cert_chain, _stderr, _rc = ipautil.run(
                 [paths.OPENSSL, 'crl2pkcs7',
                  '-certfile', self.cert_chain_file,
-                 '-nocrl'])
+                 '-nocrl'],
+                stdout_encoding='ascii')
             # Dogtag chokes on the header and footer, remove them
             # https://bugzilla.redhat.com/show_bug.cgi?id=1127838
             cert_chain = re.search(
@@ -662,7 +663,8 @@ class CAInstance(DogtagInstance):
             '%s' % ipautil.format_netloc(self.fqdn, 8443),
         ]
         (stdout, _stderr, _returncode) = ipautil.run(
-            args, nolog=(self.admin_password,))
+            args, nolog=(self.admin_password,),
+            stdout_encoding='ascii')
 
         data = stdout.split('\n')
         params = get_defList(data)
@@ -684,7 +686,8 @@ class CAInstance(DogtagInstance):
             '%s' % ipautil.format_netloc(self.fqdn, 8443),
         ]
         (stdout, _stderr, _returncode) = ipautil.run(
-            args, nolog=(self.admin_password,))
+            args, nolog=(self.admin_password,),
+            stdout_encoding='ascii')
 
         data = stdout.split('\n')
         outputList = get_outputList(data)
@@ -777,14 +780,15 @@ class CAInstance(DogtagInstance):
 
         conn.disconnect()
 
-    def __run_certutil(self, args, database=None, pwd_file=None, stdin=None):
+    def __run_certutil(self, args, database=None, pwd_file=None, stdin=None,
+                       **kwargs):
         if not database:
             database = self.ra_agent_db
         if not pwd_file:
             pwd_file = self.ra_agent_pwd
         new_args = [paths.CERTUTIL, "-d", database, "-f", pwd_file]
         new_args = new_args + args
-        return ipautil.run(new_args, stdin, nolog=(pwd_file,))
+        return ipautil.run(new_args, stdin, nolog=(pwd_file,), **kwargs)
 
     def __create_ra_agent_db(self):
         if ipautil.file_exists(self.ra_agent_db + "/cert8.db"):
@@ -829,7 +833,7 @@ class CAInstance(DogtagInstance):
              "-inform",
              "DER",
              "-print_certs",
-             ], stdin=data)
+             ], stdin=data, stdout_encoding='ascii')
 
         # Ok, now we have all the certificates in certs, walk through it
         # and pull out each certificate and add it to our database
@@ -873,7 +877,8 @@ class CAInstance(DogtagInstance):
             (stdout, _stderr, _returncode) = self.__run_certutil(
                 ["-R", "-k", "rsa", "-g", "2048", "-s",
                  str(DN(('CN', 'IPA RA'), self.subject_base)),
-                 "-z", noise_name, "-a"])
+                 "-z", noise_name, "-a"],
+                stdout_encoding='utf-8')
         finally:
             os.remove(noise_name)
 
@@ -1052,7 +1057,8 @@ class CAInstance(DogtagInstance):
 
     def publish_ca_cert(self, location):
         args = ["-L", "-n", self.canickname, "-a"]
-        (cert, _err, _returncode) = self.__run_certutil(args)
+        (cert, _err, _returncode) = self.__run_certutil(
+            args, stdout_encoding='ascii')
         fd = open(location, "w+")
         fd.write(cert)
         fd.close()
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index c918791f0be7a17e20123fe6f94c4ac0bbf09d7b..8a6aa7c52ecc9f691258503778eee6abf72d445f 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -173,7 +173,7 @@ class CertDB(object):
         new_args = [paths.SIGNTOOL, "-d", self.secdir, "-p", password]
 
         new_args = new_args + args
-        ipautil.run(new_args, stdin)
+        ipautil.run(new_args, stdin, stdin_encoding='ascii')
 
     def create_noise_file(self):
         if ipautil.file_exists(self.noise_fname):
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 1b68573d7a64c63d6b0343a1536c90b527f21a05..d1bb3dff5c0e80c23753850ca58c2ef0d6fcf411 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -28,6 +28,9 @@ import re
 import dbus
 import shlex
 import pipes
+import locale
+
+import six
 
 from ipaserver.install import service
 from ipaserver.install import certs
@@ -63,13 +66,16 @@ def httpd_443_configured():
     False otherwise.
     """
     try:
-        (stdout, stderr, rc) = ipautil.run([paths.HTTPD, '-t', '-D', 'DUMP_VHOSTS'])
+        stdout, _stderr, _rc = ipautil.run(
+            [paths.HTTPD, '-t', '-D', 'DUMP_VHOSTS'])
     except ipautil.CalledProcessError as e:
         service.print_msg("WARNING: cannot check if port 443 is already configured")
         service.print_msg("httpd returned error when checking: %s" % e)
         return False
 
     port_line_re = re.compile(r'(?P<address>\S+):(?P<port>\d+)')
+    if six.PY3:
+        stdout = stdout.decode(locale.getpreferredencoding(), errors='replace')
     for line in stdout.splitlines():
         m = port_line_re.match(line)
         if m and int(m.group('port')) == 443:
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 6d97ef13b383b9917fa70426a99463f8c14955e8..244508fbc2a2a0f72e88773509996675b3c5d208 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -26,7 +26,9 @@ import pwd
 from optparse import OptionGroup
 from ipaplatform.paths import paths
 from ipaplatform import services
+import locale
 
+import six
 from six.moves.configparser import SafeConfigParser
 
 from ipalib import api, errors
@@ -86,8 +88,10 @@ def encrypt_file(filename, keyring, remove_original=True):
     args.append('-e')
     args.append(source)
 
-    (stdout, stderr, rc) = run(args, raiseonerr=False)
+    _stdout, stderr, rc = run(args, raiseonerr=False)
     if rc != 0:
+        if six.PY3:
+            stderr = stderr.decode(locale.getpreferredencoding(), errors='replace')
         raise admintool.ScriptError('gpg failed: %s' % stderr)
 
     if remove_original:
@@ -420,8 +424,11 @@ class Backup(admintool.AdminTool):
                     '-r',
                     '-n', backend,
                     '-a', ldiffile]
-            (stdout, stderr, rc) = run(args, raiseonerr=False)
+            _stdout, stderr, rc = run(args, raiseonerr=False)
             if rc != 0:
+                if six.PY3:
+                    stderr = stderr.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 self.log.critical("db2ldif failed: %s", stderr)
 
         # Move the LDIF backup to our location
@@ -464,8 +471,11 @@ class Backup(admintool.AdminTool):
             wait_for_task(conn, dn)
         else:
             args = [paths.DB2BAK, bakdir, '-Z', instance]
-            (stdout, stderr, rc) = run(args, raiseonerr=False)
+            _stdout, stderr, rc = run(args, raiseonerr=False)
             if rc != 0:
+                if six.PY3:
+                    stderr = stderr.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 self.log.critical("db2bak failed: %s" % stderr)
 
         shutil.move(bakdir, self.dir)
@@ -493,8 +503,11 @@ class Backup(admintool.AdminTool):
         if options.logs:
             args.extend(verify_directories(self.logs))
 
-        (stdout, stderr, rc) = run(args, raiseonerr=False)
+        _stdout, stderr, rc = run(args, raiseonerr=False)
         if rc != 0:
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             raise admintool.ScriptError('tar returned non-zero code '
                 '%d: %s' % (rc, stderr))
 
@@ -514,15 +527,21 @@ class Backup(admintool.AdminTool):
                    ]
             args.extend(missing_directories)
 
-            (stdout, stderr, rc) = run(args, raiseonerr=False)
+            _stdout, stderr, rc = run(args, raiseonerr=False)
             if rc != 0:
+                if six.PY3:
+                    stderr = stderr.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 raise admintool.ScriptError('tar returned non-zero %d when adding '
                     'directory structure: %s' % (rc, stderr))
 
         # Compress the archive. This is done separately, since 'tar' cannot
         # append to a compressed archive.
-        (stdout, stderr, rc) = run(['gzip', tarfile], raiseonerr=False)
+        _stdout, stderr, rc = run(['gzip', tarfile], raiseonerr=False)
         if rc != 0:
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             raise admintool.ScriptError('gzip returned non-zero %d when '
                 'compressing the backup: %s' % (rc, stderr))
 
@@ -596,9 +615,12 @@ class Backup(admintool.AdminTool):
                 filename,
                 '.'
                ]
-        (stdout, stderr, rc) = run(args, raiseonerr=False)
+        _stdout, stderr, rc = run(args, raiseonerr=False)
         if rc != 0:
-            raise admintool.ScriptError('tar returned non-zero %d: %s' % (rc, stdout))
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
+            raise admintool.ScriptError('tar returned non-zero %d: %s' % (rc, stderr))
 
         if encrypt:
             self.log.info('Encrypting %s' % filename)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index a257b7892c3d32bc81cbff1574c2898eeedcda5a..6edc25d3681ca98d08b2c65ad15a7aec592af5d8 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -25,7 +25,9 @@ import time
 import pwd
 import ldif
 import itertools
+import locale
 
+import six
 from six.moves.configparser import SafeConfigParser
 
 from ipalib import api, errors, constants
@@ -88,8 +90,11 @@ def decrypt_file(tmpdir, filename, keyring):
     args.append('-d')
     args.append(source)
 
-    (stdout, stderr, rc) = run(args, raiseonerr=False)
+    _stdout, stderr, rc = run(args, raiseonerr=False)
     if rc != 0:
+        if six.PY3:
+            stderr = stderr.decode(locale.getpreferredencoding(),
+                                   errors='replace')
         raise admintool.ScriptError('gpg failed: %s' % stderr)
 
     return dest
@@ -365,8 +370,11 @@ class Restore(admintool.AdminTool):
                     dirsrv.start(capture_output=False)
             else:
                 self.log.info('Stopping IPA services')
-                (stdout, stderr, rc) = run(['ipactl', 'stop'], raiseonerr=False)
+                _stdout, stderr, rc = run(['ipactl', 'stop'], raiseonerr=False)
                 if rc not in [0, 6]:
+                    if six.PY3:
+                        stderr = stderr.decode(locale.getpreferredencoding(),
+                                               errors='replace')
                     self.log.warn('Stopping IPA failed: %s' % stderr)
 
                 self.restore_selinux_booleans()
@@ -573,8 +581,11 @@ class Restore(admintool.AdminTool):
                     '-Z', instance,
                     '-i', ldiffile,
                     '-n', backend]
-            (stdout, stderr, rc) = run(args, raiseonerr=False)
+            _stdout, stderr, rc = run(args, raiseonerr=False)
             if rc != 0:
+                if six.PY3:
+                    stderr = stderr.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 self.log.critical("ldif2db failed: %s" % stderr)
 
 
@@ -628,8 +639,11 @@ class Restore(admintool.AdminTool):
             if backend is not None:
                 args.append('-n')
                 args.append(backend)
-            (stdout, stderr, rc) = run(args, raiseonerr=False)
+            _stdout, stderr, rc = run(args, raiseonerr=False)
             if rc != 0:
+                if six.PY3:
+                    stderr = stderr.decode(locale.getpreferredencoding(),
+                                           errors='replace')
                 self.log.critical("bak2db failed: %s" % stderr)
 
 
@@ -650,8 +664,11 @@ class Restore(admintool.AdminTool):
                 paths.IPA_DEFAULT_CONF[1:],
                ]
 
-        (stdout, stderr, rc) = run(args, raiseonerr=False)
+        _stdout, stderr, rc = run(args, raiseonerr=False)
         if rc != 0:
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             self.log.critical('Restoring %s failed: %s' %
                               (paths.IPA_DEFAULT_CONF, stderr))
         os.chdir(cwd)
@@ -696,8 +713,11 @@ class Restore(admintool.AdminTool):
             args.append('--exclude')
             args.append('var/log')
 
-        (stdout, stderr, rc) = run(args, raiseonerr=False)
+        _stdout, stderr, rc = run(args, raiseonerr=False)
         if rc != 0:
+            if six.PY3:
+                stderr = stderr.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             self.log.critical('Restoring files failed: %s', stderr)
 
         os.chdir(cwd)
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 1dd807c714aea52faeca732c2c1aba12d83fdc53..32708c95a3dc0e143b45de8116c41577602134f9 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -28,6 +28,7 @@ import os
 import pwd
 import socket
 import dns.name
+import locale
 
 from ipaserver.install import service
 from ipaserver.install import installutils
@@ -42,6 +43,7 @@ from ipapython.dn import DN
 from ipaserver.install import replication
 from ipaserver.install import dsinstance
 
+import six
 import pyasn1.codec.ber.decoder
 import struct
 
@@ -280,7 +282,8 @@ class KrbInstance(service.Service):
             self.master_password + '\n',
         )
         try:
-            ipautil.run(args, nolog=(self.master_password,), stdin=''.join(dialogue))
+            ipautil.run(args, nolog=(self.master_password,), stdin=''.join(dialogue),
+                        stdin_encoding=locale.getpreferredencoding())
         except ipautil.CalledProcessError as e:
             print("Failed to initialize the realm container")
 
@@ -294,8 +297,11 @@ class KrbInstance(service.Service):
         MIN_KRB5KDC_WITH_WORKERS = "1.9"
         cpus = os.sysconf('SC_NPROCESSORS_ONLN')
         workers = False
-        (stdout, stderr, rc) = ipautil.run(['klist', '-V'], raiseonerr=False)
+        stdout, _stderr, rc = ipautil.run(['klist', '-V'], raiseonerr=False)
         if rc == 0:
+            if six.PY3:
+                stdout = stdout.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             verstr = stdout.split()[-1]
             ver = version.LooseVersion(verstr)
             min = version.LooseVersion(MIN_KRB5KDC_WITH_WORKERS)
diff --git a/ipaserver/install/opendnssecinstance.py b/ipaserver/install/opendnssecinstance.py
index 4baf6b6bc3d1898aadade8c741f5105b5742d749..c3559741d94027e289422090432bc7c68a68608b 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -288,8 +288,9 @@ class OpenDNSSECInstance(service.Service):
             # regenerate zonelist.xml
             ods_enforcerd = services.knownservices.ods_enforcerd
             cmd = [paths.ODS_KSMUTIL, 'zonelist', 'export']
-            stdout, stderr, retcode = ipautil.run(cmd,
-                                          runas=ods_enforcerd.get_user_name())
+            stdout, _stderr, _retcode = ipautil.run(cmd,
+                                          runas=ods_enforcerd.get_user_name(),
+                                          stdout_encoding='ascii')
             with open(paths.OPENDNSSEC_ZONELIST_FILE, 'w') as zonelistf:
                 zonelistf.write(stdout)
                 os.chown(paths.OPENDNSSEC_ZONELIST_FILE,
@@ -304,7 +305,7 @@ class OpenDNSSECInstance(service.Service):
             ]
 
             ods_enforcerd = services.knownservices.ods_enforcerd
-            ipautil.run(command, stdin="y", runas=ods_enforcerd.get_user_name())
+            ipautil.run(command, stdin=b"y", runas=ods_enforcerd.get_user_name())
 
     def __setup_dnskeysyncd(self):
         # set up dnskeysyncd this is DNSSEC master
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index b408d4fbea87e77e18b1be4e4aedf0ec2b3bc5c6..0d4c798342da73b340b61ca1516de0bedf965a2a 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -88,7 +88,7 @@ def replica_conn_check(master_host, host_name, realm, check_ca,
 
     if check_ca and dogtag_master_ds_port == 7389:
         args.append('--check-ca')
-    (stdin, stderr, returncode) = ipautil.run(
+    _stdin, _stderr, returncode = ipautil.run(
         args, raiseonerr=False, capture_output=False, nolog=nolog)
 
     if returncode != 0:
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 6ecb87ac9b27c78579a08de79c9df8f5ed5f114d..305989c101b1ce57aef6cd9ea9063317e265cce1 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -12,6 +12,7 @@ import shutil
 import sys
 import tempfile
 import textwrap
+import locale
 
 import six
 
@@ -1042,17 +1043,19 @@ def uninstall(installer):
 
     print("Shutting down all IPA services")
     try:
-        (stdout, stderr, rc) = run([paths.IPACTL, "stop"], raiseonerr=False)
+        run([paths.IPACTL, "stop"], raiseonerr=False)
     except Exception as e:
         pass
 
     print("Removing IPA client configuration")
     try:
-        (stdout, stderr, rc) = run([paths.IPA_CLIENT_INSTALL, "--on-master",
+        stdout, _stderr, rc = run([paths.IPA_CLIENT_INSTALL, "--on-master",
                                     "--unattended", "--uninstall"],
                                    raiseonerr=False)
         if rc not in [0, 2]:
-            root_logger.debug("ipa-client-install returned %d" % rc)
+            if six.PY3:
+                stdout = stdout.decode(locale.getpreferredencoding(),
+                                       errors='replace')
             raise RuntimeError(stdout)
     except Exception as e:
         rv = 1
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index d574bd809ab72a6459fd111e319b7bf4638f7b90..e2b825e8399d30ea4c20b0c430e66b8ee37b4591 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -1,3 +1,5 @@
+# encoding: utf-8
+
 # Authors:
 #   Jan Cholasta <jchol...@redhat.com>
 #
@@ -417,3 +419,36 @@ class TestTimeParser(object):
         nose.tools.assert_equal(-30, time.tzinfo.minoffset)
         offset = time.tzinfo.utcoffset(time.tzinfo.dst())
         nose.tools.assert_equal(((24 - 9) * 60 * 60) - (30 * 60), offset.seconds)
+
+
+def test_run():
+    out, _err, rc = ipautil.run(['echo', 'foo\x02bar'])
+    assert rc == 0
+    assert out == b'foo\x02bar\n'
+
+
+def test_run_bytes():
+    out, _err, rc = ipautil.run(['echo', b'\x01\x02'])
+    assert rc == 0
+    assert out == b'\x01\x02\n'
+
+
+def test_run_decode():
+    out, _err, rc = ipautil.run(['echo', u'á'.encode('utf-8')],
+                                stdout_encoding='utf-8')
+    assert rc == 0
+    if six.PY3:
+        assert out == 'á\n'
+    else:
+        assert out == 'á\n'.encode('utf-8')
+
+
+def test_run_decode_bad():
+    if six.PY3:
+        with pytest.raises(UnicodeDecodeError):
+            ipautil.run(['echo', b'\xa0\xa1'], stdout_encoding='utf-8')
+    else:
+        out, _err, rc = ipautil.run(['echo', '\xa0\xa1'],
+                                    stdout_encoding='utf-8')
+        assert rc == 0
+        assert out == '\xa0\xa1\n'
diff --git a/ipatests/test_ipapython/test_keyring.py b/ipatests/test_ipapython/test_keyring.py
index 84b9f0ffa4252b4fdeb4302bf4b67212173dd685..79b4b3334a0e42779802ca96be4ce6f9506f6dda 100644
--- a/ipatests/test_ipapython/test_keyring.py
+++ b/ipatests/test_ipapython/test_keyring.py
@@ -28,8 +28,8 @@ import pytest
 pytestmark = pytest.mark.tier0
 
 TEST_KEY = 'ipa_test'
-TEST_VALUE = 'abc123'
-UPDATE_VALUE = '123abc'
+TEST_VALUE = b'abc123'
+UPDATE_VALUE = b'123abc'
 
 SIZE_256 = 'abcdefgh' * 32
 SIZE_512 = 'abcdefgh' * 64
@@ -94,9 +94,10 @@ class test_keyring(object):
 
         # Now update it 10 times
         for i in range(10):
-            kernel_keyring.update_key(TEST_KEY, 'test %d' %  i)
+            value = ('test %d' %  i).encode('ascii')
+            kernel_keyring.update_key(TEST_KEY, value)
             result = kernel_keyring.read_key(TEST_KEY)
-            assert(result == 'test %d' % i)
+            assert(result == value)
 
         kernel_keyring.del_key(TEST_KEY)
 
@@ -134,9 +135,9 @@ class test_keyring(object):
         """
         Test 512-bytes of data
         """
-        kernel_keyring.add_key(TEST_KEY, SIZE_512)
+        kernel_keyring.add_key(TEST_KEY, SIZE_512.encode('ascii'))
         result = kernel_keyring.read_key(TEST_KEY)
-        assert(result == SIZE_512)
+        assert(result == SIZE_512.encode('ascii'))
 
         kernel_keyring.del_key(TEST_KEY)
 
@@ -144,8 +145,8 @@ class test_keyring(object):
         """
         Test 1k bytes of data
         """
-        kernel_keyring.add_key(TEST_KEY, SIZE_1024)
+        kernel_keyring.add_key(TEST_KEY, SIZE_1024.encode('ascii'))
         result = kernel_keyring.read_key(TEST_KEY)
-        assert(result == SIZE_1024)
+        assert(result == SIZE_1024.encode('ascii'))
 
         kernel_keyring.del_key(TEST_KEY)
diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index 868c09ce8b1974b9eacf08a28bc24454b3416902..052d70de8f2fd9c9e0a20de4b587d176de8e970d 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -694,7 +694,7 @@ class TestHostFalsePwdChange(XMLRPC_test):
         ]
 
         try:
-            out, err, rc = ipautil.run(new_args)
+            ipautil.run(new_args)
         except ipautil.CalledProcessError as e:
             # join operation may fail on 'adding key into keytab', but
             # the keytab is not necessary for further tests
-- 
2.5.0

-- 
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

Reply via email to