Blob Blame History Raw
From 785c496dceb76a8f628249ce598e0540b1dfec6e Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Tue, 6 Nov 2018 13:57:14 +0100
Subject: [PATCH] Allow ipaapi user to access SSSD's info pipe

For smart card authentication, ipaapi must be able to access to sss-ifp.
During installation and upgrade, the ipaapi user is now added to
[ifp]allowed_uids.

The commit also fixes two related issues:

* The server upgrade code now enables ifp service in sssd.conf. The
  existing code modified sssd.conf but never wrote the changes to disk.
* sssd_enable_service() no longer fails after it has detected an
  unrecognized service.

Fixes: https://pagure.io/freeipa/issue/7751
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipaclient/install/client.py                | 41 ++++++++++++++++++----
 ipaserver/install/server/upgrade.py        | 27 +++++++++-----
 ipatests/test_integration/test_commands.py | 31 ++++++++++++++++
 3 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index 05255fe61b..f9b003ef57 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -35,6 +35,7 @@
 # pylint: enable=import-error
 
 from ipalib import api, errors, x509
+from ipalib.constants import IPAAPI_USER
 from ipalib.install import certmonger, certstore, service, sysrestore
 from ipalib.install import hostname as hostname_
 from ipalib.install.kinit import kinit_keytab, kinit_password
@@ -914,7 +915,7 @@ def configure_sssd_conf(
         domain = sssdconfig.new_domain(cli_domain)
 
     if options.on_master:
-        sssd_enable_service(sssdconfig, 'ifp')
+        sssd_enable_ifp(sssdconfig)
 
     if (
         (options.conf_ssh and os.path.isfile(paths.SSH_CONFIG)) or
@@ -1018,21 +1019,47 @@ def configure_sssd_conf(
     return 0
 
 
-def sssd_enable_service(sssdconfig, service):
+def sssd_enable_service(sssdconfig, name):
     try:
-        sssdconfig.new_service(service)
+        sssdconfig.new_service(name)
     except SSSDConfig.ServiceAlreadyExists:
         pass
     except SSSDConfig.ServiceNotRecognizedError:
         logger.error(
-            "Unable to activate the %s service in SSSD config.", service)
+            "Unable to activate the '%s' service in SSSD config.", name)
         logger.info(
             "Please make sure you have SSSD built with %s support "
-            "installed.", service)
+            "installed.", name)
         logger.info(
-            "Configure %s support manually in /etc/sssd/sssd.conf.", service)
+            "Configure %s support manually in /etc/sssd/sssd.conf.", name)
+        return None
 
-    sssdconfig.activate_service(service)
+    sssdconfig.activate_service(name)
+    return sssdconfig.get_service(name)
+
+
+def sssd_enable_ifp(sssdconfig):
+    """Enable and configure libsss_simpleifp plugin
+    """
+    service = sssd_enable_service(sssdconfig, 'ifp')
+    if service is None:
+        # unrecognized service
+        return
+
+    try:
+        uids = service.get_option('allowed_uids')
+    except SSSDConfig.NoOptionError:
+        uids = set()
+    else:
+        uids = {s.strip() for s in uids.split(',') if s.strip()}
+    # SSSD supports numeric and string UIDs
+    # ensure that root is allowed to access IFP, might be 0 or root
+    if uids.isdisjoint({'0', 'root'}):
+        uids.add('root')
+    # allow IPA API to access IFP
+    uids.add(IPAAPI_USER)
+    service.set_option('allowed_uids', ', '.join(sorted(uids)))
+    sssdconfig.save_service(service)
 
 
 def change_ssh_config(filename, changes, sections):
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 96c95b7a07..698afd347e 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -23,7 +23,7 @@
 import ipalib.util
 import ipalib.errors
 from ipaclient.install import timeconf
-from ipaclient.install.client import sssd_enable_service
+from ipaclient.install.client import sssd_enable_ifp
 from ipaplatform import services
 from ipaplatform.tasks import tasks
 from ipapython import ipautil, version
@@ -1408,6 +1408,22 @@ def set_sssd_domain_option(option, value):
     sssdconfig.write(paths.SSSD_CONF)
 
 
