[Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code
Petr Spacek
pspacek at redhat.com
Wed Mar 11 11:42:11 UTC 2015
Hello Martin^3,
good work, we are almost there! Please see my nitpicks in-line.
On 9.3.2015 13:06, Martin Babinsky wrote:
> On 03/06/2015 01:05 PM, Martin Babinsky wrote:
>> This series of patches for the master/4.1 branch attempts to implement
>> some of the Rob's and Petr Vobornik's ideas which originated from a
>> discussion on this list regarding my original patch fixing
>> https://fedorahosted.org/freeipa/ticket/4808.
>>
>> I suppose that these patches are just a first iteration, we may further
>> discuss if this is the right thing to do.
>>
>> Below is a quote from the original discussion just to get the context:
>>
>>
>>
>
> The next iteration of patches is attached below. Thanks to jcholast and
> pvoborni for the comments and insights.
>
> --
> Martin^3 Babinsky
>
> freeipa-mbabinsk-0015-2-ipautil-new-functions-kinit_keytab-and-kinit_passwor.patch
>
>
> From 97e4eed332391bab7a12dc593152e369f347fd3c Mon Sep 17 00:00:00 2001
> From: Martin Babinsky <mbabinsk at redhat.com>
> Date: Mon, 9 Mar 2015 12:53:10 +0100
> Subject: [PATCH 1/3] ipautil: new functions kinit_keytab and kinit_password
>
> kinit_keytab replaces kinit_hostprincipal and performs Kerberos auth using
> keytab file. Function is also able to repeat authentication multiple times
> before giving up and raising StandardError.
>
> kinit_password wraps kinit auth using password and also supports FAST
> authentication using httpd armor ccache.
> ---
> ipapython/ipautil.py | 60 ++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 14 deletions(-)
>
> diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
> index 4116d974e620341119b56fad3cff1bda48af3bab..4547165ccf24ff6edf5c65e756aa321aa34b9e61 100644
> --- a/ipapython/ipautil.py
> +++ b/ipapython/ipautil.py
> @@ -1175,27 +1175,59 @@ def wait_for_open_socket(socket_name, timeout=0):
> else:
> raise e
>
> -def kinit_hostprincipal(keytab, ccachedir, principal):
> +
> +def kinit_keytab(keytab, ccache_path, principal, attempts=1):
> """
> - Given a ccache directory and a principal kinit as that user.
> + Given a ccache_path , keytab file and a principal kinit as that user.
> +
> + The optional parameter 'attempts' specifies how many times the credential
> + initialization should be attempted before giving up and raising
> + StandardError.
>
> This blindly overwrites the current CCNAME so if you need to save
> it do so before calling this function.
>
> Thus far this is used to kinit as the local host.
> """
> - try:
> - ccache_file = 'FILE:%s/ccache' % ccachedir
> - krbcontext = krbV.default_context()
> - ktab = krbV.Keytab(name=keytab, context=krbcontext)
> - princ = krbV.Principal(name=principal, context=krbcontext)
> - os.environ['KRB5CCNAME'] = ccache_file
> - ccache = krbV.CCache(name=ccache_file, context=krbcontext, primary_principal=princ)
> - ccache.init(princ)
> - ccache.init_creds_keytab(keytab=ktab, principal=princ)
> - return ccache_file
> - except krbV.Krb5Error, e:
> - raise StandardError('Error initializing principal %s in %s: %s' % (principal, keytab, str(e)))
> + root_logger.debug("Initializing principal %s using keytab %s"
> + % (principal, keytab))
> + for attempt in xrange(attempts):
I would like to see new code compatible with Python 3. Here I'm not sure what
is the generic solution for xrange but in this particular case I would
recommend you to use just range. Attempts variable should have small values so
the x/range differences do not matter here.
> + try:
> + krbcontext = krbV.default_context()
> + ktab = krbV.Keytab(name=keytab, context=krbcontext)
> + princ = krbV.Principal(name=principal, context=krbcontext)
> + os.environ['KRB5CCNAME'] = ccache_path
This is a bit scary, especially when it comes to multi-threaded execution. If
it is really necessary please add comment that this method is not thread-safe.
> + ccache = krbV.CCache(name=ccache_path, context=krbcontext,
> + primary_principal=princ)
> + ccache.init(princ)
> + ccache.init_creds_keytab(keytab=ktab, principal=princ)
> + root_logger.debug("Attempt %d/%d: success"
> + % (attempt + 1, attempts))
What about adding + 1 to range boundaries instead of + 1 here and there?
> + return
> + except krbV.Krb5Error, e:
except ... , ... syntax is not going to work in Python 3. Maybe 'as' would be
better?
> + root_logger.debug("Attempt %d/%d: failed"
> + % (attempt + 1, attempts))
> + time.sleep(1)
> +
> + root_logger.debug("Maximum number of attempts (%d) reached"
> + % attempts)
> + raise StandardError("Error initializing principal %s: %s"
> + % (principal, str(e)))
> +
> +
> +def kinit_password(principal, password, env={}, armor_ccache=""):
> + """perform interactive kinit as principal using password. Additional
> + enviroment variables can be specified using env. If using FAST for
> + web-based authentication, use armor_ccache to specify http service ccache.
> + """
> + root_logger.debug("Initializing principal %s using password" % principal)
> + args = [paths.KINIT, principal]
> + if armor_ccache:
> + root_logger.debug("Using armor ccache %s for FAST webauth"
> + % armor_ccache)
> + args.extend(['-T', armor_ccache])
> + run(args, env=env, stdin=password)
> +
>
> def dn_attribute_property(private_name):
> '''
> -- 2.1.0
>
>
> freeipa-mbabinsk-0016-2-ipa-client-install-try-to-get-host-TGT-several-times.patch
>
>
> From e438d8a0711b4271d24d7d24e98395503912a1c4 Mon Sep 17 00:00:00 2001
> From: Martin Babinsky <mbabinsk at redhat.com>
> Date: Mon, 9 Mar 2015 12:53:57 +0100
> Subject: [PATCH 2/3] ipa-client-install: try to get host TGT several times
> before giving up
>
> New option '--kinit-attempts' enables the host to make multiple attempts to
> obtain TGT from KDC before giving up and aborting client installation.
>
> In addition, all kinit attempts were replaced by calls to
> 'ipautil.kinit_keytab' and 'ipautil.kinit_password'.
>
> https://fedorahosted.org/freeipa/ticket/4808
> ---
> ipa-client/ipa-install/ipa-client-install | 65 +++++++++++++++++--------------
> ipa-client/man/ipa-client-install.1 | 5 +++
> 2 files changed, 41 insertions(+), 29 deletions(-)
>
> diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
> index ccaab5536e83b4b6ac60b81132c3455c0af19ae1..c817f9e86dbaa6a2cca7d1a463f53d491fa7badb 100755
> --- a/ipa-client/ipa-install/ipa-client-install
> +++ b/ipa-client/ipa-install/ipa-client-install
> @@ -91,6 +91,13 @@ def parse_options():
>
> parser.values.ca_cert_file = value
>
> + def validate_kinit_attempts_option(option, opt, value, parser):
> + if value < 1 or value > sys.maxint:
> + raise OptionValueError(
> + "%s option has invalid value %d" % (opt, value))
It would be nice if the error message said what is the expected value.
("Expected integer in range <1,%s>" % sys.maxint)
BTW is it possible to do this using existing option parser? I would expect
some generic support for type=uint or something similar.
> +
> + parser.values.kinit_attempts = value
> +
> parser = IPAOptionParser(version=version.VERSION)
>
> basic_group = OptionGroup(parser, "basic options")
> @@ -144,6 +151,11 @@ def parse_options():
> help="do not modify the nsswitch.conf and PAM configuration")
> basic_group.add_option("-f", "--force", dest="force", action="store_true",
> default=False, help="force setting of LDAP/Kerberos conf")
> + basic_group.add_option('--kinit-attempts', dest='kinit_attempts',
> + action='callback', type='int', default=5,
It would be good to check lockout numbers in default configuration to make
sure that replication delay will not lock the principal.
> + callback=validate_kinit_attempts_option,
> + help=("number of attempts to obtain host TGT"
> + " (defaults to %default)."))
> basic_group.add_option("-d", "--debug", dest="debug", action="store_true",
> default=False, help="print debugging information")
> basic_group.add_option("-U", "--unattended", dest="unattended",
> @@ -2351,6 +2363,7 @@ def install(options, env, fstore, statestore):
> root_logger.debug(
> "will use principal provided as option: %s", options.principal)
>
> + host_principal = 'host/%s@%s' % (hostname, cli_realm)
> if not options.on_master:
> nolog = tuple()
> # First test out the kerberos configuration
> @@ -2371,7 +2384,6 @@ def install(options, env, fstore, statestore):
> env['KRB5_CONFIG'] = krb_name
> (ccache_fd, ccache_name) = tempfile.mkstemp()
> os.close(ccache_fd)
> - env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = ccache_name
> join_args = [paths.SBIN_IPA_JOIN,
> "-s", cli_server[0],
> "-b", str(realm_to_suffix(cli_realm)),
> @@ -2409,29 +2421,24 @@ def install(options, env, fstore, statestore):
> else:
> stdin = sys.stdin.readline()
>
> - (stderr, stdout, returncode) = run(["kinit", principal],
> - raiseonerr=False,
> - stdin=stdin,
> - env=env)
> - if returncode != 0:
> + try:
> + ipautil.kinit_password(principal, stdin, env)
> + except CalledProcessError, e:
> print_port_conf_info()
> - root_logger.error("Kerberos authentication failed")
> - root_logger.info("%s", stdout)
> + root_logger.error("Kerberos authentication failed: %s"
> + % str(e))
Isn't str() redundant? IMHO %s calls str() implicitly.
> return CLIENT_INSTALL_ERROR
> elif options.keytab:
> join_args.append("-f")
> if os.path.exists(options.keytab):
> - (stderr, stdout, returncode) = run(
> - [paths.KINIT,'-k', '-t', options.keytab,
> - 'host/%s@%s' % (hostname, cli_realm)],
> - env=env,
> - raiseonerr=False)
> -
> - if returncode != 0:
> + try:
> + ipautil.kinit_keytab(options.keytab, ccache_name,
> + host_principal,
> + attempts=options.kinit_attempts)
> + except StandardError, e:
> print_port_conf_info()
> - root_logger.error("Kerberos authentication failed "
> - "using keytab: %s", options.keytab)
> - root_logger.info("%s", stdout)
> + root_logger.error("Kerberos authentication failed: %s"
> + % str(e))
Again str().
> return CLIENT_INSTALL_ERROR
> else:
> root_logger.error("Keytab file could not be found: %s"
> @@ -2501,12 +2508,13 @@ def install(options, env, fstore, statestore):
> # only the KDC we're installing under is contacted.
> # Other KDCs might not have replicated the principal yet.
> # Once we have the TGT, it's usable on any server.
> - env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = CCACHE_FILE
> try:
> - run([paths.KINIT, '-k', '-t', paths.KRB5_KEYTAB,
> - 'host/%s@%s' % (hostname, cli_realm)], env=env)
> - except CalledProcessError, e:
> - root_logger.error("Failed to obtain host TGT.")
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, CCACHE_FILE,
> + host_principal,
> + attempts=options.kinit_attempts)
> + except StandardError, e:
> + print_port_conf_info()
> + root_logger.error("Failed to obtain host TGT: %s" % str(e))
str()?
> # failure to get ticket makes it impossible to login and bind
> # from sssd to LDAP, abort installation and rollback changes
> return CLIENT_INSTALL_ERROR
> @@ -2543,16 +2551,15 @@ def install(options, env, fstore, statestore):
> return CLIENT_INSTALL_ERROR
> root_logger.info("Configured /etc/sssd/sssd.conf")
>
> - host_principal = 'host/%s@%s' % (hostname, cli_realm)
> if options.on_master:
> # If on master assume kerberos is already configured properly.
> # Get the host TGT.
> - os.environ['KRB5CCNAME'] = CCACHE_FILE
> try:
> - run([paths.KINIT, '-k', '-t', paths.KRB5_KEYTAB,
> - host_principal])
> - except CalledProcessError, e:
> - root_logger.error("Failed to obtain host TGT.")
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, CCACHE_FILE,
> + host_principal,
> + attempts=options.kinit_attempts)
> + except StandardError, e:
> + root_logger.error("Failed to obtain host TGT: %s" % str(e))
str()? I will not mention it again but please look for it.
> return CLIENT_INSTALL_ERROR
> else:
> # Configure krb5.conf
> diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
> index 726a6c133132dd2e3ba2fde43d8a2ec0549bfcef..56ed899a25e626b8ae61714f77f3588059fa86f9 100644
> --- a/ipa-client/man/ipa-client-install.1
> +++ b/ipa-client/man/ipa-client-install.1
> @@ -152,6 +152,11 @@ Do not use Authconfig to modify the nsswitch.conf and PAM configuration.
> \fB\-f\fR, \fB\-\-force\fR
> Force the settings even if errors occur
> .TP
> +\fB\-\-kinit\-attempts\fR=\fIKINIT_ATTEMPTS\fR
> +Number of unsuccessful attempts to obtain host TGT that will be performed
> +before aborting client installation. \fIKINIT_ATTEMPTS\fR should be a number
> +greater than zero. By default 5 attempts to get TGT are performed.
It would be nice to add a rationale to the text. Current text is somehow
confusing for users not familiar with replication and related problems.
My hope is descriptive manual will prevent users from creating cargo-cults
based on copy&pastings texts they do not understand.
> +.TP
> \fB\-d\fR, \fB\-\-debug\fR
> Print debugging information to stdout
> .TP
> -- 2.1.0
>
>
> freeipa-mbabinsk-0017-2-Adopted-kinit_keytab-and-kinit_password-for-kerberos.patch
>
>
> From 912113529138e5b1bd8357ae6a17376cb5d32759 Mon Sep 17 00:00:00 2001
> From: Martin Babinsky <mbabinsk at redhat.com>
> Date: Mon, 9 Mar 2015 12:54:36 +0100
> Subject: [PATCH 3/3] Adopted kinit_keytab and kinit_password for kerberos auth
>
> ---
> daemons/dnssec/ipa-dnskeysync-replica | 6 ++-
> daemons/dnssec/ipa-dnskeysyncd | 2 +-
> daemons/dnssec/ipa-ods-exporter | 5 ++-
> .../certmonger/dogtag-ipa-ca-renew-agent-submit | 3 +-
> install/restart_scripts/renew_ca_cert | 7 ++--
> install/restart_scripts/renew_ra_cert | 4 +-
> ipa-client/ipa-install/ipa-client-automount | 9 ++--
> ipa-client/ipaclient/ipa_certupdate.py | 3 +-
> ipaserver/rpcserver.py | 49 +++++++++++-----------
> 9 files changed, 47 insertions(+), 41 deletions(-)
>
> diff --git a/daemons/dnssec/ipa-dnskeysync-replica b/daemons/dnssec/ipa-dnskeysync-replica
> index d04f360e04ee018dcdd1ba9b2ca42b1844617af9..e9cae519202203a10678b7384e5acf748f256427 100755
> --- a/daemons/dnssec/ipa-dnskeysync-replica
> +++ b/daemons/dnssec/ipa-dnskeysync-replica
> @@ -139,14 +139,16 @@ log.setLevel(level=logging.DEBUG)
> # Kerberos initialization
> PRINCIPAL = str('%s/%s' % (DAEMONNAME, ipalib.api.env.host))
> log.debug('Kerberos principal: %s', PRINCIPAL)
> -ipautil.kinit_hostprincipal(paths.IPA_DNSKEYSYNCD_KEYTAB, WORKDIR, PRINCIPAL)
> +ccache_filename = os.path.join(WORKDIR, 'ccache')
BTW I really appreciate this patch set! We finally can use more descriptive
names like 'ipa-dnskeysync-replica.ccache' which sometimes make debugging easier.
> +ipautil.kinit_keytab(paths.IPA_DNSKEYSYNCD_KEYTAB, ccache_filename,
> + PRINCIPAL)
> log.debug('Got TGT')
>
> # LDAP initialization
> ldap = ipalib.api.Backend[ldap2]
> # fixme
> log.debug('Connecting to LDAP')
> -ldap.connect(ccache="%s/ccache" % WORKDIR)
> +ldap.connect(ccache=ccache_filename)
> log.debug('Connected')
>
>
> diff --git a/daemons/dnssec/ipa-dnskeysyncd b/daemons/dnssec/ipa-dnskeysyncd
> index 54a08a1e6307e89b3f52e78bddbe28cda8ac1345..4e64596c7f8ccd6cff47df4772c875917c71606a 100755
> --- a/daemons/dnssec/ipa-dnskeysyncd
> +++ b/daemons/dnssec/ipa-dnskeysyncd
> @@ -65,7 +65,7 @@ log = root_logger
> # Kerberos initialization
> PRINCIPAL = str('%s/%s' % (DAEMONNAME, api.env.host))
> log.debug('Kerberos principal: %s', PRINCIPAL)
> -ipautil.kinit_hostprincipal(KEYTAB_FB, WORKDIR, PRINCIPAL)
> +ipautil.kinit_keytab(KEYTAB_FB, os.path.join(WORKDIR, 'ccache'), PRINCIPAL)
'ipa-dnskeysyncd.ccache'?
> # LDAP initialization
> basedn = DN(api.env.container_dns, api.env.basedn)
> diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter
> index dc1851d3a34bb09c1a87c86d101b11afe35e49fe..0cf825950338cdce330a15b3ea22150f6e02588a 100755
> --- a/daemons/dnssec/ipa-ods-exporter
> +++ b/daemons/dnssec/ipa-ods-exporter
> @@ -399,7 +399,8 @@ ipalib.api.finalize()
> # Kerberos initialization
> PRINCIPAL = str('%s/%s' % (DAEMONNAME, ipalib.api.env.host))
> log.debug('Kerberos principal: %s', PRINCIPAL)
> -ipautil.kinit_hostprincipal(paths.IPA_ODS_EXPORTER_KEYTAB, WORKDIR, PRINCIPAL)
> +ccache_name = os.path.join(WORKDIR, 'ccache')
'ipa-ods-exporter.ccache'?
> +ipautil.kinit_keytab(paths.IPA_ODS_EXPORTER_KEYTAB, ccache_name, PRINCIPAL)
> log.debug('Got TGT')
>
> # LDAP initialization
> @@ -407,7 +408,7 @@ dns_dn = DN(ipalib.api.env.container_dns, ipalib.api.env.basedn)
> ldap = ipalib.api.Backend[ldap2]
> # fixme
> log.debug('Connecting to LDAP')
> -ldap.connect(ccache="%s/ccache" % WORKDIR)
> +ldap.connect(ccache=ccache_name)
> log.debug('Connected')
>
>
> diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
> index 7b91fc61148912c77d0ae962b3847d73e8d0720e..13d2c2a912d2fcf84053d36da5e07fc834f9cf25 100755
> --- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
> +++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
> @@ -440,7 +440,8 @@ def main():
> certs.renewal_lock.acquire()
> try:
> principal = str('host/%s@%s' % (api.env.host, api.env.realm))
> - ipautil.kinit_hostprincipal(paths.KRB5_KEYTAB, tmpdir, principal)
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, os.path.join(tmpdir, 'ccache'),
'dogtag-ipa-ca-renew-agent-submit.ccache'?
> + principal)
>
> profile = os.environ.get('CERTMONGER_CA_PROFILE')
> if profile:
> diff --git a/install/restart_scripts/renew_ca_cert b/install/restart_scripts/renew_ca_cert
> index c7bd5d74c5b4659b3ad66d630653ff6419868d99..67156122bb75f00a4c3f612697092e5bab3723fb 100644
> --- a/install/restart_scripts/renew_ca_cert
> +++ b/install/restart_scripts/renew_ca_cert
> @@ -73,8 +73,9 @@ def _main():
> tmpdir = tempfile.mkdtemp(prefix="tmp-")
> try:
> principal = str('host/%s@%s' % (api.env.host, api.env.realm))
> - ccache = ipautil.kinit_hostprincipal(paths.KRB5_KEYTAB, tmpdir,
> - principal)
> + ccache_filename = '%s/ccache' % tmpdir
'renew_ca_cert.ccache'?
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, ccache_filename,
> + principal)
>
> ca = cainstance.CAInstance(host_name=api.env.host, ldapi=False)
> ca.update_cert_config(nickname, cert, configured_constants)
> @@ -139,7 +140,7 @@ def _main():
> conn = None
> try:
> conn = ldap2(shared_instance=False, ldap_uri=api.env.ldap_uri)
> - conn.connect(ccache=ccache)
> + conn.connect(ccache=ccache_filename)
> except Exception, e:
> syslog.syslog(
> syslog.LOG_ERR, "Failed to connect to LDAP: %s" % e)
> diff --git a/install/restart_scripts/renew_ra_cert b/install/restart_scripts/renew_ra_cert
> index 7dae3562380e919b2cc5f53825820291fc93fdc5..6276de78e4528dc1caa39be6628094a9d00e5988 100644
> --- a/install/restart_scripts/renew_ra_cert
> +++ b/install/restart_scripts/renew_ra_cert
> @@ -42,8 +42,8 @@ def _main():
> tmpdir = tempfile.mkdtemp(prefix="tmp-")
> try:
> principal = str('host/%s@%s' % (api.env.host, api.env.realm))
> - ccache = ipautil.kinit_hostprincipal(paths.KRB5_KEYTAB, tmpdir,
> - principal)
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, '%s/ccache' % tmpdir,
'renew_ra_cert.ccache'?
> + principal)
>
> ca = cainstance.CAInstance(host_name=api.env.host, ldapi=False)
> if ca.is_renewal_master():
> diff --git a/ipa-client/ipa-install/ipa-client-automount b/ipa-client/ipa-install/ipa-client-automount
> index 7b9e701dead5f50a033a455eb62e30df78cc0249..19197d34ca580062742b3d7363e5dfb2dad0e4de 100755
> --- a/ipa-client/ipa-install/ipa-client-automount
> +++ b/ipa-client/ipa-install/ipa-client-automount
> @@ -425,10 +425,11 @@ def main():
> os.close(ccache_fd)
> try:
> try:
> - os.environ['KRB5CCNAME'] = ccache_name
> - ipautil.run([paths.KINIT, '-k', '-t', paths.KRB5_KEYTAB, 'host/%s@%s' % (api.env.host, api.env.realm)])
> - except ipautil.CalledProcessError, e:
> - sys.exit("Failed to obtain host TGT.")
> + host_princ = str('host/%s@%s' % (api.env.host, api.env.realm))
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB, ccache_name,
I'm not sure what is ccache_name here but it should be something descriptive.
> + host_princ)
> + except StandardError, e:
> + sys.exit("Failed to obtain host TGT: %s" % e)
> # Now we have a TGT, connect to IPA
> try:
> api.Backend.rpcclient.connect()
> diff --git a/ipa-client/ipaclient/ipa_certupdate.py b/ipa-client/ipaclient/ipa_certupdate.py
> index 031a34c3a54a02d43978eedcb794678a1550702b..d6f7bbb3daff3ae4dced5d69f83a0516003235ab 100644
> --- a/ipa-client/ipaclient/ipa_certupdate.py
> +++ b/ipa-client/ipaclient/ipa_certupdate.py
> @@ -57,7 +57,8 @@ class CertUpdate(admintool.AdminTool):
> tmpdir = tempfile.mkdtemp(prefix="tmp-")
> try:
> principal = str('host/%s@%s' % (api.env.host, api.env.realm))
> - ipautil.kinit_hostprincipal(paths.KRB5_KEYTAB, tmpdir, principal)
> + ipautil.kinit_keytab(paths.KRB5_KEYTAB,
> + os.path.join(tmpdir, 'ccache'), principal)
'ipa_certupdate.ccache'?
>
> api.Backend.rpcclient.connect()
> try:
> diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
> index d6bc955b9d9910a24eec5df1def579310eb54786..36f16908ac8477d9982bfee613b77576853054eb 100644
> --- a/ipaserver/rpcserver.py
> +++ b/ipaserver/rpcserver.py
> @@ -958,8 +958,8 @@ class login_password(Backend, KerberosSession, HTTP_Status):
>
> def kinit(self, user, realm, password, ccache_name):
> # get http service ccache as an armor for FAST to enable OTP authentication
> - armor_principal = krb5_format_service_principal_name(
> - 'HTTP', self.api.env.host, realm)
> + armor_principal = str(krb5_format_service_principal_name(
> + 'HTTP', self.api.env.host, realm))
> keytab = paths.IPA_KEYTAB
> armor_name = "%sA_%s" % (krbccache_prefix, user)
> armor_path = os.path.join(krbccache_dir, armor_name)
> @@ -967,34 +967,33 @@ class login_password(Backend, KerberosSession, HTTP_Status):
> self.debug('Obtaining armor ccache: principal=%s keytab=%s ccache=%s',
> armor_principal, keytab, armor_path)
>
> - (stdout, stderr, returncode) = ipautil.run(
> - [paths.KINIT, '-kt', keytab, armor_principal],
> - env={'KRB5CCNAME': armor_path}, raiseonerr=False)
> -
> - if returncode != 0:
> - raise CCacheError()
> + try:
> + ipautil.kinit_keytab(paths.IPA_KEYTAB, armor_path,
> + armor_principal)
> + except StandardError, e:
> + raise CCacheError(str(e))
>
> # Format the user as a kerberos principal
> principal = krb5_format_principal_name(user, realm)
>
> - (stdout, stderr, returncode) = ipautil.run(
> - [paths.KINIT, principal, '-T', armor_path],
> - env={'KRB5CCNAME': ccache_name, 'LC_ALL': 'C'},
> - stdin=password, raiseonerr=False)
> + try:
> + ipautil.kinit_password(principal, password,
> + env={'KRB5CCNAME': ccache_name,
> + 'LC_ALL': 'C'},
> + armor_ccache=armor_path)
>
> - self.debug('kinit: principal=%s returncode=%s, stderr="%s"',
> - principal, returncode, stderr)
> -
> - self.debug('Cleanup the armor ccache')
> - ipautil.run(
> - [paths.KDESTROY, '-A', '-c', armor_path],
> - env={'KRB5CCNAME': armor_path},
> - raiseonerr=False)
> -
> - if returncode != 0:
> - if stderr.strip() == 'kinit: Cannot read password while getting initial credentials':
> - raise PasswordExpired(principal=principal, message=unicode(stderr))
> - raise InvalidSessionPassword(principal=principal, message=unicode(stderr))
> + self.debug('Cleanup the armor ccache')
> + ipautil.run(
> + [paths.KDESTROY, '-A', '-c', armor_path],
> + env={'KRB5CCNAME': armor_path},
> + raiseonerr=False)
> + except ipautil.CalledProcessError, e:
> + if ('kinit: Cannot read password while '
> + 'getting initial credentials') in e.output:
I know it is not your code but please make sure it will work with non-English
LANG or LC_MESSAGE.
> + raise PasswordExpired(principal=principal,
> + message=unicode(e.output))
> + raise InvalidSessionPassword(principal=principal,
> + message=unicode(e.output))
>
> class change_password(Backend, HTTP_Status):
Hopefully this review is not too annoying :-)
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list