Adjust keytab kvno workarounds

In krb5_ktfile_get_entry(), change the pivot and fuzzy match
workarounds for kvnos to work with the 32-bit kvno extension.  For the
pivot logic, try to recognize kvno wraparound at boundary by looking
at the relative timestamps and the size of the version difference.
For the fuzzy match logic, remember the first match against the low 8
bits of the desired kvno, but keep searching for an exact match.

ticket: 7532

diff --git a/src/lib/krb5/keytab/kt_file.c b/src/lib/krb5/keytab/kt_file.c
index a54a06b32080bccf0ad715473af309c94ea4ef70..ace654da02157a698c5f45f374649f13d958d87b 100644 (file)
--- a/src/lib/krb5/keytab/kt_file.c
+++ b/src/lib/krb5/keytab/kt_file.c
@@ -253,6 +253,26 @@ krb5_ktfile_close(krb5_context context, krb5_keytab id)
     return (0);
 }
 
+/* Return true if k1 is more recent than k2, applying wraparound heuristics. */
+static krb5_boolean
+more_recent(const krb5_keytab_entry *k1, const krb5_keytab_entry *k2)
+{
+    /*
+     * If a small kvno was written at the same time or later than a large kvno,
+     * the kvno probably wrapped at some boundary, so consider the small kvno
+     * more recent.  Wraparound can happen due to pre-1.14 keytab file format
+     * limitations (8-bit kvno storage), pre-1.14 kadmin protocol limitations
+     * (8-bit kvno marshalling), or KDB limitations (16-bit kvno storage).
+     */
+    if (k1->timestamp >= k2->timestamp && k1->vno < 128 && k2->vno > 240)
+        return TRUE;
+    if (k1->timestamp <= k2->timestamp && k1->vno > 240 && k2->vno < 128)
+        return FALSE;
+
+    /* Otherwise do a simple version comparison. */
+    return k1->vno > k2->vno;
+}
+
 /*
  * This is the get_entry routine for the file based keytab implementation.
  * It opens the keytab file, and either retrieves the entry or returns
@@ -268,7 +288,6 @@ krb5_ktfile_get_entry(krb5_context context, krb5_keytab id,
     krb5_error_code kerror = 0;
     int found_wrong_kvno = 0;
     krb5_boolean similar;
-    int kvno_offset = 0;
     int was_open;
     char *princname;
 
@@ -339,46 +358,33 @@ krb5_ktfile_get_entry(krb5_context context, krb5_keytab id,
         }
 
         if (kvno == IGNORE_VNO) {
-            /* if this is the first match, or if the new vno is
-               bigger, free the current and keep the new.  Otherwise,
-               free the new. */
-            /* A 1.2.x keytab contains only the low 8 bits of the key
-               version number.  Since it can be much bigger, and thus
-               the 8-bit value can wrap, we need some heuristics to
-               figure out the "highest" numbered key if some numbers
-               close to 255 and some near 0 are used.
-
-               The heuristic here:
-
-               If we have any keys with versions over 240, then assume
-               that all version numbers 0-127 refer to 256+N instead.
-               Not perfect, but maybe good enough?  */
-
-#define M(VNO) (((VNO) - kvno_offset + 256) % 256)
-
-            if (new_entry.vno > 240)
-                kvno_offset = 128;
-            if (! cur_entry.principal ||
-                M(new_entry.vno) > M(cur_entry.vno)) {
+            /* If this entry is more recent (or the first match), free the
+             * current and keep the new.  Otherwise, free the new. */
+            if (cur_entry.principal == NULL ||
+                more_recent(&new_entry, &cur_entry)) {
                 krb5_kt_free_entry(context, &cur_entry);
                 cur_entry = new_entry;
             } else {
                 krb5_kt_free_entry(context, &new_entry);
             }
         } else {
-            /* if this kvno matches, free the current (will there ever
-               be one?), keep the new, and break out.  Otherwise, remember
-               that we were here so we can return the right error, and
-               free the new */
-            /* Yuck.  The krb5-1.2.x keytab format only stores one byte
-               for the kvno, so we're toast if the kvno requested is
-               higher than that.  Short-term workaround: only compare
-               the low 8 bits.  */
-
-            if (new_entry.vno == (kvno & 0xff)) {
+            /*
+             * If this kvno matches exactly, free the current, keep the new,
+             * and break out.  If it matches the low 8 bits of the desired
+             * kvno, remember the first match (because the recorded kvno may
+             * have been truncated due to pre-1.14 keytab format or kadmin
+             * protocol limitations) but keep looking for an exact match.
+             * Otherwise, remember that we were here so we can return the right
+             * error, and free the new.
+             */
+            if (new_entry.vno == kvno) {
                 krb5_kt_free_entry(context, &cur_entry);
                 cur_entry = new_entry;
-                break;
+                if (new_entry.vno == kvno)
+                    break;
+            } else if (new_entry.vno == (kvno & 0xff) &&
+                       cur_entry.principal == NULL) {
+                cur_entry = new_entry;
             } else {
                 found_wrong_kvno++;
                 krb5_kt_free_entry(context, &new_entry);