Michael Šimáček <msima...@redhat.com> writes:

> On 2015-07-29 19:20, Robbie Harwood wrote:
>> Michael Šimáček <msima...@redhat.com> writes:
>>> -    # The keytab may have stale key material (from older trust-add run)
>>> -    if not os.path.isfile(oneway_ccache_name):
>>> -        oneway_ccache = kinit_keytab(oneway_principal, oneway_keytab_name, 
>>> oneway_ccache_name)
>>> -except krbV.Krb5Error as e:
>>> +    try:
>>> +        # The keytab may have stale key material (from older trust-add run)
>>> +        cred = kinit_keytab(oneway_principal, oneway_keytab_name, 
>>> oneway_ccache_name)
>>> +        # would raise exception if expired
>>> +        cred.lifetime
>>> +    except gssapi.exceptions.ExpiredCredentialsError:
>>> +        cred = kinit_keytab(oneway_principal, oneway_keytab_name, 
>>> oneway_ccache_name)
>>> +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)
>> In general, it's bad practice to catch *all* possible GSS errors unless
>> you intend to display their status and/or abort/raise.  If there's a
>> specific state you want to cope with here, catch the exception related
>> to it, not all of them.  Up above is a place where I think this is done
>> right.
> I haven't found any specific exception for keytab problems, what should 
> I catch?
> But there's a different error, there should be one more attempt to get 
> the credentials there. I'll fix it in the next revision of the patch.

I seem to have misread the nested except blocks, which makes this code
different from the code I thought was similar above it.

I would think the only error that could pop out for keytab problems that
can actually be fixed would be ExpiredCredentialsError, but if you're
seeing more than just that in practice that fetching the keytab again
actually fixes, then I will defer to you.

> Thank you for your feedback, I'll post a next revision of the patch 
> after we clarify how to proceed with the default realm.

Sounds good!

Attachment: signature.asc
Description: PGP signature

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to