[krbdev.mit.edu #8628] Logic error in LeashKRB5_renew()

[Greg Hudson via RT]

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