URL: https://github.com/freeipa/freeipa/pull/697
Author: dkupka
 Title: #697: Create system users for FreeIPA services during package 
installation
Action: opened

PR body:
"""
Previously system users needed by FreeIPA server services was created during
ipa-server-install. This led to problem when DBus policy was configured during
package installation but the user specified in the policy didn't exist yet (and
potentionally similar ones). Now systemd-sysusers service is used to ensure
users freeipa-server package needs exist before any installation or
configuration begins.

https://pagure.io/freeipa/issue/6743
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/697/head:pr697
git checkout pr697
From 84fc1e036861027e7f73e2f1b7a5522df3533aaf Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 6 Apr 2017 12:35:35 +0200
Subject: [PATCH] Create system users for FreeIPA services during package
 installation

Previously system users needed by FreeIPA server services was created during
ipa-server-install. This led to problem when DBus policy was configured during
package installation but the user specified in the policy didn't exist yet (and
potentionally similar ones). Now systemd-sysusers service is used to ensure
users freeipa-server package needs exist before any installation or
configuration begins.

https://pagure.io/freeipa/issue/6743
---
 configure.ac                               |  1 +
 freeipa.spec.in                            |  5 +++
 init/systemd/Makefile.am                   | 15 +++++++--
 init/systemd/ipa.sysusers.base.conf        |  5 +++
 init/systemd/ipa.sysusers.debian.conf      |  1 +
 init/systemd/ipa.sysusers.fedora.conf      |  1 +
 init/systemd/ipa.sysusers.redhat.conf      |  1 +
 init/systemd/ipa.sysusers.rhel.conf        |  1 +
 ipaplatform/base/tasks.py                  | 53 ------------------------------
 ipaplatform/redhat/tasks.py                | 26 ---------------
 ipaserver/install/cainstance.py            | 12 -------
 ipaserver/install/dsinstance.py            | 11 -------
 ipaserver/install/httpinstance.py          | 13 --------
 ipaserver/install/installutils.py          | 13 --------
 ipaserver/install/ipa_restore.py           |  7 ----
 ipaserver/install/server/install.py        |  6 +---
 ipaserver/install/server/replicainstall.py |  6 +---
 ipaserver/install/server/upgrade.py        |  2 --
 server.m4                                  |  7 ++++
 19 files changed, 37 insertions(+), 149 deletions(-)
 create mode 100644 init/systemd/ipa.sysusers.base.conf
 create mode 120000 init/systemd/ipa.sysusers.debian.conf
 create mode 120000 init/systemd/ipa.sysusers.fedora.conf
 create mode 120000 init/systemd/ipa.sysusers.redhat.conf
 create mode 120000 init/systemd/ipa.sysusers.rhel.conf

diff --git a/configure.ac b/configure.ac
index 8f8751a..2cba2cf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -627,6 +627,7 @@ AM_COND_IF([ENABLE_SERVER], [
         KRAD libs:                ${KRAD_LIBS}
         krb5rundir:               ${krb5rundir}
         systemdsystemunitdir:     ${systemdsystemunitdir}
+        sysusersdir:              ${sysusersdir}
         systemdtmpfilesdir:       ${systemdtmpfilesdir}
         build mode:               server & client"
 ], [
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 61e9acd..765e4aa 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -977,6 +977,10 @@ rm -rf %{buildroot}
 # NOTE: systemd specific section
     /bin/systemctl --system daemon-reload 2>&1 || :
 # END
+
+# ensure system users needed by FreeIPA services are created
+/bin/systemd-sysusers ipa.conf
+
 if [ $1 -gt 1 ] ; then
     /bin/systemctl condrestart certmonger.service 2>&1 || :
 fi
@@ -1176,6 +1180,7 @@ fi
 %attr(644,root,root) %{_unitdir}/ipa-dnskeysyncd.service
 %attr(644,root,root) %{_unitdir}/ipa-ods-exporter.socket
 %attr(644,root,root) %{_unitdir}/ipa-ods-exporter.service
+%attr(644,root,root) %{_sysusersdir}/ipa.conf
 # END
 %attr(755,root,root) %{plugin_dir}/libipa_pwd_extop.so
 %attr(755,root,root) %{plugin_dir}/libipa_enrollment_extop.so
diff --git a/init/systemd/Makefile.am b/init/systemd/Makefile.am
index 945f6ac..1c46cb1 100644
--- a/init/systemd/Makefile.am
+++ b/init/systemd/Makefile.am
@@ -4,13 +4,21 @@ AUTOMAKE_OPTIONS = 1.7
 
 dist_noinst_DATA = 			\
 	ipa-custodia.service.in		\
-	ipa.service.in
+	ipa.service.in	\
+	ipa.sysusers.base.conf	\
+	ipa.sysusers.debian.conf	\
+	ipa.sysusers.fedora.conf	\
+	ipa.sysusers.redhat.conf	\
+	ipa.sysusers.rhel.conf
 
 systemdsystemunit_DATA = 	\
 	ipa-custodia.service	\
 	ipa.service
 
-CLEANFILES = $(systemdsystemunit_DATA)
+sysusers_DATA = \
+	ipa.conf
+
+CLEANFILES = $(systemdsystemunit_DATA) $(sysusers_DATA)
 
 %: %.in Makefile
 	sed \
@@ -21,3 +29,6 @@ CLEANFILES = $(systemdsystemunit_DATA)
 		-e 's|@libexecdir[@]|$(libexecdir)|g' \
 		-e 's|@sysconfenvdir[@]|$(sysconfenvdir)|g' \
 		'$(srcdir)/$@.in' >$@
+
+ipa.conf: ipa.sysusers.$(IPAPLATFORM).conf
+		cp -L $(srcdir)/$< $@
diff --git a/init/systemd/ipa.sysusers.base.conf b/init/systemd/ipa.sysusers.base.conf
new file mode 100644
index 0000000..245359d
--- /dev/null
+++ b/init/systemd/ipa.sysusers.base.conf
@@ -0,0 +1,5 @@
+u kdcproxy  -   "IPA KDC Proxy User"    -
+u dirsrv    -   -   -
+u pkiuser   -   -   -
+u ipaapi    -   -   -
+m apache ipaapi
diff --git a/init/systemd/ipa.sysusers.debian.conf b/init/systemd/ipa.sysusers.debian.conf
new file mode 120000
index 0000000..829ad0a
--- /dev/null
+++ b/init/systemd/ipa.sysusers.debian.conf
@@ -0,0 +1 @@
+ipa.sysusers.base.conf
\ No newline at end of file
diff --git a/init/systemd/ipa.sysusers.fedora.conf b/init/systemd/ipa.sysusers.fedora.conf
new file mode 120000
index 0000000..e9954da
--- /dev/null
+++ b/init/systemd/ipa.sysusers.fedora.conf
@@ -0,0 +1 @@
+ipa.sysusers.redhat.conf
\ No newline at end of file
diff --git a/init/systemd/ipa.sysusers.redhat.conf b/init/systemd/ipa.sysusers.redhat.conf
new file mode 120000
index 0000000..829ad0a
--- /dev/null
+++ b/init/systemd/ipa.sysusers.redhat.conf
@@ -0,0 +1 @@
+ipa.sysusers.base.conf
\ No newline at end of file
diff --git a/init/systemd/ipa.sysusers.rhel.conf b/init/systemd/ipa.sysusers.rhel.conf
new file mode 120000
index 0000000..e9954da
--- /dev/null
+++ b/init/systemd/ipa.sysusers.rhel.conf
@@ -0,0 +1 @@
+ipa.sysusers.redhat.conf
\ No newline at end of file
diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index 9f91fef..3358b7d 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -22,9 +22,6 @@
 This module contains default platform-specific implementations of system tasks.
 '''
 
-import pwd
-import grp
-
 from pkg_resources import parse_version
 
 from ipaplatform.paths import paths
@@ -186,56 +183,6 @@ def set_selinux_booleans(self, required_settings, backup_func=None):
 
         raise NotImplementedError()
 
-    def create_system_user(self, name, group, homedir, shell,
-                           uid=None, gid=None, comment=None,
-                           create_homedir=False, groups=None):
-        """Create a system user with a corresponding group"""
-        try:
-            grp.getgrnam(group)
-        except KeyError:
-            log.debug('Adding group %s', group)
-            args = [paths.GROUPADD, '-r', group]
-            if gid:
-                args += ['-g', str(gid)]
-            try:
-                ipautil.run(args)
-                log.debug('Done adding group')
-            except ipautil.CalledProcessError as e:
-                log.critical('Failed to add group: %s', e)
-                raise
-        else:
-            log.debug('group %s exists', group)
-
-        try:
-            pwd.getpwnam(name)
-        except KeyError:
-            log.debug('Adding user %s', name)
-            args = [
-                paths.USERADD,
-                '-g', group,
-                '-d', homedir,
-                '-s', shell,
-                '-r', name,
-            ]
-            if uid:
-                args += ['-u', str(uid)]
-            if comment:
-                args += ['-c', comment]
-            if create_homedir:
-                args += ['-m']
-            else:
-                args += ['-M']
-            if groups is not None:
-                args += ['-G', groups.join(',')]
-            try:
-                ipautil.run(args)
-                log.debug('Done adding user')
-            except ipautil.CalledProcessError as e:
-                log.critical('Failed to add user: %s', e)
-                raise
-        else:
-            log.debug('user %s exists', name)
-
     @staticmethod
     def parse_ipa_version(version):
         """
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index d0ef5fb..07efeba 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -431,32 +431,6 @@ def get_setsebool_args(changes):
 
         return True
 
