Blame SOURCES/0014-Allow_ipaapi_and_Apache_user_to_access_SSSD_IFP_rhbz#1639910.patch

b01884
From 785c496dceb76a8f628249ce598e0540b1dfec6e Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Tue, 6 Nov 2018 13:57:14 +0100
b01884
Subject: [PATCH] Allow ipaapi user to access SSSD's info pipe
b01884
b01884
For smart card authentication, ipaapi must be able to access to sss-ifp.
b01884
During installation and upgrade, the ipaapi user is now added to
b01884
[ifp]allowed_uids.
b01884
b01884
The commit also fixes two related issues:
b01884
b01884
* The server upgrade code now enables ifp service in sssd.conf. The
b01884
  existing code modified sssd.conf but never wrote the changes to disk.
b01884
* sssd_enable_service() no longer fails after it has detected an
b01884
  unrecognized service.
b01884
b01884
Fixes: https://pagure.io/freeipa/issue/7751
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipaclient/install/client.py                | 41 ++++++++++++++++++----
b01884
 ipaserver/install/server/upgrade.py        | 27 +++++++++-----
b01884
 ipatests/test_integration/test_commands.py | 31 ++++++++++++++++
b01884
 3 files changed, 83 insertions(+), 16 deletions(-)
b01884
b01884
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
b01884
index 05255fe61b..f9b003ef57 100644
b01884
--- a/ipaclient/install/client.py
b01884
+++ b/ipaclient/install/client.py
b01884
@@ -35,6 +35,7 @@
b01884
 # pylint: enable=import-error
b01884
 
b01884
 from ipalib import api, errors, x509
b01884
+from ipalib.constants import IPAAPI_USER
b01884
 from ipalib.install import certmonger, certstore, service, sysrestore
b01884
 from ipalib.install import hostname as hostname_
b01884
 from ipalib.install.kinit import kinit_keytab, kinit_password
b01884
@@ -914,7 +915,7 @@ def configure_sssd_conf(
b01884
         domain = sssdconfig.new_domain(cli_domain)
b01884
 
b01884
     if options.on_master:
b01884
-        sssd_enable_service(sssdconfig, 'ifp')
b01884
+        sssd_enable_ifp(sssdconfig)
b01884
 
b01884
     if (
b01884
         (options.conf_ssh and os.path.isfile(paths.SSH_CONFIG)) or
b01884
@@ -1018,21 +1019,47 @@ def configure_sssd_conf(
b01884
     return 0
b01884
 
b01884
 
b01884
-def sssd_enable_service(sssdconfig, service):
b01884
+def sssd_enable_service(sssdconfig, name):
b01884
     try:
b01884
-        sssdconfig.new_service(service)
b01884
+        sssdconfig.new_service(name)
b01884
     except SSSDConfig.ServiceAlreadyExists:
b01884
         pass
b01884
     except SSSDConfig.ServiceNotRecognizedError:
b01884
         logger.error(
b01884
-            "Unable to activate the %s service in SSSD config.", service)
b01884
+            "Unable to activate the '%s' service in SSSD config.", name)
b01884
         logger.info(
b01884
             "Please make sure you have SSSD built with %s support "
b01884
-            "installed.", service)
b01884
+            "installed.", name)
b01884
         logger.info(
b01884
-            "Configure %s support manually in /etc/sssd/sssd.conf.", service)
b01884
+            "Configure %s support manually in /etc/sssd/sssd.conf.", name)
b01884
+        return None
b01884
 
b01884
-    sssdconfig.activate_service(service)
b01884
+    sssdconfig.activate_service(name)
b01884
+    return sssdconfig.get_service(name)
b01884
+
b01884
+
b01884
+def sssd_enable_ifp(sssdconfig):
b01884
+    """Enable and configure libsss_simpleifp plugin
b01884
+    """
b01884
+    service = sssd_enable_service(sssdconfig, 'ifp')
b01884
+    if service is None:
b01884
+        # unrecognized service
b01884
+        return
b01884
+
b01884
+    try:
b01884
+        uids = service.get_option('allowed_uids')
b01884
+    except SSSDConfig.NoOptionError:
b01884
+        uids = set()
b01884
+    else:
b01884
+        uids = {s.strip() for s in uids.split(',') if s.strip()}
b01884
+    # SSSD supports numeric and string UIDs
b01884
+    # ensure that root is allowed to access IFP, might be 0 or root
b01884
+    if uids.isdisjoint({'0', 'root'}):
b01884
+        uids.add('root')
b01884
+    # allow IPA API to access IFP
b01884
+    uids.add(IPAAPI_USER)
b01884
+    service.set_option('allowed_uids', ', '.join(sorted(uids)))
b01884
+    sssdconfig.save_service(service)
b01884
 
b01884
 
b01884
 def change_ssh_config(filename, changes, sections):
