Re: [Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
On 03/06/2015 02:45 PM, Martin Babinsky wrote: On 03/06/2015 02:08 PM, Jan Cholasta wrote: Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): 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: 3) I think it would be nice to support ccache types other than FILE. According to Petr Vobornik (see his reply), the user is limited mostly to FILE ccache type, so I don't know if it will make sense to support also other types. Actually I agree with Honza. I wanted to say that with your implementation user can't use anything else - are limited to FILE type. -- Petr Vobornik -- 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
[Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
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: -- Martin^3 Babinsky Martin Babinsky wrote: On 03/02/2015 04:28 PM, Rob Crittenden wrote: Petr Vobornik wrote: On 01/12/2015 05:45 PM, Martin Babinsky wrote: related to ticket https://fedorahosted.org/freeipa/ticket/4808 this patch seems to be a bit forgotten. It works, looks fine. One minor issue: trailing whitespaces in the man page. I also wonder if it shouldn't be used in other tools which call kinit with keytab: * ipa-client-automount:434 * ipa-client-install:2591 (this usage should be fine since it's used for server installation) * dcerpc.py:545 * rpcserver.py: 971, 981 (armor for web ui forms base auth) Most importantly the ipa-client-automount because it's called from ipa-client-install (if location is specified) and therefore it might fail during client installation. Or also, kinit call with admin creadentials worked for the user but I wonder if it was just a coincidence and may break under slightly different but similar conditions. I think that's a fine idea. In fact there is already a function that could be extended, kinit_hostprincipal(). rob So in principle we could add multiple TGT retries to kinit_hostprincipal() and then plug this function to all the places Petr mentioned in order to provide this functionality each time TGT is requested using keytab. Do I understand it correctly? Honestly I think I'd only do the retries on client installation. I don't know that the other uses would really benefit or need this. But this is an opportunity to consolidate some code, so I guess the approach I'd take is to add an option to kinit_hostprincipal of retries=0 so that only a single kinit is done. The client installers would pass in some value. This change is quite a bit more invasive but it's also early in the release cycle so the risk will be spread out. rob From fda86199e97aa661e4ee3e73858966c7086a3ee0 Mon Sep 17 00:00:00 2001 From: Martin Babinsky mbabi...@redhat.com Date: Fri, 6 Mar 2015 12:40:42 +0100 Subject: [PATCH 1/5] modifications to ipautil.kinit_hostprincipal 1.) the function can now perform multiple attempts to get host TGT before failing. 2.) instead of a directory name specifiying the location of credentials cache the function now takes the full path including ccache filename. --- ipapython/ipautil.py | 46 -- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 4116d974e620341119b56fad3cff1bda48af3bab..90a8d4035bce218c4cd000c9434125131b311dd9 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -1175,27 +1175,45 @@ def wait_for_open_socket(socket_name, timeout=0): else: raise e -def kinit_hostprincipal(keytab, ccachedir, principal): +def kinit_hostprincipal(keytab, ccache_name, principal, attempts=1): -Given a ccache directory and a principal kinit as that user. +Given a ccache_name 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))) +curr_attempt = 0 +ccache_file = 'FILE:%s' % ccache_name +root_logger.debug(Initializing principal %s % principal) +while True: +curr_attempt += 1 +try: +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) +
Re: [Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
On 03/06/2015 02:08 PM, Jan Cholasta wrote: Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): 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: 1) Why 5 patches for 2 changes (kinit_hostprincipal instead of exec kinit, ipa-client-install --kinit-attempts)? Will squish them to a smaller number (2-3) 2) IMO a for loop would be better than an infinite while loop: for attempt in range(attempts): try: # kinit ... except krbV.Krb5Error as e: # kinit failed ... else: break else: # max attempts reached ... That's true. Infinite loops are tad scary anyway. 3) I think it would be nice to support ccache types other than FILE. According to Petr Vobornik (see his reply), the user is limited mostly to FILE ccache type, so I don't know if it will make sense to support also other types. 4) I would prefer if you kept using the full ccache name returned from kinit_hostprincipal when connecting to LDAP. 5) Given that the ccache path usually ends with /ccache, I would retain the old way of calling kinit_hostprincipal. You can do something like this to support all of the above: def kinit_hostprincipal(keytab, ccache_file, principal, attempts=1): if os.path.isdir(ccache_file): ccache_file = os.path.join(ccache_file, 'ccache') ... return ccache_file (You don't need to prepend FILE:, as it is the default ccache type.) Honza Dumb me didn't realize that 'ccache_file' is a reference to an actual filesystem path and that the filename can be set dynamically depending on path type (directory vs. file). Thank you for your comments. Will update the patches accordingly. -- Martin^3 Babinsky -- 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
Re: [Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
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 original kinit_hostprincipal had `ccachedir` argument, the new one has `ccache_name`. But the new code still prepends FILE ccache type: old: ccache_file = 'FILE:%s/ccache' % ccachedir new: ccache_file = 'FILE:%s' % ccache_name I would remove the line because I understand the use of 'ccache_name' name as equivalent of KRB5CCNAME and therefore I would expect that the value of this argument would be used to set the environment variable WITHOUT any modification. And mainly, user is limited only to FILE ccache type. I also wonder if os.environ['KRB5CCNAME'] = ccache_file has to be set when ccache is defined by krbV call: ccache = krbV.CCache(name=ccache_file, ... krbV snipped doesn't use it so maybe we can remove it. https://git.fedorahosted.org/cgit/python-krbV.git/tree/krbV-code-snippets.py -- Petr Vobornik -- 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
Re: [Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): 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: 1) Why 5 patches for 2 changes (kinit_hostprincipal instead of exec kinit, ipa-client-install --kinit-attempts)? 2) IMO a for loop would be better than an infinite while loop: for attempt in range(attempts): try: # kinit ... except krbV.Krb5Error as e: # kinit failed ... else: break else: # max attempts reached ... 3) I think it would be nice to support ccache types other than FILE. 4) I would prefer if you kept using the full ccache name returned from kinit_hostprincipal when connecting to LDAP. 5) Given that the ccache path usually ends with /ccache, I would retain the old way of calling kinit_hostprincipal. You can do something like this to support all of the above: def kinit_hostprincipal(keytab, ccache_file, principal, attempts=1): if os.path.isdir(ccache_file): ccache_file = os.path.join(ccache_file, 'ccache') ... return ccache_file (You don't need to prepend FILE:, as it is the default ccache type.) Honza -- Jan Cholasta -- 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