+def sssd_update():
+    sssdconfig = SSSDConfig.SSSDConfig()
+    sssdconfig.import_config()
+    # upgrade domain
+    domain = sssdconfig.get_domain(str(api.env.domain))
+    domain.set_option('ipa_server_mode', 'True')
+    domain.set_option('ipa_server', api.env.host)
+    sssdconfig.save_domain(domain)
+    # enable and configure IFP plugin
+    sssd_enable_ifp(sssdconfig)
+    # write config and restart service
+    sssdconfig.write(paths.SSSD_CONF)
+    sssd = services.service('sssd', api)
+    sssd.restart()
+
+
 def remove_ds_ra_cert(subject_base):
     logger.info('[Removing RA cert from DS NSS database]')
 
@@ -2017,15 +2033,8 @@ def upgrade_configuration():
         cainstance.ensure_ipa_authority_entry()
 
     migrate_to_authselect()
-    set_sssd_domain_option('ipa_server_mode', 'True')
-    set_sssd_domain_option('ipa_server', api.env.host)
 
-    sssdconfig = SSSDConfig.SSSDConfig()
-    sssdconfig.import_config()
-    sssd_enable_service(sssdconfig, 'ifp')
-
-    sssd = services.service('sssd', api)
-    sssd.restart()
+    sssd_update()
 
     krb = krbinstance.KrbInstance(fstore)
     krb.fqdn = fqdn
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index 640eacfa06..1aa1bb3313 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -20,6 +20,8 @@
 from cryptography.hazmat.backends import default_backend
 from cryptography import x509
 
+from ipalib.constants import IPAAPI_USER
+
 from ipaplatform.paths import paths
 
 from ipatests.test_integration.base import IntegrationTest
@@ -28,6 +30,7 @@
 
 logger = logging.getLogger(__name__)
 
+
 class TestIPACommand(IntegrationTest):
     """
     A lot of commands can be executed against a single IPA installation
@@ -429,3 +432,31 @@ def test_certificate_out_write_to_file(self):
             x509.load_pem_x509_certificate(data, backend=default_backend())
 
             self.master.run_command(['rm', '-f', filename])
+
+    def test_sssd_ifp_access_ipaapi(self):
+        # check that ipaapi is allowed to access sssd-ifp for smartcard auth
+        # https://pagure.io/freeipa/issue/7751
+        username = 'admin'
+        # get UID for user
+        result = self.master.run_command(['ipa', 'user-show', username])
+        mo = re.search(r'UID: (\d+)', result.stdout_text)
+        assert mo is not None, result.stdout_text
+        uid = mo.group(1)
+
+        cmd = [
+            'dbus-send',
+            '--print-reply', '--system',
+            '--dest=org.freedesktop.sssd.infopipe',
+            '/org/freedesktop/sssd/infopipe/Users',
+            'org.freedesktop.sssd.infopipe.Users.FindByName',
+            'string:{}'.format(username)
+        ]
+        # test IFP as root
+        result = self.master.run_command(cmd)
+        assert uid in result.stdout_text
+
+        # test IFP as ipaapi
+        result = self.master.run_command(
+            ['sudo', '-u', IPAAPI_USER, '--'] + cmd
+        )
+        assert uid in result.stdout_text
From eb0136ea3438b6fb1145456478f401b9b7467cba Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Fri, 16 Nov 2018 14:11:16 +0100
Subject: [PATCH] Remove dead code

set_sssd_domain_option() is no longer used. Changes are handled by
sssd_update().

See: https://pagure.io/freeipa/issue/7751
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipaserver/install/server/upgrade.py | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index f1e78beb27..71bdd3670c 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1399,15 +1399,6 @@ def fix_schema_file_syntax():
     sysupgrade.set_upgrade_state('ds', 'fix_schema_syntax', True)
 
 
-def set_sssd_domain_option(option, value):
-    sssdconfig = SSSDConfig.SSSDConfig()
-    sssdconfig.import_config()
-    domain = sssdconfig.get_domain(str(api.env.domain))
-    domain.set_option(option, value)
-    sssdconfig.save_domain(domain)
-    sssdconfig.write(paths.SSSD_CONF)
-
-
 def sssd_update():
     sssdconfig = SSSDConfig.SSSDConfig()
     sssdconfig.import_config()
From 415295a6f68f4c797529e19a3f0cf956619d4bed Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Fri, 16 Nov 2018 14:51:23 +0100
Subject: [PATCH] Allow HTTPd user to access SSSD IFP

For smart card and certificate authentication, Apache's
mod_lookup_identity module must be able to acess SSSD IFP. The module
accesses IFP as Apache user, not as ipaapi user.

Apache is not allowed to use IFP by default. The update code uses the
service's ok-to-auth-as-delegate flag to detect smart card / cert auth.

See: https://pagure.io/freeipa/issue/7751
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipaclient/install/client.py         | 10 +++++++++-
 ipaserver/install/server/upgrade.py | 11 ++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index f9b003ef57..6125588802 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -47,6 +47,7 @@
     verify_host_resolvable,
 )
 from ipaplatform import services
+from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
 from ipapython import certdb, kernel_keyring, ipaldap, ipautil
@@ -1038,8 +1039,13 @@ def sssd_enable_service(sssdconfig, name):
     return sssdconfig.get_service(name)
 
 
-def sssd_enable_ifp(sssdconfig):
+def sssd_enable_ifp(sssdconfig, allow_httpd=False):
     """Enable and configure libsss_simpleifp plugin