b01884
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
b01884
index 96c95b7a07..698afd347e 100644
b01884
--- a/ipaserver/install/server/upgrade.py
b01884
+++ b/ipaserver/install/server/upgrade.py
b01884
@@ -23,7 +23,7 @@
b01884
 import ipalib.util
b01884
 import ipalib.errors
b01884
 from ipaclient.install import timeconf
b01884
-from ipaclient.install.client import sssd_enable_service
b01884
+from ipaclient.install.client import sssd_enable_ifp
b01884
 from ipaplatform import services
b01884
 from ipaplatform.tasks import tasks
b01884
 from ipapython import ipautil, version
b01884
@@ -1408,6 +1408,22 @@ def set_sssd_domain_option(option, value):
b01884
     sssdconfig.write(paths.SSSD_CONF)
b01884
 
b01884
 
b01884
+def sssd_update():
b01884
+    sssdconfig = SSSDConfig.SSSDConfig()
b01884
+    sssdconfig.import_config()
b01884
+    # upgrade domain
b01884
+    domain = sssdconfig.get_domain(str(api.env.domain))
b01884
+    domain.set_option('ipa_server_mode', 'True')
b01884
+    domain.set_option('ipa_server', api.env.host)
b01884
+    sssdconfig.save_domain(domain)
b01884
+    # enable and configure IFP plugin
b01884
+    sssd_enable_ifp(sssdconfig)
b01884
+    # write config and restart service
b01884
+    sssdconfig.write(paths.SSSD_CONF)
b01884
+    sssd = services.service('sssd', api)
b01884
+    sssd.restart()
b01884
+
b01884
+
b01884
 def remove_ds_ra_cert(subject_base):
b01884
     logger.info('[Removing RA cert from DS NSS database]')
b01884
 
b01884
@@ -2017,15 +2033,8 @@ def upgrade_configuration():
b01884
         cainstance.ensure_ipa_authority_entry()
b01884
 
b01884
     migrate_to_authselect()
b01884
-    set_sssd_domain_option('ipa_server_mode', 'True')
b01884
-    set_sssd_domain_option('ipa_server', api.env.host)
b01884
 
b01884
-    sssdconfig = SSSDConfig.SSSDConfig()
b01884
-    sssdconfig.import_config()
b01884
-    sssd_enable_service(sssdconfig, 'ifp')
b01884
-
b01884
-    sssd = services.service('sssd', api)
b01884
-    sssd.restart()
b01884
+    sssd_update()
b01884
 
b01884
     krb = krbinstance.KrbInstance(fstore)
b01884
     krb.fqdn = fqdn
b01884
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
b01884
index 640eacfa06..1aa1bb3313 100644
b01884
--- a/ipatests/test_integration/test_commands.py
b01884
+++ b/ipatests/test_integration/test_commands.py
b01884
@@ -20,6 +20,8 @@
b01884
 from cryptography.hazmat.backends import default_backend
b01884
 from cryptography import x509
b01884
 
b01884
+from ipalib.constants import IPAAPI_USER
b01884
+
b01884
 from ipaplatform.paths import paths
b01884
 
b01884
 from ipatests.test_integration.base import IntegrationTest
b01884
@@ -28,6 +30,7 @@
b01884
 
b01884
 logger = logging.getLogger(__name__)
b01884
 
b01884
+
b01884
 class TestIPACommand(IntegrationTest):
