_kdc_find_etype: do not return success if ret_key != NULL

If _kdc_find_etype() is being called with 'ret_key' != NULL, the
caller is attempting to find an actual principal key.  If 'ret_key'
is NULL then it is seeking a session key type.  Only return an enctype
that is not in the principal key list unless 'ret_key' is NULL.

As part of this change remove 'clientbest' and the associated
logic as it is both unnecessary and can produce an enctype for
which the key cannot be returned.

Change-Id: Iba319e95fc1eac139f00b0cce20e1249482d2c6f

kdc/kerberos5.c

@@ -119,7 +119,7 @@ is_default_salt_p(const krb5_salt *default_salt, const Key *key)
 /*
  * return the first appropriate key of `princ' in `ret_key'.  Look for
  * all the etypes in (`etypes', `len'), stopping as soon as we find
- * one, but preferring one that has default salt
+ * one, but preferring one that has default salt.
  */
 
 krb5_error_code
@@ -131,7 +131,6 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key,
     krb5_error_code ret;
     krb5_salt def_salt;
     krb5_enctype enctype = (krb5_enctype)ETYPE_NULL;
-    krb5_enctype clientbest = (krb5_enctype)ETYPE_NULL;
     const krb5_enctype *p;
     Key *key = NULL;
     int i, k;
@@ -166,13 +165,13 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key,
 		continue;
 
 	    /* check that the client supports it too */
-	    for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++, key = NULL) {
+	    for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++) {
+
 		if (p[i] != etypes[k])
 		    continue;
-		if (clientbest == (krb5_enctype)ETYPE_NULL)
-		    clientbest = p[i];
 
                 /* check target princ support */
+		key = NULL;
                 while (ret != 0 &&
                        hdb_next_enctype2key(context, &princ->entry, NULL,
                                              p[i], &key) == 0) {
@@ -197,12 +196,13 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key,
 	 * weak enctypes in krb5.conf and selects this key selection
 	 * algorithm, then we get exactly what RFC4120 says.
 	 */
-	for(key = NULL, i = 0; ret != 0 && i < len; i++, key = NULL) {
+	for(i = 0; ret != 0 && i < len; i++) {
 
 	    if (krb5_enctype_valid(context, etypes[i]) != 0 &&
 		!_kdc_is_weak_exception(princ->entry.principal, etypes[i]))
 		continue;
 
+	    key = NULL;
 	    while (ret != 0 &&
                    hdb_next_enctype2key(context, &princ->entry, NULL,
 					etypes[i], &key) == 0) {
@@ -218,27 +218,27 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key,
 	}
     }
 
-    if (enctype == (krb5_enctype)ETYPE_NULL &&
+    if (enctype == (krb5_enctype)ETYPE_NULL) {
-        clientbest != (krb5_enctype)ETYPE_NULL) {
-        ret = 0;
-        enctype = clientbest;
-    } else if (enctype == (krb5_enctype)ETYPE_NULL) {
         /*
          * if the service principal is one for which there is a known 1DES
          * exception and no other enctype matches both the client request and
          * the service key list, provide a DES-CBC-CRC key.
          */
-        if (_kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) {
+	if (ret_key == NULL &&
+	    _kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) {
             ret = 0;
             enctype = ETYPE_DES_CBC_CRC;
         } else {
             ret = KRB5KDC_ERR_ETYPE_NOSUPP;
         }
     }
-    if (ret == 0 && ret_enctype != NULL)
+
-        *ret_enctype = enctype;
+    if (ret == 0) {
-    if (ret == 0 && ret_key != NULL)
+	if (ret_enctype != NULL)
-        *ret_key = key;
+	    *ret_enctype = enctype;
+	if (ret_key != NULL)
+	    *ret_key = key;
+    }
 
     krb5_free_salt (context, def_salt);
     return ret;