43edf8
From 3eeeb160688eb8502217161e7c4df2aebbd2f704 Mon Sep 17 00:00:00 2001
43edf8
From: Florence Blanc-Renaud <flo@redhat.com>
43edf8
Date: Wed, 15 Mar 2023 11:58:27 +0100
43edf8
Subject: [PATCH] Defer creating the final krb5.conf on clients
43edf8
43edf8
A temporary krb5.conf is created early during client enrollment
43edf8
and was previously used only during the initial ipa-join call.
43edf8
The final krb5.conf was written soon afterward.
43edf8
43edf8
If there are multiple servers it is possible that the client
43edf8
may then choose a different KDC to connect. If the client
43edf8
is faster than replication then the client may not exist
43edf8
on all servers and therefore enrollment will fail.
43edf8
43edf8
This was seen in performance testing of how many simultaneous
43edf8
client enrollments are possible.
43edf8
43edf8
Use a decorator to wrap the _install() method to ensure the
43edf8
temporary files created during installation are cleaned up.
43edf8
43edf8
https://pagure.io/freeipa/issue/9228
43edf8
43edf8
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
43edf8
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
43edf8
Reviewed-By: Stanislav Levin <slev@altlinux.org>
43edf8
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
43edf8
---
43edf8
 ipaclient/install/client.py | 389 +++++++++++++++++++-----------------
43edf8
 1 file changed, 202 insertions(+), 187 deletions(-)
43edf8
43edf8
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
43edf8
index eb4dc037ead309299121889875e9152ef2c92b7a..0f42937997452e9c37a2c98477638de51cf46b90 100644
43edf8
--- a/ipaclient/install/client.py
43edf8
+++ b/ipaclient/install/client.py
43edf8
@@ -93,6 +93,37 @@ cli_basedn = None
43edf8
 # end of global variables
43edf8
 
43edf8
 
43edf8
+def cleanup(func):
43edf8
+    def inner(options, tdict):
43edf8
+        # Add some additional options which contain the temporary files
43edf8
+        # needed during installation.
43edf8
+        fd, krb_name = tempfile.mkstemp()
43edf8
+        os.close(fd)
43edf8
+        ccache_dir = tempfile.mkdtemp(prefix='krbcc')
43edf8
+
43edf8
+        tdict['krb_name'] = krb_name
43edf8
+        tdict['ccache_dir'] = ccache_dir
43edf8
+
43edf8
+        func(options, tdict)
43edf8
+
43edf8
+        os.environ.pop('KRB5_CONFIG', None)
43edf8
+
43edf8
+        try:
43edf8
+            os.remove(krb_name)
43edf8
+        except OSError:
43edf8
+            logger.error("Could not remove %s", krb_name)
43edf8
+        try:
43edf8
+            os.rmdir(ccache_dir)
43edf8
+        except OSError:
43edf8
+            pass
43edf8
+        try:
43edf8
+            os.remove(krb_name + ".ipabkp")
43edf8
+        except OSError:
43edf8
+            logger.error("Could not remove %s.ipabkp", krb_name)
43edf8
+
43edf8
+    return inner
43edf8
+
43edf8
+
43edf8
 def remove_file(filename):