b01884
     """
b01884
     A lot of commands can be executed against a single IPA installation
b01884
@@ -429,3 +432,31 @@ def test_certificate_out_write_to_file(self):
b01884
             x509.load_pem_x509_certificate(data, backend=default_backend())
b01884
 
b01884
             self.master.run_command(['rm', '-f', filename])
b01884
+
b01884
+    def test_sssd_ifp_access_ipaapi(self):
b01884
+        # check that ipaapi is allowed to access sssd-ifp for smartcard auth
b01884
+        # https://pagure.io/freeipa/issue/7751
b01884
+        username = 'admin'
b01884
+        # get UID for user
b01884
+        result = self.master.run_command(['ipa', 'user-show', username])
b01884
+        mo = re.search(r'UID: (\d+)', result.stdout_text)
b01884
+        assert mo is not None, result.stdout_text
b01884
+        uid = mo.group(1)
b01884
+
b01884
+        cmd = [
b01884
+            'dbus-send',
b01884
+            '--print-reply', '--system',
b01884
+            '--dest=org.freedesktop.sssd.infopipe',
b01884
+            '/org/freedesktop/sssd/infopipe/Users',
b01884
+            'org.freedesktop.sssd.infopipe.Users.FindByName',
b01884
+            'string:{}'.format(username)
b01884
+        ]
b01884
+        # test IFP as root
b01884
+        result = self.master.run_command(cmd)
b01884
+        assert uid in result.stdout_text
b01884
+
b01884
+        # test IFP as ipaapi
b01884
+        result = self.master.run_command(
b01884
+            ['sudo', '-u', IPAAPI_USER, '--'] + cmd
b01884
+        )
b01884
+        assert uid in result.stdout_text
b01884
From eb0136ea3438b6fb1145456478f401b9b7467cba Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Fri, 16 Nov 2018 14:11:16 +0100
b01884
Subject: [PATCH] Remove dead code
b01884
b01884
set_sssd_domain_option() is no longer used. Changes are handled by
b01884
sssd_update().
b01884
b01884
See: https://pagure.io/freeipa/issue/7751
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipaserver/install/server/upgrade.py | 9 ---------
b01884
 1 file changed, 9 deletions(-)
b01884
b01884
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
b01884
index f1e78beb27..71bdd3670c 100644
b01884
--- a/ipaserver/install/server/upgrade.py
b01884
+++ b/ipaserver/install/server/upgrade.py
b01884
@@ -1399,15 +1399,6 @@ def fix_schema_file_syntax():
b01884
     sysupgrade.set_upgrade_state('ds', 'fix_schema_syntax', True)
b01884
 
b01884
 
b01884
-def set_sssd_domain_option(option, value):
b01884
-    sssdconfig = SSSDConfig.SSSDConfig()
b01884
-    sssdconfig.import_config()
b01884
-    domain = sssdconfig.get_domain(str(api.env.domain))
b01884
-    domain.set_option(option, value)
b01884
-    sssdconfig.save_domain(domain)
b01884
-    sssdconfig.write(paths.SSSD_CONF)
b01884
-
b01884
-
b01884
 def sssd_update():
b01884
     sssdconfig = SSSDConfig.SSSDConfig()
b01884
     sssdconfig.import_config()
b01884
From 415295a6f68f4c797529e19a3f0cf956619d4bed Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Fri, 16 Nov 2018 14:51:23 +0100
b01884
Subject: [PATCH] Allow HTTPd user to access SSSD IFP
b01884
b01884
For smart card and certificate authentication, Apache's
b01884
mod_lookup_identity module must be able to acess SSSD IFP. The module
b01884
accesses IFP as Apache user, not as ipaapi user.
b01884
b01884
Apache is not allowed to use IFP by default. The update code uses the
b01884
service's ok-to-auth-as-delegate flag to detect smart card / cert auth.
b01884
b01884
See: https://pagure.io/freeipa/issue/7751
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipaclient/install/client.py         | 10 +++++++++-
b01884
 ipaserver/install/server/upgrade.py | 11 ++++++++++-
b01884
 2 files changed, 19 insertions(+), 2 deletions(-)
b01884
b01884
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
b01884
index f9b003ef57..6125588802 100644
b01884
--- a/ipaclient/install/client.py
b01884
+++ b/ipaclient/install/client.py
b01884
@@ -47,6 +47,7 @@
b01884
     verify_host_resolvable,
b01884
 )
b01884
 from ipaplatform import services
b01884
+from ipaplatform.constants import constants
b01884
 from ipaplatform.paths import paths
b01884
 from ipaplatform.tasks import tasks
b01884
 from ipapython import certdb, kernel_keyring, ipaldap, ipautil
b01884
@@ -1038,8 +1039,13 @@ def sssd_enable_service(sssdconfig, name):
b01884
     return sssdconfig.get_service(name)
b01884
 
b01884
 
b01884
-def sssd_enable_ifp(sssdconfig):
b01884
+def sssd_enable_ifp(sssdconfig, allow_httpd=False):
b01884
     """Enable and configure libsss_simpleifp plugin
