krb5 commit: Adjust keytab kvno workarounds

[Greg Hudson]

https://github.com/krb5/krb5/commit/73bdca02cd4c0f908affeea32a1693535955a766
commit 73bdca02cd4c0f908affeea32a1693535955a766
Author: Greg Hudson <ghudson at mit.edu>
Date:   Sun Mar 8 16:52:11 2015 -0400

    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

 src/lib/krb5/keytab/kt_file.c |   72 ++++++++++++++++++++++-------------------
 1 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/src/lib/krb5/keytab/kt_file.c b/src/lib/krb5/keytab/kt_file.c
index a54a06b..ace654d 100644
--- 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);