Ticket History #3334: libkrb5 treats all KDC errors as terminal

Could you be more specific about whether this is for AS_REQ or
TGS_REQ, and about which functions are affected?

Also, note that our code doesn't actually define
KDC_ERR_SVC_UNAVAILABLE. Do you know what its semantics are? Are
there other KDC error codes you feel should be "soft" errors?

---Tom

FYI, we've indicated to Luke in private mail that we would accept a
patch for this.

Here's the patch we tried:

http://www.padl.com/~lukeh/krb5-1.3.5-svc-unavailable.patch

Not elegant, but it appears to work (from inserting error in code path,
unfortunately we can't make the MS KDC force this error).

-- Luke


--

Our customer has requested whether this patch (or something like
it) will be integrated. Did anyone ever have a change to look at
the patch I submitted?

-- Luke


--
www.padl.com | www.lukehoward.com

Overall, I think this is the right approach to use. I'd suggest a
few minor(?) changes, and have some notes:

* Make the error message a little more meaningful to the user.
Frankly, I think it's kind of silly to be telling the end user about
specific internal errors in the KDC, but here we are...

* The sendto and sendto_kdc interfaces are private; we can just
change them, and the callers. It probably means a bigger patch, but
less proliferation of internal interfaces. (And for simplicity,
perhaps a null callback function pointer should mean to return on
receipt of any complete message.)

* After working with a few callback interfaces, I think it's a good
idea to avoid mingling "abandon this connection" flags and OS errors
or errors reported by the other party in a callback interface. The
controlling loop is simpler if the callback can only tell it "keep
going" vs "abandon this connection" vs "we're done", and any error
indications needed but irrelevant to the controlling loop itself can
be stuffed in the callback data structure, to be checked by the
caller when the controlling loop returns. This is especially true if
you can get errors that you might want to report depending on the
results of other calls, so your control loop would potentially have
to track multiple error conditions.

* Rename the typedef krb5_select_kdc_fct so it doesn't refer to KDCs
specifically in the sendto interface, which isn't specific to KDC
exchanges. Maybe something like sendto_response_handler, and maybe
make the function pointer plus data pointer into a struct like was
done with struct sendto_callback_info (which perhaps now should be
renamed since it's not the only callback) for constructing per-server
messages.

* Someday, we could make sendto_kdc return the error structure we've
already parsed (to see if it was SVC_UNAVAILABLE). Currently,
sendto_kdc just passes back a krb5_data. Just noting this for the
future...

* We could also check for RESPONSE_TOO_BIG, and shut down only the
one UDP socket. Our current behavior is to quit out of the whole
sendto loop when we get the response, and shut down all the UDP and
TCP sockets; then we analyze the returned message, see that it's
RESPONSE_TOO_BIG, look up the TCP KDC service again, and start up the
TCP connections again. That's pretty wasteful. Keeping the other
UDP sockets open (and, in fact, possibly opening new UDP sockets)
until we get back RESPONSE_TOO_BIG on them is also wasteful, but
perhaps not quite as bad.

Ken

Ken,

What's the status on this -- are you waiting for me to provide a
candidate implementation?

I can't remember where we left it.

-- Luke

Debugged version of patch worked up with Luke.

Adds a callback to krb5int_sendto to examine the response and indicate
whether to quit the loop or not. For sendto_kdc, keep going if the
returned error is "service unavailable". Updated all other callers to
pass a null function pointer, which means to always break out of the
loop on any response (the old behavior).


Commit By: raeburn



Revision: 19738
Changed Files:
_U trunk/
U trunk/src/include/k5-int.h
U trunk/src/lib/krb4/send_to_kdc.c
U trunk/src/lib/krb5/error_tables/krb5_err.et
U trunk/src/lib/krb5/os/changepw.c
U trunk/src/lib/krb5/os/send524.c
U trunk/src/lib/krb5/os/sendto_kdc.c

The change to the signature of an accessor function probably requires
bumping the version number on the accessor.

bump accessor version number

Commit By: tlyu



Revision: 19911
Changed Files:
_U trunk/
U trunk/src/include/k5-int.h

Ken has the current patch, so you'd need to ask him.

Tom Yu via RT wrote:


--
www.padl.com | www.lukehoward.com

pull up r19456 from trunk (prereq for r19738)

r19456@cathode-dark-space: raeburn | 2007-04-13 01:44:38 -0400
Produce a more informative error message for KDC_UNREACH with KDC not responding.


Commit By: tlyu



Revision: 19933
Changed Files:
_U branches/krb5-1-6/
U branches/krb5-1-6/src/lib/krb5/os/sendto_kdc.c