b01884
+
b01884
+    Allow the ``ipaapi`` user to access IFP. In case allow_httpd is true,
b01884
+    the Apache HTTPd user is also allowed to access IFP. For smart card
b01884
+    authentication, mod_lookup_identity must be allowed to access user
b01884
+    information.
b01884
     """
b01884
     service = sssd_enable_service(sssdconfig, 'ifp')
b01884
     if service is None:
b01884
@@ -1058,6 +1064,8 @@ def sssd_enable_ifp(sssdconfig):
b01884
         uids.add('root')
b01884
     # allow IPA API to access IFP
b01884
     uids.add(IPAAPI_USER)
b01884
+    if allow_httpd:
b01884
+        uids.add(constants.HTTPD_USER)
b01884
     service.set_option('allowed_uids', ', '.join(sorted(uids)))
b01884
     sssdconfig.save_service(service)
b01884
 
b01884
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
b01884
index 71bdd3670c..4de7fd974d 100644
b01884
--- a/ipaserver/install/server/upgrade.py
b01884
+++ b/ipaserver/install/server/upgrade.py
b01884
@@ -1407,8 +1407,17 @@ def sssd_update():
b01884
     domain.set_option('ipa_server_mode', 'True')
b01884
     domain.set_option('ipa_server', api.env.host)
b01884
     sssdconfig.save_domain(domain)
b01884
+    # check if service has ok_to_auth_as_delegate
b01884
+    service = 'HTTP/{}'.format(api.env.host)
b01884
+    result = api.Command.service_show(service, all=True)
b01884
+    flag = result['result'].get('ipakrboktoauthasdelegate', False)
b01884
+    if flag:
b01884
+        logger.debug(
b01884
+            "%s has ok_to_auth_as_delegate, allow Apache to access IFP",
b01884
+            services
b01884
+        )
b01884
     # enable and configure IFP plugin
b01884
-    sssd_enable_ifp(sssdconfig)
b01884
+    sssd_enable_ifp(sssdconfig, allow_httpd=flag)
b01884
     # write config and restart service
b01884
     sssdconfig.write(paths.SSSD_CONF)
b01884
     sssd = services.service('sssd', api)
b01884
From d7d17ece57ae1322c8368b7853f24d56b1d6a150 Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Fri, 16 Nov 2018 14:54:32 +0100
b01884
Subject: [PATCH] Smart card auth advise: Allow Apache user
b01884
b01884
Modify the smard card auth advise script to use sssd_enable_ifp() in
b01884
order to allow Apache to access SSSD IFP.
b01884
b01884
See: https://pagure.io/freeipa/issue/7751
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipaserver/advise/plugins/smart_card_auth.py | 21 ++++++++++++++++++++-
b01884
 1 file changed, 20 insertions(+), 1 deletion(-)
b01884
b01884
diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
b01884
index 97e23303b0..9a7a315ed5 100644
b01884
--- a/ipaserver/advise/plugins/smart_card_auth.py
b01884
+++ b/ipaserver/advise/plugins/smart_card_auth.py
b01884
@@ -105,6 +105,7 @@ class config_server_for_smart_card_auth(common_smart_card_auth_config):
b01884
     ssl_conf = paths.HTTPD_SSL_CONF
b01884
     ssl_ocsp_directive = OCSP_DIRECTIVE
b01884
     kdc_service_name = services.knownservices.krb5kdc.systemd_name
b01884
+    httpd_service_name = services.knownservices.httpd.systemd_name
b01884
 
b01884
     def get_info(self):
b01884
         self.log.exit_on_nonroot_euid()
b01884
@@ -117,6 +118,7 @@ def get_info(self):
b01884
         self.record_httpd_ocsp_status()
b01884
         self.check_and_enable_pkinit()
b01884
         self.enable_ok_to_auth_as_delegate_on_http_principal()
b01884
+        self.allow_httpd_ifp()
b01884
         self.upload_smartcard_ca_certificates_to_systemwide_db()
b01884
         self.install_smart_card_signing_ca_certs()
b01884
         self.update_ipa_ca_certificate_store()
b01884
@@ -183,7 +185,9 @@ def _format_command(self, fmt_line, directive, filename):
b01884
 
b01884
     def restart_httpd(self):
b01884
         self.log.comment('finally restart apache')
b01884
-        self.log.command('systemctl restart httpd')
b01884
+        self.log.command(
b01884
+            'systemctl restart {}'.format(self.httpd_service_name)
b01884
+        )
b01884
 
b01884
     def record_httpd_ocsp_status(self):
b01884
         self.log.comment('store the OCSP upgrade state')
b01884
@@ -214,6 +218,21 @@ def enable_ok_to_auth_as_delegate_on_http_principal(self):
b01884
             ["Failed to set OK_AS_AUTH_AS_DELEGATE flag on HTTP principal"]
b01884
         )
b01884
 
b01884
+    def allow_httpd_ifp(self):
b01884
+        self.log.comment('Allow Apache to access SSSD IFP')
b01884
+        self.log.exit_on_failed_command(
b01884
+            '{} -c "import SSSDConfig; '
b01884
+            'from ipaclient.install.client import sssd_enable_ifp; '
b01884
+            'from ipaplatform.paths import paths; '
b01884
+            'c = SSSDConfig.SSSDConfig(); '
b01884
+            'c.import_config(); '
b01884
+            'sssd_enable_ifp(c, allow_httpd=True); '
b01884
+            'c.write(paths.SSSD_CONF)"'.format(sys.executable),
b01884
+            ['Failed to modify SSSD config']
b01884
+        )
b01884
+        self.log.comment('Restart sssd')
b01884
+        self.log.command('systemctl restart sssd')
b01884
+
b01884
     def restart_kdc(self):
b01884
         self.log.exit_on_failed_command(
b01884
             'systemctl restart {}'.format(self.kdc_service_name),
b01884
From b56db8daa704782c44683412b85a454654eabc19 Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Mon, 19 Nov 2018 14:19:16 +0100
b01884
Subject: [PATCH] Log stderr in run_command
b01884
b01884
pytest_multihost's run_command() does not log stderr when a command
b01884
fails. Wrap the function call to log stderr so it's easier to debug
b01884
failing tests.
b01884
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipatests/pytest_ipa/integration/host.py | 19 +++++++++++++++++++
b01884
 1 file changed, 19 insertions(+)
b01884
b01884
diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
b01884
index 28f6e2cd32..6aed58ae96 100644
b01884
--- a/ipatests/pytest_ipa/integration/host.py
b01884
+++ b/ipatests/pytest_ipa/integration/host.py
b01884
@@ -18,6 +18,7 @@
b01884
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
b01884
 
b01884
 """Host class for integration testing"""
b01884
+import subprocess
b01884
 
b01884
 import pytest_multihost.host
b01884
 
b01884
@@ -60,6 +61,24 @@ def to_env(self, **kwargs):
b01884
         from ipatests.pytest_ipa.integration.env_config import host_to_env
b01884
         return host_to_env(self, **kwargs)
b01884
 
b01884
+    def run_command(self, argv, set_env=True, stdin_text=None,
b01884
+                    log_stdout=True, raiseonerr=True,
b01884
+                    cwd=None, bg=False, encoding='utf-8'):
b01884
+        # Wrap run_command to log stderr on raiseonerr=True
b01884
+        result = super().run_command(
b01884
+            argv, set_env=set_env, stdin_text=stdin_text,
b01884
+            log_stdout=log_stdout, raiseonerr=False, cwd=cwd, bg=bg,
b01884
+            encoding=encoding
b01884
+        )
b01884
+        if result.returncode and raiseonerr:
b01884
+            result.log.error('stderr: %s', result.stderr_text)
b01884
+            raise subprocess.CalledProcessError(
b01884
+                result.returncode, argv,
b01884
+                result.stdout_text, result.stderr_text
b01884
+            )
b01884
+        else:
b01884
+            return result
b01884
+
b01884
 
b01884
 class WinHost(pytest_multihost.host.WinHost):
b01884
     """