+
+    Allow the ``ipaapi`` user to access IFP. In case allow_httpd is true,
+    the Apache HTTPd user is also allowed to access IFP. For smart card
+    authentication, mod_lookup_identity must be allowed to access user
+    information.
     """
     service = sssd_enable_service(sssdconfig, 'ifp')
     if service is None:
@@ -1058,6 +1064,8 @@ def sssd_enable_ifp(sssdconfig):
         uids.add('root')
     # allow IPA API to access IFP
     uids.add(IPAAPI_USER)
+    if allow_httpd:
+        uids.add(constants.HTTPD_USER)
     service.set_option('allowed_uids', ', '.join(sorted(uids)))
     sssdconfig.save_service(service)
 
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 71bdd3670c..4de7fd974d 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1407,8 +1407,17 @@ def sssd_update():
     domain.set_option('ipa_server_mode', 'True')
     domain.set_option('ipa_server', api.env.host)
     sssdconfig.save_domain(domain)
+    # check if service has ok_to_auth_as_delegate
+    service = 'HTTP/{}'.format(api.env.host)
+    result = api.Command.service_show(service, all=True)
+    flag = result['result'].get('ipakrboktoauthasdelegate', False)
+    if flag:
+        logger.debug(
+            "%s has ok_to_auth_as_delegate, allow Apache to access IFP",
+            services
+        )
     # enable and configure IFP plugin
-    sssd_enable_ifp(sssdconfig)
+    sssd_enable_ifp(sssdconfig, allow_httpd=flag)
     # write config and restart service
     sssdconfig.write(paths.SSSD_CONF)
     sssd = services.service('sssd', api)
From d7d17ece57ae1322c8368b7853f24d56b1d6a150 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Fri, 16 Nov 2018 14:54:32 +0100
Subject: [PATCH] Smart card auth advise: Allow Apache user

Modify the smard card auth advise script to use sssd_enable_ifp() in
order to allow Apache to access SSSD IFP.

See: https://pagure.io/freeipa/issue/7751
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipaserver/advise/plugins/smart_card_auth.py | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
index 97e23303b0..9a7a315ed5 100644
--- a/ipaserver/advise/plugins/smart_card_auth.py
+++ b/ipaserver/advise/plugins/smart_card_auth.py
@@ -105,6 +105,7 @@ class config_server_for_smart_card_auth(common_smart_card_auth_config):
     ssl_conf = paths.HTTPD_SSL_CONF
     ssl_ocsp_directive = OCSP_DIRECTIVE
     kdc_service_name = services.knownservices.krb5kdc.systemd_name
+    httpd_service_name = services.knownservices.httpd.systemd_name
 
     def get_info(self):
         self.log.exit_on_nonroot_euid()
@@ -117,6 +118,7 @@ def get_info(self):
         self.record_httpd_ocsp_status()
         self.check_and_enable_pkinit()
         self.enable_ok_to_auth_as_delegate_on_http_principal()
+        self.allow_httpd_ifp()
         self.upload_smartcard_ca_certificates_to_systemwide_db()
         self.install_smart_card_signing_ca_certs()
         self.update_ipa_ca_certificate_store()
@@ -183,7 +185,9 @@ def _format_command(self, fmt_line, directive, filename):
 
     def restart_httpd(self):
         self.log.comment('finally restart apache')
-        self.log.command('systemctl restart httpd')
+        self.log.command(
+            'systemctl restart {}'.format(self.httpd_service_name)
+        )
 
     def record_httpd_ocsp_status(self):
         self.log.comment('store the OCSP upgrade state')
@@ -214,6 +218,21 @@ def enable_ok_to_auth_as_delegate_on_http_principal(self):
             ["Failed to set OK_AS_AUTH_AS_DELEGATE flag on HTTP principal"]
         )
 
+    def allow_httpd_ifp(self):
+        self.log.comment('Allow Apache to access SSSD IFP')
+        self.log.exit_on_failed_command(
+            '{} -c "import SSSDConfig; '
+            'from ipaclient.install.client import sssd_enable_ifp; '
+            'from ipaplatform.paths import paths; '
+            'c = SSSDConfig.SSSDConfig(); '
+            'c.import_config(); '
+            'sssd_enable_ifp(c, allow_httpd=True); '
+            'c.write(paths.SSSD_CONF)"'.format(sys.executable),
+            ['Failed to modify SSSD config']
+        )
+        self.log.comment('Restart sssd')
+        self.log.command('systemctl restart sssd')
+
     def restart_kdc(self):
         self.log.exit_on_failed_command(
             'systemctl restart {}'.format(self.kdc_service_name),
From b56db8daa704782c44683412b85a454654eabc19 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Mon, 19 Nov 2018 14:19:16 +0100
Subject: [PATCH] Log stderr in run_command

pytest_multihost's run_command() does not log stderr when a command
fails. Wrap the function call to log stderr so it's easier to debug
failing tests.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipatests/pytest_ipa/integration/host.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
index 28f6e2cd32..6aed58ae96 100644
--- a/ipatests/pytest_ipa/integration/host.py
+++ b/ipatests/pytest_ipa/integration/host.py
@@ -18,6 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 """Host class for integration testing"""
+import subprocess
 
 import pytest_multihost.host
 
@@ -60,6 +61,24 @@ def to_env(self, **kwargs):
         from ipatests.pytest_ipa.integration.env_config import host_to_env
         return host_to_env(self, **kwargs)
 
+    def run_command(self, argv, set_env=True, stdin_text=None,
+                    log_stdout=True, raiseonerr=True,
+                    cwd=None, bg=False, encoding='utf-8'):
+        # Wrap run_command to log stderr on raiseonerr=True
+        result = super().run_command(
+            argv, set_env=set_env, stdin_text=stdin_text,
+            log_stdout=log_stdout, raiseonerr=False, cwd=cwd, bg=bg,
+            encoding=encoding
+        )
+        if result.returncode and raiseonerr:
+            result.log.error('stderr: %s', result.stderr_text)
+            raise subprocess.CalledProcessError(
+                result.returncode, argv,
+                result.stdout_text, result.stderr_text
+            )
+        else:
+            return result
+
 
 class WinHost(pytest_multihost.host.WinHost):
     """
