[PATCH] heimdal - avoid an endless loop when KDC replies KRB5KDC_ERR_SVC_UNAVAILABLE

Uri Simchoni urisimchoni at gmail.com
Mon Jun 15 15:33:41 MDT 2015


!krb5_krbhst_retry_exceeded() is the equivalent of i<context->max_retries,
krb5_krbhst_retry() at the end of the loop is the equivalent of ++i
(which is also executed at the end of the loop).

So unless I'm missing something, logic hasn't changed, it's the same
loop with same exit conditions as before, except the loop counter
doesn't start from zero on subsequent invocation if the history has
not been re-created.

The way I see it:
- krb5_sendto() only returns if it got an answer (success or failure -
doesn't matter) or if all history elements have been tried several
times. For this function, "success" means getting anything back.
- the answer received can be success or error. For most error codes
the outer function does not continue - it's the domain's final answer,
so to speak.

- For KRB5KRB_ERR_RESPONSE_TOO_BIG - history is deleted and  we start
over with TCP.

- KRB5KDC_ERR_SVC_UNAVAILABLE is the equivalent of not getting an
answer at all. In principle, krb5_sendto() should not have returned
upon getting such an answer but it doesn't have the wits to realize
that. So it should continue from where it left off. By calling it
again with a non-null history, it continues with the next history
element (previous one was consumed by get_next()). The only problem
with the original code is that it does not continue __EXACTLY__ from
where it left off - the retry counter is reset to 0, which is what
this patch corrects.


Does this explain things? If I fail to see the problem or change in
behavior, can you give a specific example or an alternative patch?
Thanks,
Uri.


On Mon, Jun 15, 2015 at 11:31 PM, Alexander Bokovoy <ab at samba.org> wrote:
> On Mon, Jun 15, 2015 at 11:07:02PM +0300, Uri Simchoni wrote:
>> This is a fix to heimdal code.
>> We've seen that if samba is making a Kerberos request via Heimdal to a
>> KDC, and the KDC
>> replies with KRB5KDC_ERR_SVC_UNAVAILABLE, then Heimdal enters an endless loop.
>>
>> This happened in a customer site when sending an AS request for a
>> specific user (we still don't know the reason for that) and I also
>> encountered it in the lab working against a DC VM that ran on an
>> overly-crowded hypervisor, but have not been able so far to reproduce
>> it reliably (of course with samba I can just tweak the KDC into
>> returning this :)).
>>
>> The upstream version of Heimdal, according to my best judgement (but
>> not testing), does not have this bug. However the code there is vastly
>> different, so I figured an independent fix is in order, and that it
>> cannot wait for a future heimdal merge.
> One issue I have with this change is that this chunk changes logic:
>
> --- a/source4/heimdal/lib/krb5/send_to_kdc.c
> +++ b/source4/heimdal/lib/krb5/send_to_kdc.c
> @@ -375,7 +375,7 @@ krb5_sendto (krb5_context context,
>
>       krb5_data_zero(receive);
>
> -     for (i = 0; i < context->max_retries; ++i) {
> +     while (!krb5_krbhst_retry_exceeded(context, handle)) {
>          krb5_krbhst_info *hi;
>
>          while (krb5_krbhst_next(context, handle, &hi) == 0) {
>
>
> Now a failure to send a single element of the history will break out and
> leave other potentially not yet delivered history elements and will
> never send them as long as this unlucky history element is in queue.
>
>
> --
> / Alexander Bokovoy


More information about the samba-technical mailing list