b01884
From 97776d2c4eed5de73780476bb11a635a2e47ebc5 Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Wed, 21 Nov 2018 10:00:20 +0100
b01884
Subject: [PATCH] Test smart card advise scripts
b01884
b01884
Create and execute the server and client smart card advise scripts.
b01884
b01884
See: See: https://pagure.io/freeipa/issue/7751
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipatests/prci_definitions/gating.yaml    |  2 +-
b01884
 ipatests/test_integration/test_advise.py | 99 ++++++++++++++++++------
b01884
 2 files changed, 75 insertions(+), 26 deletions(-)
b01884
b01884
diff --git a/ipatests/prci_definitions/gating.yaml b/ipatests/prci_definitions/gating.yaml
b01884
index 7a9b612dea..13f8851d02 100644
b01884
--- a/ipatests/prci_definitions/gating.yaml
b01884
+++ b/ipatests/prci_definitions/gating.yaml
b01884
@@ -157,7 +157,7 @@ jobs:
b01884
         test_suite: test_integration/test_advise.py
b01884
         template: *ci-master-f29
b01884
         timeout: 3600
b01884
-        topology: *master_1repl
b01884
+        topology: *master_1repl_1client
b01884
 
b01884
   fedora-29/test_testconfig:
b01884
     requires: [fedora-29/build]
b01884
diff --git a/ipatests/test_integration/test_advise.py b/ipatests/test_integration/test_advise.py
b01884
index 3b821c8797..b548614922 100644
b01884
--- a/ipatests/test_integration/test_advise.py
b01884
+++ b/ipatests/test_integration/test_advise.py
b01884
@@ -21,11 +21,17 @@
b01884
 # pylint: disable=no-member
b01884
 
b01884
 import re
b01884
+
b01884
+from ipalib.constants import IPAAPI_USER
b01884
+from ipaplatform.paths import paths
b01884
+from ipaplatform.constants import constants
b01884
+
b01884
+from ipatests.create_external_ca import ExternalCA
b01884
 from ipatests.pytest_ipa.integration import tasks
b01884
 from ipatests.test_integration.base import IntegrationTest
b01884
 
b01884
 
b01884
-def run_advice(master, advice_id, advice_regex, raiseerr):
b01884
+def run_advice(master, advice_id, advice_regex, raiseerr=True):
b01884
     # Obtain the advice from the server
b01884
     tasks.kinit_admin(master)