From 97776d2c4eed5de73780476bb11a635a2e47ebc5 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Wed, 21 Nov 2018 10:00:20 +0100
Subject: [PATCH] Test smart card advise scripts

Create and execute the server and client smart card advise scripts.

See: See: https://pagure.io/freeipa/issue/7751
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipatests/prci_definitions/gating.yaml    |  2 +-
 ipatests/test_integration/test_advise.py | 99 ++++++++++++++++++------
 2 files changed, 75 insertions(+), 26 deletions(-)

diff --git a/ipatests/prci_definitions/gating.yaml b/ipatests/prci_definitions/gating.yaml
index 7a9b612dea..13f8851d02 100644
--- a/ipatests/prci_definitions/gating.yaml
+++ b/ipatests/prci_definitions/gating.yaml
@@ -157,7 +157,7 @@ jobs:
         test_suite: test_integration/test_advise.py
         template: *ci-master-f29
         timeout: 3600
-        topology: *master_1repl
+        topology: *master_1repl_1client
 
   fedora-29/test_testconfig:
     requires: [fedora-29/build]
diff --git a/ipatests/test_integration/test_advise.py b/ipatests/test_integration/test_advise.py
index 3b821c8797..b548614922 100644
--- a/ipatests/test_integration/test_advise.py
+++ b/ipatests/test_integration/test_advise.py
@@ -21,11 +21,17 @@
 # pylint: disable=no-member
 
 import re
