On Wed, 2015-03-25 at 11:52 +0100, Martin Babinsky wrote:
> On 03/23/2015 03:13 PM, Simo Sorce wrote:
> > On Mon, 2015-03-23 at 14:22 +0100, Petr Spacek wrote:
> >> 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.)
> >
> > If we have that problem we should instead use a temporary krb5.conf file
> > that lists explicitly only the server we are joining.
> >
> > Simo.
> >
>
> This is already done since ipa-3-0: by default only one server/KDC is
> used during client install so there are actually no problems with
> replication delay, only with KDC timeouts.
>
> Anyway I'm sending updated patches.
LGTM!
Simo.
--
Simo Sorce * Red Hat, Inc * New York
--
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