b01884
     result = master.run_command(['ipa-advise', advice_id],
b01884
@@ -43,28 +49,38 @@ class TestAdvice(IntegrationTest):
b01884
     """
b01884
     Tests ipa-advise output.
b01884
     """
b01884
-    advice_id = None
b01884
-    raiseerr = None
b01884
-    advice_regex = ''
b01884
     topology = 'line'
b01884
+    num_replicas = 0
b01884
+    num_clients = 1
b01884
+
b01884
+    def execute_advise(self, host, advice_id, *args):
b01884
+        # ipa-advise script is only available on a server
b01884
+        tasks.kinit_admin(self.master)
b01884
+        advice = self.master.run_command(['ipa-advise', advice_id])
b01884
+        # execute script on host (client or master)
b01884
+        if host is not self.master:
b01884
+            tasks.kinit_admin(host)
b01884
+        filename = tasks.upload_temp_contents(host, advice.stdout_text)
b01884
+        cmd = ['sh', filename]
b01884
+        cmd.extend(args)
b01884
+        try:
b01884
+            result = host.run_command(cmd)
b01884
+        finally:
b01884
+            host.run_command(['rm', '-f', filename])
b01884
+        return advice, result
b01884
 
b01884
     def test_invalid_advice(self):
b01884
         advice_id = r'invalid-advise-param'
b01884
         advice_regex = r"invalid[\s]+\'advice\'.*"
b01884
-        raiseerr = False
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
-
b01884
+        run_advice(self.master, advice_id, advice_regex, raiseerr=False)
b01884
 
b01884
     def test_advice_FreeBSDNSSPAM(self):
b01884
         advice_id = 'config-freebsd-nss-pam-ldapd'
b01884
         advice_regex = r"\#\!\/bin\/sh.*" \
b01884
                        r"pkg_add[\s]+\-r[\s]+nss\-pam\-ldapd[\s]+curl.*" \
b01884
                        r"\/usr\/local\/etc\/rc\.d\/nslcd[\s]+restart"
b01884
-        raiseerr = True
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
 
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
 
b01884
     def test_advice_GenericNSSPAM(self):
b01884
         advice_id = 'config-generic-linux-nss-pam-ldapd'
b01884
@@ -75,20 +91,16 @@ def test_advice_GenericNSSPAM(self):
b01884
             r"service[\s]+nscd[\s]+stop[\s]+\&\&[\s]+service[\s]+"
b01884
             r"nslcd[\s]+restart"
b01884
         )
b01884
-        raiseerr = True
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
 
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
 
b01884
     def test_advice_GenericSSSDBefore19(self):
b01884
         advice_id = r'config-generic-linux-sssd-before-1-9'
b01884
         advice_regex = r"\#\!\/bin\/sh.*" \
b01884
                        r"apt\-get[\s]+\-y[\s]+install sssd curl openssl.*" \
b01884
                        r"service[\s]+sssd[\s]+start"
b01884
-        raiseerr = True
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
 
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
 
b01884
     def test_advice_RedHatNSS(self):
b01884
         advice_id = 'config-redhat-nss-ldap'
b01884
@@ -100,10 +112,8 @@ def test_advice_RedHatNSS(self):
b01884
             r"[\s]+\-\-enableldapauth[\s]+"
b01884
             r"\-\-ldapserver=.*[\s]+\-\-ldapbasedn=.*"
b01884
         )
b01884
-        raiseerr = True
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
 
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
 
b01884
     def test_advice_RedHatNSSPAM(self):
b01884
         advice_id = 'config-redhat-nss-pam-ldapd'
b01884
@@ -113,10 +123,8 @@ def test_advice_RedHatNSSPAM(self):
b01884
                        r"authconfig[\s]+\-\-updateall[\s]+\-\-enableldap"\
b01884
                        r"[\s]+\-\-enableldaptls[\s]+\-\-enableldapauth[\s]+" \
b01884
                        r"\-\-ldapserver=.*[\s]+\-\-ldapbasedn=.*"
b01884
-        raiseerr = True
b01884
-
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
 
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
 
b01884
     def test_advice_RedHatSSSDBefore19(self):
b01884
         advice_id = 'config-redhat-sssd-before-1-9'
b01884
@@ -125,6 +133,47 @@ def test_advice_RedHatSSSDBefore19(self):
b01884
             r"yum[\s]+install[\s]+\-y[\s]+sssd[\s]+authconfig[\s]+"
b01884
             r"curl[\s]+openssl.*service[\s]+sssd[\s]+start"
b01884
         )
b01884
-        raiseerr = True
b01884
 