+
+from ipalib.constants import IPAAPI_USER
+from ipaplatform.paths import paths
+from ipaplatform.constants import constants
+
+from ipatests.create_external_ca import ExternalCA
 from ipatests.pytest_ipa.integration import tasks
 from ipatests.test_integration.base import IntegrationTest
 
 
-def run_advice(master, advice_id, advice_regex, raiseerr):
+def run_advice(master, advice_id, advice_regex, raiseerr=True):
     # Obtain the advice from the server
     tasks.kinit_admin(master)
     result = master.run_command(['ipa-advise', advice_id],
@@ -43,28 +49,38 @@ class TestAdvice(IntegrationTest):
     """
     Tests ipa-advise output.
     """
-    advice_id = None
-    raiseerr = None
-    advice_regex = ''
     topology = 'line'
+    num_replicas = 0
+    num_clients = 1
+
+    def execute_advise(self, host, advice_id, *args):
+        # ipa-advise script is only available on a server
+        tasks.kinit_admin(self.master)
+        advice = self.master.run_command(['ipa-advise', advice_id])
+        # execute script on host (client or master)
+        if host is not self.master:
+            tasks.kinit_admin(host)
+        filename = tasks.upload_temp_contents(host, advice.stdout_text)
+        cmd = ['sh', filename]
+        cmd.extend(args)
+        try:
+            result = host.run_command(cmd)
+        finally:
+            host.run_command(['rm', '-f', filename])
+        return advice, result
 
     def test_invalid_advice(self):
         advice_id = r'invalid-advise-param'
         advice_regex = r"invalid[\s]+\'advice\'.*"
-        raiseerr = False
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
-
+        run_advice(self.master, advice_id, advice_regex, raiseerr=False)
 
     def test_advice_FreeBSDNSSPAM(self):
         advice_id = 'config-freebsd-nss-pam-ldapd'
         advice_regex = r"\#\!\/bin\/sh.*" \
                        r"pkg_add[\s]+\-r[\s]+nss\-pam\-ldapd[\s]+curl.*" \
                        r"\/usr\/local\/etc\/rc\.d\/nslcd[\s]+restart"
-        raiseerr = True
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
 
+        run_advice(self.master, advice_id, advice_regex)
 
     def test_advice_GenericNSSPAM(self):
         advice_id = 'config-generic-linux-nss-pam-ldapd'
@@ -75,20 +91,16 @@ def test_advice_GenericNSSPAM(self):
             r"service[\s]+nscd[\s]+stop[\s]+\&\&[\s]+service[\s]+"
             r"nslcd[\s]+restart"
         )
-        raiseerr = True
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
 
+        run_advice(self.master, advice_id, advice_regex)
 
     def test_advice_GenericSSSDBefore19(self):
         advice_id = r'config-generic-linux-sssd-before-1-9'
         advice_regex = r"\#\!\/bin\/sh.*" \
                        r"apt\-get[\s]+\-y[\s]+install sssd curl openssl.*" \
                        r"service[\s]+sssd[\s]+start"
-        raiseerr = True
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
 
+        run_advice(self.master, advice_id, advice_regex)
 
     def test_advice_RedHatNSS(self):
         advice_id = 'config-redhat-nss-ldap'
@@ -100,10 +112,8 @@ def test_advice_RedHatNSS(self):
             r"[\s]+\-\-enableldapauth[\s]+"
             r"\-\-ldapserver=.*[\s]+\-\-ldapbasedn=.*"
         )
-        raiseerr = True
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
 
+        run_advice(self.master, advice_id, advice_regex)
 
     def test_advice_RedHatNSSPAM(self):
         advice_id = 'config-redhat-nss-pam-ldapd'
@@ -113,10 +123,8 @@ def test_advice_RedHatNSSPAM(self):
                        r"authconfig[\s]+\-\-updateall[\s]+\-\-enableldap"\
                        r"[\s]+\-\-enableldaptls[\s]+\-\-enableldapauth[\s]+" \
                        r"\-\-ldapserver=.*[\s]+\-\-ldapbasedn=.*"
-        raiseerr = True
-
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
 
+        run_advice(self.master, advice_id, advice_regex)
 
     def test_advice_RedHatSSSDBefore19(self):
         advice_id = 'config-redhat-sssd-before-1-9'