-    def create_system_user(self, name, group, homedir, shell,
-                           uid=None, gid=None, comment=None,
-                           create_homedir=False, groups=None):
-        """
-        Create a system user with a corresponding group
-
-        According to https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation
-        some system users should have fixed UID, GID and other parameters set.
-        This values should be constant and may be hardcoded.
-        Add other values for other users when needed.
-        """
-        if name == constants.PKI_USER:
-            if uid is None:
-                uid = 17
-            if gid is None:
-                gid = 17
-            if comment is None:
-                comment = 'CA System User'
-        if name == constants.DS_USER:
-            if comment is None:
-                comment = 'DS System User'
-
-        super(RedHatTaskNamespace, self).create_system_user(
-            name, group, homedir, shell, uid, gid, comment, create_homedir,
-            groups)
-
     def parse_ipa_version(self, version):
         """
         :param version: textual version
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 1d44c0d..1c8bb27 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -46,7 +46,6 @@
 import ipalib.constants
 from ipalib.install import certmonger
 from ipaplatform import services
-from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
 
@@ -263,16 +262,6 @@ def is_ca_installed_locally():
     return os.path.exists(paths.CA_CS_CFG_PATH)
 
 
-def create_ca_user():
-    """Create PKI user/group if it doesn't exist yet."""
-    tasks.create_system_user(
-        name=constants.PKI_USER,
-        group=constants.PKI_GROUP,
-        homedir=paths.VAR_LIB,
-        shell=paths.NOLOGIN,
-    )
-
-
 class CAInstance(DogtagInstance):
     """
     When using a dogtag CA the DS database contains just the
@@ -382,7 +371,6 @@ def configure_instance(self, host_name, dm_password, admin_password,
             has_ra_cert = False
 
         if not ra_only:
-            self.step("creating certificate server user", create_ca_user)
             if promote:
                 # Setup Database
                 self.step("creating certificate server db", self.__create_ds_db)
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 79dc90e..52814d4 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -158,16 +158,6 @@ def is_ds_running(server_id=''):
     return services.knownservices.dirsrv.is_running(instance_name=server_id)
 
 
-def create_ds_user():
-    """Create DS user/group if it doesn't exist yet."""
-    tasks.create_system_user(
-        name=DS_USER,
-        group=DS_USER,
-        homedir=paths.VAR_LIB_DIRSRV,
-        shell=paths.NOLOGIN,
-    )
-
-
 def get_domain_level(api=api):
     ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=api.env.realm)
     conn = ipaldap.LDAPClient(ldap_uri)