43edf8
     """
43edf8
     Deletes a file. If the file does not exist (OSError 2) does nothing.
43edf8
@@ -2388,7 +2419,7 @@ def update_ipa_nssdb():
43edf8
 
43edf8
 def install(options):
43edf8
     try:
43edf8
-        _install(options)
43edf8
+        _install(options, dict())
43edf8
     except ScriptError as e:
43edf8
         if e.rval == CLIENT_INSTALL_ERROR:
43edf8
             if options.force:
43edf8
@@ -2415,12 +2446,16 @@ def install(options):
43edf8
             pass
43edf8
 
43edf8
 
43edf8
-def _install(options):
43edf8
+@cleanup
43edf8
+def _install(options, tdict):
43edf8
     env = {'PATH': SECURE_PATH}
43edf8
 
43edf8
     fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
43edf8
     statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE)
43edf8
 
43edf8
+    krb_name = tdict['krb_name']
43edf8
+    ccache_dir = tdict['ccache_dir']
43edf8
+
43edf8
     if not options.on_master:
43edf8
         # Try removing old principals from the keytab
43edf8
         purge_host_keytab(cli_realm)
43edf8
@@ -2476,186 +2511,166 @@ def _install(options):
43edf8
     host_principal = 'host/%s@%s' % (hostname, cli_realm)
43edf8
     if not options.on_master:
43edf8
         nolog = tuple()
43edf8
-        # First test out the kerberos configuration
43edf8
-        fd, krb_name = tempfile.mkstemp()
43edf8
-        os.close(fd)
43edf8
-        ccache_dir = tempfile.mkdtemp(prefix='krbcc')
43edf8
-        try:
43edf8
-            configure_krb5_conf(
43edf8
-                cli_realm=cli_realm,
43edf8
-                cli_domain=cli_domain,
43edf8
-                cli_server=cli_server,
43edf8
-                cli_kdc=cli_kdc,
43edf8
-                dnsok=False,
43edf8
-                filename=krb_name,
43edf8
-                client_domain=client_domain,
43edf8
-                client_hostname=hostname,
43edf8
-                configure_sssd=options.sssd,
43edf8
-                force=options.force)
43edf8
-            env['KRB5_CONFIG'] = krb_name
43edf8
-            ccache_name = os.path.join(ccache_dir, 'ccache')
43edf8
-            join_args = [paths.SBIN_IPA_JOIN,
43edf8
-                         "-s", cli_server[0],
43edf8
-                         "-b", str(realm_to_suffix(cli_realm)),
43edf8
-                         "-h", hostname]
43edf8
-            if options.debug:
43edf8
-                join_args.append("-d")
43edf8
-                env['XMLRPC_TRACE_CURL'] = 'yes'
43edf8
-            if options.force_join:
43edf8
-                join_args.append("-f")
43edf8
-            if options.principal is not None:
43edf8
-                stdin = None
43edf8
-                principal = options.principal
43edf8
-                if principal.find('@') == -1:
43edf8
-                    principal = '%s@%s' % (principal, cli_realm)
43edf8
-                if options.password is not None:
43edf8
-                    stdin = options.password
43edf8
+        configure_krb5_conf(
43edf8
+            cli_realm=cli_realm,
43edf8
+            cli_domain=cli_domain,
43edf8
+            cli_server=cli_server,
43edf8
+            cli_kdc=cli_kdc,
43edf8
+            dnsok=False,
43edf8
+            filename=krb_name,
43edf8
+            client_domain=client_domain,
43edf8
+            client_hostname=hostname,
43edf8
+            configure_sssd=options.sssd,
43edf8
+            force=options.force)
43edf8
+        env['KRB5_CONFIG'] = krb_name
43edf8
+        ccache_name = os.path.join(ccache_dir, 'ccache')
43edf8
+        join_args = [paths.SBIN_IPA_JOIN,
43edf8
+                     "-s", cli_server[0],
43edf8
+                     "-b", str(realm_to_suffix(cli_realm)),
43edf8
+                     "-h", hostname]
43edf8
+        if options.debug:
43edf8
+            join_args.append("-d")
43edf8
+            env['XMLRPC_TRACE_CURL'] = 'yes'
43edf8
+        if options.force_join:
43edf8
+            join_args.append("-f")
43edf8
+        if options.principal is not None:
43edf8
+            stdin = None
43edf8
+            principal = options.principal
43edf8
+            if principal.find('@') == -1:
43edf8
+                principal = '%s@%s' % (principal, cli_realm)
43edf8
+            if options.password is not None:
43edf8
+                stdin = options.password
43edf8
+            else:
43edf8
+                if not options.unattended:
43edf8
+                    try:
43edf8
+                        stdin = getpass.getpass(
43edf8
+                            "Password for %s: " % principal)
43edf8
+                    except EOFError:
43edf8
+                        stdin = None
43edf8
+                    if not stdin:
43edf8
+                        raise ScriptError(
43edf8
+                            "Password must be provided for {}.".format(
43edf8
+                                principal),
43edf8
+                            rval=CLIENT_INSTALL_ERROR)
43edf8
                 else:
43edf8
-                    if not options.unattended:
43edf8
-                        try:
43edf8
-                            stdin = getpass.getpass(
43edf8
-                                "Password for %s: " % principal)
43edf8
-                        except EOFError:
43edf8
-                            stdin = None
43edf8
-                        if not stdin:
43edf8
-                            raise ScriptError(
43edf8
-                                "Password must be provided for {}.".format(
43edf8
-                                    principal),
43edf8
-                                rval=CLIENT_INSTALL_ERROR)
43edf8
+                    if sys.stdin.isatty():
43edf8
+                        logger.error(
43edf8
+                            "Password must be provided in "
43edf8
+                            "non-interactive mode.")
43edf8
+                        logger.info(
43edf8
+                            "This can be done via "
43edf8
+                            "echo password | ipa-client-install ... "
43edf8
+                            "or with the -w option.")
43edf8
+                        raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
                     else:
43edf8
-                        if sys.stdin.isatty():
43edf8
-                            logger.error(
43edf8
-                                "Password must be provided in "
43edf8
-                                "non-interactive mode.")
43edf8
-                            logger.info(
43edf8
-                                "This can be done via "
43edf8
-                                "echo password | ipa-client-install ... "
43edf8
-                                "or with the -w option.")
43edf8
-                            raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
-                        else:
43edf8
-                            stdin = sys.stdin.readline()
43edf8
+                        stdin = sys.stdin.readline()
43edf8
 
43edf8
+            try:
43edf8
+                kinit_password(principal, stdin, ccache_name,
43edf8
+                               config=krb_name)
43edf8
+            except RuntimeError as e:
43edf8
+                print_port_conf_info()
43edf8
+                raise ScriptError(
43edf8
+                    "Kerberos authentication failed: {}".format(e),
43edf8
+                    rval=CLIENT_INSTALL_ERROR)
43edf8
+        elif options.keytab:
43edf8
+            join_args.append("-f")
43edf8
+            if os.path.exists(options.keytab):
43edf8
                 try:
43edf8
-                    kinit_password(principal, stdin, ccache_name,
43edf8
-                                   config=krb_name)
43edf8
-                except RuntimeError as e:
43edf8
+                    kinit_keytab(host_principal,
43edf8
+                                 options.keytab,
43edf8
+                                 ccache_name,
43edf8
+                                 config=krb_name,
43edf8
+                                 attempts=options.kinit_attempts)
43edf8
+                except gssapi.exceptions.GSSError as e:
43edf8
                     print_port_conf_info()
43edf8
                     raise ScriptError(
43edf8
                         "Kerberos authentication failed: {}".format(e),
43edf8
                         rval=CLIENT_INSTALL_ERROR)
43edf8
-            elif options.keytab:
43edf8
-                join_args.append("-f")
43edf8
-                if os.path.exists(options.keytab):
43edf8
-                    try:
43edf8
-                        kinit_keytab(host_principal,
43edf8
-                                     options.keytab,
43edf8
-                                     ccache_name,
43edf8
-                                     config=krb_name,
43edf8
-                                     attempts=options.kinit_attempts)
43edf8
-                    except gssapi.exceptions.GSSError as e:
43edf8
-                        print_port_conf_info()
43edf8
-                        raise ScriptError(
43edf8
-                            "Kerberos authentication failed: {}".format(e),
43edf8
-                            rval=CLIENT_INSTALL_ERROR)
43edf8
-                else:
43edf8
-                    raise ScriptError(
43edf8
-                        "Keytab file could not be found: {}".format(
43edf8
-                            options.keytab),
43edf8
-                        rval=CLIENT_INSTALL_ERROR)
43edf8
-            elif options.password:
43edf8
-                nolog = (options.password,)
43edf8
-                join_args.append("-w")
43edf8
-                join_args.append(options.password)
43edf8
-            elif options.prompt_password:
43edf8
-                if options.unattended:
43edf8
-                    raise ScriptError(
43edf8
-                        "Password must be provided in non-interactive mode",
43edf8
-                        rval=CLIENT_INSTALL_ERROR)
43edf8
-                try:
43edf8
-                    password = getpass.getpass("Password: ")
43edf8
-                except EOFError:
43edf8
-                    password = None
43edf8
-                if not password:
43edf8
-                    raise ScriptError(
43edf8
-                        "Password must be provided.",
43edf8
-                        rval=CLIENT_INSTALL_ERROR)
43edf8
-                join_args.append("-w")
43edf8
-                join_args.append(password)
43edf8
-                nolog = (password,)
43edf8
-
43edf8
-            env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = ccache_name
43edf8
-            # Get the CA certificate
43edf8
+            else:
43edf8
+                raise ScriptError(
43edf8
+                    "Keytab file could not be found: {}".format(
43edf8
+                        options.keytab),
43edf8
+                    rval=CLIENT_INSTALL_ERROR)
43edf8
+        elif options.password:
43edf8
+            nolog = (options.password,)
43edf8
+            join_args.append("-w")
43edf8
+            join_args.append(options.password)
43edf8
+        elif options.prompt_password:
43edf8
+            if options.unattended:
43edf8
+                raise ScriptError(
43edf8
+                    "Password must be provided in non-interactive mode",
43edf8
+                    rval=CLIENT_INSTALL_ERROR)
43edf8
             try:
43edf8
-                os.environ['KRB5_CONFIG'] = env['KRB5_CONFIG']
43edf8
-                get_ca_certs(fstore, options, cli_server[0], cli_basedn,
43edf8
-                             cli_realm)
43edf8
-                del os.environ['KRB5_CONFIG']
43edf8
-            except errors.FileError as e:
43edf8
-                logger.error('%s', e)
43edf8
-                raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
-            except Exception as e:
43edf8
-                logger.error("Cannot obtain CA certificate\n%s", e)
43edf8
-                raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
+                password = getpass.getpass("Password: ")
43edf8
+            except EOFError:
43edf8
+                password = None
43edf8
+            if not password:
43edf8
+                raise ScriptError(
43edf8
+                    "Password must be provided.",
43edf8
+                    rval=CLIENT_INSTALL_ERROR)
43edf8
+            join_args.append("-w")
43edf8
+            join_args.append(password)
43edf8
+            nolog = (password,)
43edf8
 
43edf8
-            # Now join the domain
43edf8
-            result = run(
43edf8
-                join_args, raiseonerr=False, env=env, nolog=nolog,
43edf8
-                capture_error=True)
43edf8
-            stderr = result.error_output
43edf8
+        env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = ccache_name
43edf8
+        # Get the CA certificate
43edf8
+        try:
43edf8
+            os.environ['KRB5_CONFIG'] = env['KRB5_CONFIG']
43edf8
+            get_ca_certs(fstore, options, cli_server[0], cli_basedn,
43edf8
+                         cli_realm)
43edf8
+        except errors.FileError as e:
43edf8
+            logger.error('%s', e)
43edf8
+            raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
+        except Exception as e:
43edf8
+            logger.error("Cannot obtain CA certificate\n%s", e)
43edf8
+            raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
 
43edf8
-            if result.returncode != 0:
43edf8
-                logger.error("Joining realm failed: %s", stderr)
43edf8
-                if not options.force:
43edf8
-                    if result.returncode == 13:
43edf8
-                        logger.info(
43edf8
-                            "Use --force-join option to override the host "
43edf8
-                            "entry on the server and force client enrollment.")
43edf8
-                    raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
-                logger.info(
43edf8
-                    "Use ipa-getkeytab to obtain a host "
43edf8
-                    "principal for this server.")
43edf8
-            else:
43edf8
-                logger.info("Enrolled in IPA realm %s", cli_realm)
43edf8
-
43edf8
-            start = stderr.find('Certificate subject base is: ')
43edf8
-            if start >= 0:
43edf8
-                start = start + 29
43edf8
-                subject_base = stderr[start:]
43edf8
-                subject_base = subject_base.strip()
43edf8
-                subject_base = DN(subject_base)
43edf8
-
43edf8
-            if options.principal is not None:
43edf8
-                run([paths.KDESTROY], raiseonerr=False, env=env)
43edf8
-
43edf8
-            # Obtain the TGT. We do it with the temporary krb5.conf, so that
43edf8
-            # only the KDC we're installing under is contacted.
43edf8
-            # Other KDCs might not have replicated the principal yet.
43edf8
-            # Once we have the TGT, it's usable on any server.
43edf8
-            try:
43edf8
-                kinit_keytab(host_principal, paths.KRB5_KEYTAB, CCACHE_FILE,
43edf8
-                             config=krb_name,
43edf8
-                             attempts=options.kinit_attempts)
43edf8
-                env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = CCACHE_FILE
43edf8
-            except gssapi.exceptions.GSSError as e:
43edf8
-                print_port_conf_info()
43edf8
-                logger.error("Failed to obtain host TGT: %s", e)
43edf8
-                # failure to get ticket makes it impossible to login and bind
43edf8
-                # from sssd to LDAP, abort installation and rollback changes
43edf8
-                raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
+        # Now join the domain
43edf8
+        result = run(
43edf8
+            join_args, raiseonerr=False, env=env, nolog=nolog,
43edf8
+            capture_error=True)
43edf8
+        stderr = result.error_output
43edf8
 
43edf8
-        finally:
43edf8
-            try:
43edf8
-                os.remove(krb_name)
43edf8
-            except OSError:
43edf8
-                logger.error("Could not remove %s", krb_name)
43edf8
-            try:
43edf8
-                os.rmdir(ccache_dir)
43edf8
-            except OSError:
43edf8
-                pass
43edf8
-            try:
43edf8
-                os.remove(krb_name + ".ipabkp")
43edf8
-            except OSError:
43edf8
-                logger.error("Could not remove %s.ipabkp", krb_name)
43edf8
+        if result.returncode != 0:
43edf8
+            logger.error("Joining realm failed: %s", stderr)
43edf8
+            if not options.force:
43edf8
+                if result.returncode == 13:
43edf8
+                    logger.info(
43edf8
+                        "Use --force-join option to override the host "
43edf8
+                        "entry on the server and force client enrollment.")
43edf8
+                raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
+            logger.info(
43edf8
+                "Use ipa-getkeytab to obtain a host "
43edf8
+                "principal for this server.")
43edf8
+        else:
43edf8
+            logger.info("Enrolled in IPA realm %s", cli_realm)
43edf8
+
43edf8
+        start = stderr.find('Certificate subject base is: ')
43edf8
+        if start >= 0:
43edf8
+            start = start + 29
43edf8
+            subject_base = stderr[start:]
43edf8
+            subject_base = subject_base.strip()
43edf8
+            subject_base = DN(subject_base)
43edf8
+
43edf8
+        if options.principal is not None:
43edf8
+            run([paths.KDESTROY], raiseonerr=False, env=env)
43edf8
+
43edf8
+        # Obtain the TGT. We do it with the temporary krb5.conf, so that
43edf8
+        # only the KDC we're installing under is contacted.
43edf8
+        # Other KDCs might not have replicated the principal yet.
43edf8
+        # Once we have the TGT, it's usable on any server.
43edf8
+        try:
43edf8
+            kinit_keytab(host_principal, paths.KRB5_KEYTAB, CCACHE_FILE,
43edf8
+                         config=krb_name,
43edf8
+                         attempts=options.kinit_attempts)
43edf8
+            env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = CCACHE_FILE
43edf8
+        except gssapi.exceptions.GSSError as e:
43edf8
+            print_port_conf_info()
43edf8
+            logger.error("Failed to obtain host TGT: %s", e)
43edf8
+            # failure to get ticket makes it impossible to login and bind
43edf8
+            # from sssd to LDAP, abort installation and rollback changes
43edf8
+            raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
 
43edf8
     # Configure ipa.conf
43edf8
     if not options.on_master:
43edf8
@@ -2692,23 +2707,6 @@ def _install(options):
43edf8
             except gssapi.exceptions.GSSError as e:
43edf8
                 logger.error("Failed to obtain host TGT: %s", e)
43edf8
                 raise ScriptError(rval=CLIENT_INSTALL_ERROR)
43edf8
-        else:
43edf8
-            # Configure krb5.conf
43edf8
-            fstore.backup_file(paths.KRB5_CONF)
43edf8
-            configure_krb5_conf(
43edf8
-                cli_realm=cli_realm,
43edf8
-                cli_domain=cli_domain,
43edf8
-                cli_server=cli_server,
43edf8
-                cli_kdc=cli_kdc,
43edf8
-                dnsok=dnsok,
43edf8
-                filename=paths.KRB5_CONF,
43edf8
-                client_domain=client_domain,
43edf8
-                client_hostname=hostname,
43edf8
-                configure_sssd=options.sssd,
43edf8
-                force=options.force)
43edf8
-
43edf8
-            logger.info(
43edf8
-                "Configured /etc/krb5.conf for IPA realm %s", cli_realm)
43edf8
 
43edf8
         # Clear out any current session keyring information
43edf8
         try:
43edf8
@@ -3020,6 +3018,23 @@ def _install(options):
43edf8
         configure_nisdomain(
43edf8
             options=options, domain=cli_domain, statestore=statestore)
43edf8
 
43edf8
+    # Configure the final krb5.conf
43edf8
+    if not options.on_master:
43edf8
+        fstore.backup_file(paths.KRB5_CONF)
43edf8
+        configure_krb5_conf(
43edf8
+            cli_realm=cli_realm,
43edf8
+            cli_domain=cli_domain,
43edf8
+            cli_server=cli_server,
43edf8
+            cli_kdc=cli_kdc,
43edf8
+            dnsok=dnsok,
43edf8
+            filename=paths.KRB5_CONF,
43edf8
+            client_domain=client_domain,
43edf8
+            client_hostname=hostname,
43edf8
+            configure_sssd=options.sssd,
43edf8
+            force=options.force)
43edf8
+
43edf8
+        logger.info("Configured /etc/krb5.conf for IPA realm %s", cli_realm)
43edf8
+
43edf8
     logger.info('Client configuration complete.')
43edf8
 
43edf8
 
43edf8
-- 
43edf8
2.39.2
43edf8