@@ -125,6 +133,47 @@ def test_advice_RedHatSSSDBefore19(self):
             r"yum[\s]+install[\s]+\-y[\s]+sssd[\s]+authconfig[\s]+"
             r"curl[\s]+openssl.*service[\s]+sssd[\s]+start"
         )
-        raiseerr = True
 
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
+        run_advice(self.master, advice_id, advice_regex)
+
+    # trivial checks
+    def test_advice_enable_admins_sudo(self):
+        advice_id = 'enable_admins_sudo'
+        advice_regex = r"\#\!\/bin\/sh.*"
+        run_advice(self.master, advice_id, advice_regex)
+
+    def test_advice_config_server_for_smart_card_auth(self):
+        advice_id = 'config_server_for_smart_card_auth'
+        advice_regex = r"\#\!\/bin\/sh.*"
+        run_advice(self.master, advice_id, advice_regex)
+
+        ca_pem = ExternalCA().create_ca()
+        ca_file = tasks.upload_temp_contents(self.master, ca_pem)
+        try:
+            self.execute_advise(self.master, advice_id, ca_file)
+        except Exception:
+            # debug: sometimes ipa-certupdate times out in
+            # "Resubmitting certmonger request"
+            self.master.run_command(['getcert', 'list'])
+            raise
+        finally:
+            self.master.run_command(['rm', '-f', ca_file])
+        sssd_conf = self.master.get_file_contents(
+            paths.SSSD_CONF, encoding='utf-8'
+        )
+        assert constants.HTTPD_USER in sssd_conf
+        assert IPAAPI_USER in sssd_conf
+
+    def test_advice_config_client_for_smart_card_auth(self):
+        advice_id = 'config_client_for_smart_card_auth'
+        advice_regex = r"\#\!\/bin\/sh.*"
+        run_advice(self.master, advice_id, advice_regex)
+
+        client = self.clients[0]
+
+        ca_pem = ExternalCA().create_ca()
+        ca_file = tasks.upload_temp_contents(client, ca_pem)
+        try:
+            self.execute_advise(client, advice_id, ca_file)
+        finally:
+            client.run_command(['rm', '-f', ca_file])
From 6ed90a2ac08c070e8e5c47a1eb3c52d7d30cabb8 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Wed, 21 Nov 2018 10:44:55 +0100
Subject: [PATCH] Add install/remove package helpers to advise

The smart card advise scripts assume that yum is installed. However
Fedora has dnf and the yum wrapper is not installed by default.
Installation and removal of packages is now provided by two helper
methods that detect the package manager.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipaserver/advise/base.py                    | 36 +++++++++++++++++++++
 ipaserver/advise/plugins/smart_card_auth.py | 24 +++++++-------
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
index 07b1431e84..ec65113e34 100644
--- a/ipaserver/advise/base.py
+++ b/ipaserver/advise/base.py
@@ -227,6 +227,7 @@ def __init__(self):
         self.content = []
         self.prefix = '# '
         self.options = None
+        self.pkgmgr_detected = False
         self._indentation_tracker = _IndentationTracker(
             spaces_per_indent=DEFAULT_INDENTATION_INCREMENT)
 
@@ -312,6 +313,41 @@ def exit_on_predicate(self, predicate, error_message_lines):
 
             self.command('exit 1')
 
+    def detect_pkgmgr(self):
+        self.commands_on_predicate(
+            'which yum >/dev/null',
+            commands_to_run_when_true=['PKGMGR=yum'],
+            commands_to_run_when_false=['PKGMGR=dnf']
+        )
+        self.pkgmgr_detected = True
+
+    def install_packages(self, names, error_message_lines):
+        assert isinstance(names, list)
+        self.detect_pkgmgr()
+        self.command('rpm -qi {} > /dev/null'.format(' '.join(names)))
+        self.commands_on_predicate(
+            '[ "$?" -ne "0" ]',
+            ['$PKGMGR install -y {}'.format(' '.join(names))]
+        )
+        self.exit_on_predicate(
+            '[ "$?" -ne "0" ]',
+            error_message_lines
+        )
+
+    def remove_package(self, name, error_message_lines):
+        # remove only supports one package name
+        assert ' ' not in name
+        self.detect_pkgmgr()
+        self.command('rpm -qi {} > /dev/null'.format(name))
+        self.commands_on_predicate(
+            '[ "$?" -eq "0" ]',
+            ['$PKGMGR remove -y {} || exit 1'.format(name)]
+        )
+        self.exit_on_predicate(
+            '[ "$?" -ne "0" ]',
+            error_message_lines
+        )
+
     @contextmanager
     def unbranched_if(self, predicate):
         with self._compound_statement(UnbranchedIfStatement, predicate):
diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
index 9a7a315ed5..411124f935 100644
--- a/ipaserver/advise/plugins/smart_card_auth.py
+++ b/ipaserver/advise/plugins/smart_card_auth.py
@@ -135,9 +135,10 @@ def resolve_ipaca_records(self):
 
         self.log.comment('make sure bind-utils are installed so that we can '
                          'dig for ipa-ca records')
-        self.log.exit_on_failed_command(
-            'yum install -y bind-utils',
-            ['Failed to install bind-utils'])
+        self.log.install_packages(
+            ['bind-utils'],
+            ['Failed to install bind-utils']
+        )
 
         self.log.comment('make sure ipa-ca records are resolvable, '
                          'otherwise error out and instruct')
@@ -272,26 +273,23 @@ def get_info(self):
         self.restart_sssd()
 
     def check_and_remove_pam_pkcs11(self):
-        self.log.command('rpm -qi pam_pkcs11 > /dev/null')
-        self.log.commands_on_predicate(
-            '[ "$?" -eq "0" ]',
-            [
-                'yum remove -y pam_pkcs11'
-            ]
+        self.log.remove_package(
+            'pam_pkcs11',
+            ['Could not remove pam_pkcs11 package']
         )
 
     def install_opensc_and_dconf_packages(self):
         self.log.comment(
             'authconfig often complains about missing dconf, '
             'install it explicitly')
-        self.log.exit_on_failed_command(
-            'yum install -y {} dconf'.format(self.opensc_module_name.lower()),
+        self.log.install_packages(
+            [self.opensc_module_name.lower(), 'dconf'],
             ['Could not install OpenSC package']
         )
 
     def install_krb5_client_dependencies(self):
-        self.log.exit_on_failed_command(
-            'yum install -y krb5-pkinit-openssl',
+        self.log.install_packages(
+            ['krb5-pkinit-openssl'],
             ['Failed to install Kerberos client PKINIT extensions.']
         )
 
From e05ce4a20d2395179580db7e3db75c601c8f364c Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Thu, 13 Dec 2018 14:40:44 +0100
Subject: [PATCH] Python 2 compatibility

Make new test helpers and test code compatible with Python 2.7.

See: https://pagure.io/freeipa/issue/7751
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 ipatests/pytest_ipa/integration/host.py  | 4 ++--
 ipatests/test_integration/test_advise.py | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
index 6aed58ae96..eb05872467 100644
--- a/ipatests/pytest_ipa/integration/host.py
+++ b/ipatests/pytest_ipa/integration/host.py
@@ -65,7 +65,7 @@ def run_command(self, argv, set_env=True, stdin_text=None,
                     log_stdout=True, raiseonerr=True,
                     cwd=None, bg=False, encoding='utf-8'):
         # Wrap run_command to log stderr on raiseonerr=True
-        result = super().run_command(
+        result = super(Host, self).run_command(
             argv, set_env=set_env, stdin_text=stdin_text,
             log_stdout=log_stdout, raiseonerr=False, cwd=cwd, bg=bg,
             encoding=encoding
@@ -74,7 +74,7 @@ def run_command(self, argv, set_env=True, stdin_text=None,
             result.log.error('stderr: %s', result.stderr_text)
             raise subprocess.CalledProcessError(
                 result.returncode, argv,
-                result.stdout_text, result.stderr_text
+                result.stderr_text
             )
         else:
             return result
diff --git a/ipatests/test_integration/test_advise.py b/ipatests/test_integration/test_advise.py
index b548614922..761f278238 100644
--- a/ipatests/test_integration/test_advise.py
+++ b/ipatests/test_integration/test_advise.py
@@ -20,6 +20,8 @@
 # FIXME: Pylint errors
 # pylint: disable=no-member
 
+from __future__ import absolute_import
+
 import re
 
 from ipalib.constants import IPAAPI_USER