Ticket History #8628: Logic error in LeashKRB5_renew()

Michael McConville, using a code analysis tool, discovered a
conditional in LeashKRB5_renew() which is always true:

if (code) {
if ( code != KRB5KDC_ERR_ETYPE_NOSUPP ||
code != KRB5_KDC_UNREACH)
Leash_krb5_error(code, "krb5_get_renewed_creds()", 0,
&ctx, &cc);
goto cleanup;
}

The effect of this apparent mistake is to always report the error,
where the apparent intent is to suppress reporting two specific
errors. Since the current behavior has been in place for a long
time, any behavior changes should be tested.

Going back to the version history in the pismere repository, this
code originally didn't have the second conditional at all (it always
reported an error on any non-zero value of code). In early 2004, it
was changed to suppress reporting KRB5KDC_ERR_ETYPE_NOSUPP; that
commit had this message:

* On Win2003, imported tickets may not have a session key. They
will appear with keytype NULL. These tickets cannot be renewed.
If ticket renewal fails, be sure to perform an Import on them.

* If a ticket renewal fails, and the ticket principal matches the
principal available via the MSLSA ccache, then perform an import.

with associated changes to other code (mostly LeashView.cpp). In
mid-2004, the code was changed to its current form, with the commit
message:

* do not allow the leash import button to be pressed if the default
ccname is MSLSA:

with an associated change to LeashView.cpp.

Suppressing reporting of KRB5KDC_ERR_ETYPE_NOSUPP makes some sense,
if that's the error we got when trying to renew a TGT with no session
key and we want to handle that case silently in the caller.
Suppressing reporting of KRB5_KDC_UNREACH does not make sense to me--
or at least, it doesn't seem to have anything to do with the commit
message.

Mail thread here:
http://mailman.mit.edu/pipermail/krbdev/2017-December/012880.html