[Freeipa-devel] [PATCH 0002] Port from python-krbV to python-gssapi

Mon Aug 24 15:18:30 UTC 2015

On 2015-08-24 14:50, Jan Cholasta wrote:
> On 23.8.2015 23:27, Michael Šimáček wrote:
>>
>>
>> On 2015-08-21 15:52, Michael Šimáček wrote:
>>>
>>>
>>> On 2015-08-20 20:42, Robbie Harwood wrote:
>>>> Michael Šimáček <msimacek at redhat.com> writes:
>>>>
>>>>> On 2015-08-20 12:32, Michael Šimáček wrote:
>>>>>
>>>>>>>>> Michael Šimáček <msimacek at redhat.com> writes:
>>>>>>>>>
>>>>>>>>>> Attaching new revision of the patch. Changes from the previous:
>>>>>>>>>> - ldap2's connect now chooses the bind type same way as in
>>>>>>>>>> ipaldap
>>>>>>>>>> - get_default_realm usages replaced by api.env.realm
>>>>>>>>>> - fixed missing third kinit attempt in trust-fetch-domains
>>>>>>>>>> - removed rewrapping gssapi errors to ccache errors in krb_utils
>>>>>>>>>> - updated some parts of exception handling
>>>>>>
>>>>>> Rebased on top of current master.
>>>>>
>>>>> One of the commits reintroduced krbV dependency that I didn't notice.
>>>>> Attaching updated revision. Only changes against previous revision are
>>>>> in files daemons/dnssec/ipa-dnskeysync-replica and
>>>>> daemons/dnssec/ipa-ods-exporter.
>>>>
>>>> This is much better, thanks!  I've got some comments inline.
>>>>
>>>>> +except gssapi.exceptions.GSSError:
>>>>>       # If there was failure on using keytab, assume it is stale and
>>>>> retrieve again
>>>>>       retrieve_keytab(api, ccache_name, oneway_keytab_name,
>>>>> oneway_principal)
>>>>
>>>> This code still bothers me a bit, but I think fixing it is probably
>>>> beyond the scope of a python-gssapi port.
>>>
>>> The code catches all GSSAPI exceptions and retries to do the same thing
>>> with different keytab. So if there was a problem unrelated to keytab,
>>> the same exception will be raised again. Nothing will be ignored
>>> silently.
>>>
>>>>
>>>>> +    try:
>>>>> +        creds = get_credentials(name=name, ccache_name=ccache_name)
>>>>> +        # property access would raise exception if expired
>>>>> +        if creds.lifetime > 0:
>>>>> +            return creds
>>>>> +    except gssapi.exceptions.ExpiredCredentialsError:
>>>>> +        return None
>>>>
>>>> Per rfc2744, lifetime is unsigned.  It's not immediately clear what
>>>> will
>>>> happen when `creds.lifetime == 0`; perhaps an explicit `return Nune` in
>>>> that case?
>>>
>>> I think the check is probably redundant, gssapi raises exception upon
>>> inquiring expired credentials. In trust-fetch-domains I just access the
>>> lifetime in try-except without using the value, so I could do the same
>>> here. It would be nice if gssapi provided some 'is_valid' or
>>> 'is_expired' method, so I wouldn't need to rely on side-effects of
>>> property access, which is hard to read and confuses pylint.
>>>
>>>>
>>>>>           # Setup LDAP connection
>>>>>           try:
>>>>> -            ctx = krbV.default_context()
>>>>> -            ccache = ctx.default_ccache()
>>>>> -            api.Backend.ldap2.connect(ccache)
>>>>> +            api.Backend.ldap2.connect()
>>>>>               cls.ldap = api.Backend.ldap2
>>>>> -        except krbV.Krb5Error as e:
>>>>> +        except gssapi.exceptions.GSSError:
>>>>>               sys.exit("Must have Kerberos credentials to migrate
>>>>> Winsync users.")
>>>>
>>>> Can you log the error here?  The other places GSSError is being caught
>>>> are doing a great job of either filtering-and-raising or
>>>> logging-and-exiting, so thanks for fixing those.
>>>
>>> Yes, I'll update it in next revision of the patch.
>>>
>>>>
>>>>> +# Ugly hack for test purposes only. GSSAPI has no way to get default
>>>>> ccache
>>>>> +# name, but we don't need it outside test server
>>>>> +def get_default_ccache_name():
>>>>> +    try:
>>>>> +        out = check_output(['klist'])
>>>>> +    except CalledProcessError:
>>>>> +        raise RuntimeError("Default ccache not found. Did you
>>>>> kinit?")
>>>>> +    match = re.match(r'^Ticket cache:\s*(\S+)', out)
>>>>> +    if not match:
>>>>> +        raise RuntimeError("Cannot obtain ccache name")
>>>>> +    return match.group(1)
>>>>
>>>> Yup, this is still ugly.  Ah well, it's only test code.
>>>>
>>>
>>> I was trying to modify the code to not need the variable and just use
>>> the default, but it is used for manipulating it as a file - in
>>> production it is always defined by mod_auth_gssapi. So I'd keep this
>>> as is.
>>>
>>
>>
>> Next revision of the patch. Changes from previous rev: printing
>> exception in ipa_winsync_migrate and more thorough dealing with
>> credentials expiration in krb_utils and trust-fetch-domains.
>
> 1) There is a merge conflict in freeipa.spec.in, please rebase the patch
> on top of current master.
>

Done.

>
> 2) pylint fails with:
>
> ************* Module ipa-ods-exporter
> daemons/dnssec/ipa-ods-exporter:23: [E0611(no-name-in-module), ] No name
> 'GSSError' in module 'gssapi')
> ************* Module ipa-dnskeysync-replica
> daemons/dnssec/ipa-dnskeysync-replica:15: [E0611(no-name-in-module), ]
> No name 'GSSError' in module 'gssapi')
>
> Both failures are caused by "from gssapi import GSSError" statement,
> which should read "from gssapi.exceptions import GSSError".
>

Fixed.

>
> 3) ipa-adtrust-install fails with:
>
> admin password:
>
> Unrecognized error during check of admin rights:
> admin at abc.idm.lab.eng.brq.redhat.com: user not found
>
> Apparently there is a "user-show admin at abc.idm.lab.eng.brq.redhat.com"
> call where a "user-show admin" call should be.
>

Fixed. python-gssapi has a display_as method that could pull the name 
from it, but it doesn't work in current version, therefore using 
partition to split on '@'

>
> 4) ipa-client-automount fails with:
>
> Failed to obtain host TGT: Major (851968): Unspecified GSS failure.
> Minor code may provide more information, Minor (2529639111): Bad format
> in credentials cache
>

gssapi apparently refuses to accept empty file as ccache. In order to 
not create a race condition / security hole by deleting and recreating 
the temp file, I changed it to use per-process kernel keyring (I believe 
non-persistent keyring should be available on all supported platforms).

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-msimacek-0002-8-Port-from-python-krbV-to-python-gssapi.patch
Type: text/x-patch
Size: 73861 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150824/0d19868f/attachment.bin>