b01884
-        run_advice(self.master, advice_id, advice_regex, raiseerr)
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
+
b01884
+    # trivial checks
b01884
+    def test_advice_enable_admins_sudo(self):
b01884
+        advice_id = 'enable_admins_sudo'
b01884
+        advice_regex = r"\#\!\/bin\/sh.*"
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
+
b01884
+    def test_advice_config_server_for_smart_card_auth(self):
b01884
+        advice_id = 'config_server_for_smart_card_auth'
b01884
+        advice_regex = r"\#\!\/bin\/sh.*"
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
+
b01884
+        ca_pem = ExternalCA().create_ca()
b01884
+        ca_file = tasks.upload_temp_contents(self.master, ca_pem)
b01884
+        try:
b01884
+            self.execute_advise(self.master, advice_id, ca_file)
b01884
+        except Exception:
b01884
+            # debug: sometimes ipa-certupdate times out in
b01884
+            # "Resubmitting certmonger request"
b01884
+            self.master.run_command(['getcert', 'list'])
b01884
+            raise
b01884
+        finally:
b01884
+            self.master.run_command(['rm', '-f', ca_file])
b01884
+        sssd_conf = self.master.get_file_contents(
b01884
+            paths.SSSD_CONF, encoding='utf-8'
b01884
+        )
b01884
+        assert constants.HTTPD_USER in sssd_conf
b01884
+        assert IPAAPI_USER in sssd_conf
b01884
+
b01884
+    def test_advice_config_client_for_smart_card_auth(self):
b01884
+        advice_id = 'config_client_for_smart_card_auth'
b01884
+        advice_regex = r"\#\!\/bin\/sh.*"
b01884
+        run_advice(self.master, advice_id, advice_regex)
b01884
+
b01884
+        client = self.clients[0]
b01884
+
b01884
+        ca_pem = ExternalCA().create_ca()
b01884
+        ca_file = tasks.upload_temp_contents(client, ca_pem)
b01884
+        try:
b01884
+            self.execute_advise(client, advice_id, ca_file)
b01884
+        finally:
b01884
+            client.run_command(['rm', '-f', ca_file])
b01884
From 6ed90a2ac08c070e8e5c47a1eb3c52d7d30cabb8 Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Wed, 21 Nov 2018 10:44:55 +0100
b01884
Subject: [PATCH] Add install/remove package helpers to advise
b01884
b01884
The smart card advise scripts assume that yum is installed. However
b01884
Fedora has dnf and the yum wrapper is not installed by default.
b01884
Installation and removal of packages is now provided by two helper
b01884
methods that detect the package manager.
b01884
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipaserver/advise/base.py                    | 36 +++++++++++++++++++++
b01884
 ipaserver/advise/plugins/smart_card_auth.py | 24 +++++++-------
b01884
 2 files changed, 47 insertions(+), 13 deletions(-)
b01884
b01884
diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
b01884
index 07b1431e84..ec65113e34 100644
b01884
--- a/ipaserver/advise/base.py
b01884
+++ b/ipaserver/advise/base.py
b01884
@@ -227,6 +227,7 @@ def __init__(self):
b01884
         self.content = []
b01884
         self.prefix = '# '
b01884
         self.options = None
b01884
+        self.pkgmgr_detected = False
b01884
         self._indentation_tracker = _IndentationTracker(
b01884
             spaces_per_indent=DEFAULT_INDENTATION_INCREMENT)
b01884
 
b01884
@@ -312,6 +313,41 @@ def exit_on_predicate(self, predicate, error_message_lines):
b01884
 
b01884
             self.command('exit 1')
b01884
 
b01884
+    def detect_pkgmgr(self):
b01884
+        self.commands_on_predicate(
b01884
+            'which yum >/dev/null',
b01884
+            commands_to_run_when_true=['PKGMGR=yum'],
b01884
+            commands_to_run_when_false=['PKGMGR=dnf']
b01884
+        )
b01884
+        self.pkgmgr_detected = True
b01884
+
b01884
+    def install_packages(self, names, error_message_lines):
b01884
+        assert isinstance(names, list)
b01884
+        self.detect_pkgmgr()
b01884
+        self.command('rpm -qi {} > /dev/null'.format(' '.join(names)))
b01884
+        self.commands_on_predicate(
b01884
+            '[ "$?" -ne "0" ]',
b01884
+            ['$PKGMGR install -y {}'.format(' '.join(names))]
b01884
+        )
b01884
+        self.exit_on_predicate(
b01884
+            '[ "$?" -ne "0" ]',
b01884
+            error_message_lines
b01884
+        )
b01884
+
b01884
+    def remove_package(self, name, error_message_lines):
b01884
+        # remove only supports one package name
b01884
+        assert ' ' not in name
b01884
+        self.detect_pkgmgr()
b01884
+        self.command('rpm -qi {} > /dev/null'.format(name))
b01884
+        self.commands_on_predicate(
b01884
+            '[ "$?" -eq "0" ]',
b01884
+            ['$PKGMGR remove -y {} || exit 1'.format(name)]
b01884
+        )
b01884
+        self.exit_on_predicate(
b01884
+            '[ "$?" -ne "0" ]',
b01884
+            error_message_lines
b01884
+        )
b01884
+
b01884
     @contextmanager
b01884
     def unbranched_if(self, predicate):
b01884
         with self._compound_statement(UnbranchedIfStatement, predicate):
b01884
diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
b01884
index 9a7a315ed5..411124f935 100644
b01884
--- a/ipaserver/advise/plugins/smart_card_auth.py
b01884
+++ b/ipaserver/advise/plugins/smart_card_auth.py
b01884
@@ -135,9 +135,10 @@ def resolve_ipaca_records(self):
b01884
 