@@ -258,7 +248,6 @@ def __init__(self, realm_name=None, domain_name=None, fstore=None,
 
     def __common_setup(self, enable_ssl=False):
 
-        self.step("creating directory server user", create_ds_user)
         self.step("creating directory server instance", self.__create_instance)
         self.step("enabling ldapi", self.__enable_ldapi)
         self.step("configure autobind for root", self.__root_autobind)
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 079ea92..10221f8 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -102,18 +102,6 @@ def httpd_443_configured():
     return False
 
 
-def create_kdcproxy_user():
-    """Create KDC proxy user/group if it doesn't exist yet."""
-    tasks.create_system_user(
-        name=KDCPROXY_USER,
-        group=KDCPROXY_USER,
-        homedir=paths.VAR_LIB_KDCPROXY,
-        shell=paths.NOLOGIN,
-        comment="IPA KDC Proxy User",
-        create_homedir=True,
-    )
-
-
 class WebGuiInstance(service.SimpleServiceInstance):
     def __init__(self):
         service.SimpleServiceInstance.__init__(self, "ipa_webgui")
@@ -182,7 +170,6 @@ def create_instance(self, realm, fqdn, domain_name, pkcs12_info=None,
                   self.remove_httpd_ccaches)
         self.step("configuring SELinux for httpd", self.configure_selinux_for_httpd)
         if not self.is_kdcproxy_configured():
-            self.step("create KDC proxy user", create_kdcproxy_user)
             self.step("create KDC proxy config", self.create_kdcproxy_conf)
             self.step("enable KDC proxy", self.enable_kdcproxy)
         self.step("restarting httpd", self.__start)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index ef6a399..9230e70 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -44,7 +44,6 @@
 from six.moves.configparser import SafeConfigParser, NoOptionError
 # pylint: enable=import-error
 
-from ipalib.constants import IPAAPI_USER, IPAAPI_GROUP
 from ipalib.install import sysrestore
 from ipalib.install.kinit import kinit_password
 import ipaplatform
@@ -56,7 +55,6 @@
 from ipapython.dn import DN
 from ipaserver.install import certs, service, sysupgrade
 from ipaplatform import services
-from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
 
@@ -1515,14 +1513,3 @@ def default_subject_base(realm_name):
 
 def default_ca_subject_dn(subject_base):
     return DN(('CN', 'Certificate Authority'), subject_base)
-
-
-def create_ipaapi_user():
-    """Create IPA API user/group if it doesn't exist yet."""
-    tasks.create_system_user(
-        name=IPAAPI_USER,
-        group=IPAAPI_GROUP,
-        homedir=paths.VAR_LIB,
-        shell=paths.NOLOGIN
-    )
-    tasks.add_user_to_group(constants.HTTPD_USER, IPAAPI_GROUP)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 2552bbd..378c013 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -36,8 +36,6 @@
 from ipapython.ipautil import run, user_input
 from ipapython import admintool
 from ipapython.dn import DN
-from ipaserver.install.dsinstance import create_ds_user
-from ipaserver.install.cainstance import create_ca_user
 from ipaserver.install.replication import (wait_for_task, ReplicationManager,
                                            get_cs_replication_manager)
 from ipaserver.install import installutils
@@ -296,7 +294,6 @@ def run(self):
                     not user_input("Continue to restore?", False)):
                 raise admintool.ScriptError("Aborted")
 
