[Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Petr Spacek]

On 23.3.2015 14:08, Simo Sorce wrote:
> On Mon, 2015-03-23 at 12:48 +0100, Martin Babinsky wrote:
>> On 03/17/2015 06:00 PM, Simo Sorce wrote:
>>> On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:
>>>> On 03/16/2015 12:15 PM, Martin Kosek wrote:
>>>>> On 03/13/2015 05:37 PM, Martin Babinsky wrote:
>>>>>> Attaching the next iteration of patches.
>>>>>>
>>>>>> I have tried my best to reword the ipa-client-install man page bit about the
>>>>>> new option. Any suggestions to further improve it are welcome.
>>>>>>
>>>>>> I have also slightly modified the 'kinit_keytab' function so that in Kerberos
>>>>>> errors are reported for each attempt and the text of the last error is retained
>>>>>> when finally raising exception.
>>>>>
>>>>> The approach looks very good. I think that my only concern with this patch is
>>>>> this part:
>>>>>
>>>>> +            ccache.init_creds_keytab(keytab=ktab, principal=princ)
>>>>> ...
>>>>> +        except krbV.Krb5Error as e:
>>>>> +            last_exc = str(e)
>>>>> +            root_logger.debug("Attempt %d/%d: failed: %s"
>>>>> +                              % (attempt, attempts, last_exc))
>>>>> +            time.sleep(1)
>>>>> +
>>>>> +    root_logger.debug("Maximum number of attempts (%d) reached"
>>>>> +                      % attempts)
>>>>> +    raise StandardError("Error initializing principal %s: %s"
>>>>> +                        % (principal, last_exc))
>>>>>
>>>>> The problem here is that this function will raise the super-generic
>>>>> StandardError instead of the proper with all the context and information about
>>>>> the error that the caller can then process.
>>>>>
>>>>> I think that
>>>>>
>>>>>       except krbV.Krb5Error as e:
>>>>>           if attempt == max_attempts:
>>>>>               log something
>>>>>               raise
>>>>>
>>>>> would be better.
>>>>>
>>>>
>>>> Yes that seems reasonable. I'm just thinking whether we should re-raise
>>>> Krb5Error or raise ipalib.errors.KerberosError? the latter options makes
>>>> more sense to me as we would not have to additionally import Krb5Error
>>>> everywhere and it would also make the resulting errors more consistent.
>>>>
>>>> I am thinking about someting like this:
>>>>
>>>>       except krbV.Krb5Error as e:
>>>>          if attempt == attempts:
>>>>              # log that we have reaches maximum number of attempts
>>>>              raise KerberosError(minor=str(e))
>>>>
>>>> What do you think?
>>>
>>> Are you retrying on any error ?
>>> Please do *not* do that, if you retry many times on an error that
>>> indicates the password is wrong you may end up locking an administrative
>>> account. If you want to retry you should do it only for very specific
>>> timeout errors.
>>>
>>> Simo.
>>>
>>>
>> I have taken a look at the logs attached to the original BZ 
>> (https://bugzilla.redhat.com/show_bug.cgi?id=1161722).
>>
>> In ipaclient-install.log the kinit error is:
>>
>> "Cannot contact any KDC for realm 'ITW.USPTO.GOV' while getting initial 
>> credentials"
>>
>> which can be translated to krbV.KRB5_KDC_UNREACH error. However, 
>> krb5kdc.log (http://pastebin.test.redhat.com/271394) reports errors 
>> which are seemingly unrelated to the root cause (kinit timing out on 
>> getting host TGT).
>>
>> Thus I'm not quite sure which errors should we chceck against in this 
>> case, anyone care to advise? These are potential candidates:
>>
>> KRB5KDC_ERR_SVC_UNAVAILABLE, "A service is not available that is 
>> required to process the request"
>> KRB5KRB_ERR_RESPONSE_TOO_BIG,    "Response too big for UDP, retry with TCP"
>> KRB5_REALM_UNKNOWN,      "Cannot find KDC for requested realm"
>> KRB5_KDC_UNREACH,        "Cannot contact any KDC for requested realm"
>>
> 
> The only ones that you should retry on, at first glance are
> KRB5_KDC_UNREACH, KRB5KDC_ERR_SVC_UNAVAILABLE.
> 
> You should never see KRB5KRB_ERR_RESPONSE_TOO_BIG in the script as it
> should be handled automatically by the library, and if you get
> KRB5_REALM_UNKNOWN I do not think that retrying will make any
> difference.

I might be wrong but I was under the impression that this feature was also for
workarounding replication delay - service is not available / key is not
present / something like that.

(This could happen if host/principal was added to one server but then the
client connected to another server or so.)

-- 
Petr^2 Spacek