b01884
         self.log.comment('make sure bind-utils are installed so that we can '
b01884
                          'dig for ipa-ca records')
b01884
-        self.log.exit_on_failed_command(
b01884
-            'yum install -y bind-utils',
b01884
-            ['Failed to install bind-utils'])
b01884
+        self.log.install_packages(
b01884
+            ['bind-utils'],
b01884
+            ['Failed to install bind-utils']
b01884
+        )
b01884
 
b01884
         self.log.comment('make sure ipa-ca records are resolvable, '
b01884
                          'otherwise error out and instruct')
b01884
@@ -272,26 +273,23 @@ def get_info(self):
b01884
         self.restart_sssd()
b01884
 
b01884
     def check_and_remove_pam_pkcs11(self):
b01884
-        self.log.command('rpm -qi pam_pkcs11 > /dev/null')
b01884
-        self.log.commands_on_predicate(
b01884
-            '[ "$?" -eq "0" ]',
b01884
-            [
b01884
-                'yum remove -y pam_pkcs11'
b01884
-            ]
b01884
+        self.log.remove_package(
b01884
+            'pam_pkcs11',
b01884
+            ['Could not remove pam_pkcs11 package']
b01884
         )
b01884
 
b01884
     def install_opensc_and_dconf_packages(self):
b01884
         self.log.comment(
b01884
             'authconfig often complains about missing dconf, '
b01884
             'install it explicitly')
b01884
-        self.log.exit_on_failed_command(
b01884
-            'yum install -y {} dconf'.format(self.opensc_module_name.lower()),
b01884
+        self.log.install_packages(
b01884
+            [self.opensc_module_name.lower(), 'dconf'],
b01884
             ['Could not install OpenSC package']
b01884
         )
b01884
 
b01884
     def install_krb5_client_dependencies(self):
b01884
-        self.log.exit_on_failed_command(
b01884
-            'yum install -y krb5-pkinit-openssl',
b01884
+        self.log.install_packages(
b01884
+            ['krb5-pkinit-openssl'],
b01884
             ['Failed to install Kerberos client PKINIT extensions.']
b01884
         )
b01884
 
b01884
From e05ce4a20d2395179580db7e3db75c601c8f364c Mon Sep 17 00:00:00 2001
b01884
From: Christian Heimes <cheimes@redhat.com>
b01884
Date: Thu, 13 Dec 2018 14:40:44 +0100
b01884
Subject: [PATCH] Python 2 compatibility
b01884
b01884
Make new test helpers and test code compatible with Python 2.7.
b01884
b01884
See: https://pagure.io/freeipa/issue/7751
b01884
Signed-off-by: Christian Heimes <cheimes@redhat.com>
b01884
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
b01884
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
b01884
---
b01884
 ipatests/pytest_ipa/integration/host.py  | 4 ++--
b01884
 ipatests/test_integration/test_advise.py | 2 ++
b01884
 2 files changed, 4 insertions(+), 2 deletions(-)
b01884
b01884
diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
b01884
index 6aed58ae96..eb05872467 100644
b01884
--- a/ipatests/pytest_ipa/integration/host.py
b01884
+++ b/ipatests/pytest_ipa/integration/host.py
b01884
@@ -65,7 +65,7 @@ def run_command(self, argv, set_env=True, stdin_text=None,
b01884
                     log_stdout=True, raiseonerr=True,
b01884
                     cwd=None, bg=False, encoding='utf-8'):
b01884
         # Wrap run_command to log stderr on raiseonerr=True
b01884
-        result = super().run_command(
b01884
+        result = super(Host, self).run_command(
b01884
             argv, set_env=set_env, stdin_text=stdin_text,
b01884
             log_stdout=log_stdout, raiseonerr=False, cwd=cwd, bg=bg,
b01884
             encoding=encoding
b01884
@@ -74,7 +74,7 @@ def run_command(self, argv, set_env=True, stdin_text=None,
b01884
             result.log.error('stderr: %s', result.stderr_text)
b01884
             raise subprocess.CalledProcessError(
b01884
                 result.returncode, argv,
b01884
-                result.stdout_text, result.stderr_text
b01884
+                result.stderr_text
b01884
             )
b01884
         else:
b01884
             return result
b01884
diff --git a/ipatests/test_integration/test_advise.py b/ipatests/test_integration/test_advise.py
b01884
index b548614922..761f278238 100644
b01884
--- a/ipatests/test_integration/test_advise.py
b01884
+++ b/ipatests/test_integration/test_advise.py
b01884
@@ -20,6 +20,8 @@
b01884
 # FIXME: Pylint errors
b01884
 # pylint: disable=no-member
b01884
 
b01884
+from __future__ import absolute_import
b01884
+
b01884
 import re
b01884
 
b01884
 from ipalib.constants import IPAAPI_USER