-        create_ds_user()
         pent = pwd.getpwnam(constants.DS_USER)
 
         # Temporary directory for decrypting files before restoring
@@ -379,15 +376,11 @@ def run(self):
             # We do either a full file restore or we restore data.
             if restore_type == 'FULL':
                 self.remove_old_files()
-                if 'CA' in self.backup_services:
-                    create_ca_user()
                 self.cert_restore_prepare()
                 self.file_restore(options.no_logs)
                 self.cert_restore()
                 if 'CA' in self.backup_services:
                     self.__create_dogtag_log_dirs()
-                if http.is_kdcproxy_configured():
-                    httpinstance.create_kdcproxy_user()
 
             # Always restore the data from ldif
             # We need to restore both userRoot and ipaca.
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index d7eb0bf..0872404 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -39,7 +39,7 @@
 from ipaserver.install.installutils import (
     IPA_MODULES, BadHostError, get_fqdn, get_server_ip_address,
     is_ipa_configured, load_pkcs12, read_password, verify_fqdn,
-    update_hosts_file, create_ipaapi_user)
+    update_hosts_file)
 
 if six.PY3:
     unicode = str
@@ -721,12 +721,8 @@ def install(installer):
         update_hosts_file(ip_addresses, host_name, fstore)
 
     # Make sure tmpfiles dir exist before installing components
-    create_ipaapi_user()
     tasks.create_tmpfiles_dirs()
 
-    # Create DS user/group if it doesn't exist yet
-    dsinstance.create_ds_user()
-
     # Create a directory server instance
     if not options.external_cert_files:
         # Configure ntpd
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index f489e69..2c26609 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -42,8 +42,7 @@
     installutils, kra, krbinstance,
     ntpinstance, otpdinstance, custodiainstance, service)
 from ipaserver.install.installutils import (
-    create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured,
-    create_ipaapi_user)
+    create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured)
 from ipaserver.install.replication import (
     ReplicationManager, replica_conn_check)
 import SSSDConfig
@@ -1359,7 +1358,6 @@ def install(installer):
     ccache = os.environ['KRB5CCNAME']
 
     # Make sure tmpfiles dir exist before installing components
-    create_ipaapi_user()
     tasks.create_tmpfiles_dirs()
 
     if promote:
@@ -1388,8 +1386,6 @@ def install(installer):
         ntp = ntpinstance.NTPInstance()
         ntp.create_instance()
 
-    dsinstance.create_ds_user()
-
     try:
         if promote:
             conn.connect(ccache=ccache)
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 25b8629..927acb0 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1652,7 +1652,6 @@ def upgrade_configuration():
 
     if not http.is_kdcproxy_configured():
         root_logger.info('[Enabling KDC Proxy]')
-        httpinstance.create_kdcproxy_user()
         http.create_kdcproxy_conf()
         http.enable_kdcproxy()
 
@@ -1837,7 +1836,6 @@ def upgrade_check(options):
 
 def upgrade():
     # Do this early so that any code depending on these dirs will not fail
-    installutils.create_ipaapi_user()
     tasks.create_tmpfiles_dirs()
     tasks.configure_tmpfiles()
 
diff --git a/server.m4 b/server.m4
index 346d73e..2b46c10 100644
--- a/server.m4
+++ b/server.m4
@@ -144,3 +144,10 @@ AC_ARG_WITH([systemdtmpfilesdir],
         [systemdtmpfilesdir=$($PKG_CONFIG --define-variable=prefix='${prefix}' --variable=tmpfilesdir systemd)])
 AC_SUBST([systemdtmpfilesdir])
 
+AC_ARG_WITH([sysusersdir],
+            AS_HELP_STRING([--with-sysusersdir=DIR],
+               [Directory for sysusers configuration files]),
+            [sysusersdir=$with_sysusersdir],
+        [sysusersdir=$($PKG_CONFIG --define-variable=prefix='${prefix}' --variable=sysusersdir systemd)])
+AC_SUBST([sysusersdir])
+
-- 
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