From c6d0793e63120dcac393e188bcdfc7590d8b5e75 Mon Sep 17 00:00:00 2001
From: Nicolas Williams <nico@twosigma.com>
Date: Tue, 17 Sep 2019 16:21:29 -0500
Subject: [PATCH] Do not recover log in kadm5_get_principal()

---
 lib/kadm5/get_s.c        | 21 ++++++++-------------
 tests/kdc/check-iprop.in |  8 ++++++--
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/kadm5/get_s.c b/lib/kadm5/get_s.c
index 7963b9ba9..6c8655740 100644
--- a/lib/kadm5/get_s.c
+++ b/lib/kadm5/get_s.c
@@ -123,31 +123,26 @@ kadm5_s_get_principal(void *server_handle,
     kadm5_server_context *context = server_handle;
     kadm5_ret_t ret;
     hdb_entry_ex ent;
-    int hdb_is_rw = 1;
 
     memset(&ent, 0, sizeof(ent));
     memset(out, 0, sizeof(*out));
 
     if (!context->keep_open) {
-	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-        if (ret == EPERM || ret == EACCES) {
-            ret = context->db->hdb_open(context->context, context->db, O_RDONLY, 0);
-            hdb_is_rw = 0;
-        }
+        ret = context->db->hdb_open(context->context, context->db, O_RDONLY, 0);
 	if (ret)
 	    return ret;
     }
 
     /*
-     * Attempt to recover the log.  This will generally fail on slaves,
-     * and we can't tell if we're on a slave here.
+     * We may want to attempt to recover the log on read operations, but we
+     * because the HDB/log lock order is reversed on slaves, in order to avoid
+     * lock contention from kadm5srv apps we need to make sure that the the HDB
+     * open for read-write is optimistic and attempts only a non-blocking lock,
+     * and if it doesn't get it then it should fallback to read-only.  But we
+     * don't have that option in the hdb_open() interface at this time.
      *
-     * Perhaps we could set a flag in the kadm5_server_context to
-     * indicate whether a read has been done without recovering the log,
-     * in which case we could fail any subsequent writes.
+     * For now we won't attempt to recover the log.
      */
-    if (hdb_is_rw && kadm5_log_init_nb(context) == 0)
-        (void) kadm5_log_end(context);
 
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ,
 				      HDB_F_DECRYPT|HDB_F_ALL_KVNOS|
diff --git a/tests/kdc/check-iprop.in b/tests/kdc/check-iprop.in
index af6d9c253..8f32b14a6 100644
--- a/tests/kdc/check-iprop.in
+++ b/tests/kdc/check-iprop.in
@@ -157,7 +157,8 @@ echo foo > ${objdir}/foopassword
 echo "Test log recovery"
 ${kadmin} -l add --random-key --use-defaults recovtest@${R} || exit 1
 # Test theory: save the log, make a change and save the record it
-# produced, restore the log, append to it the saved record, then get
+# produced, restore the log, append to it the saved record, then add dummy
+# record.
 
 # Save the log
 cp current.log current.log.tmp
@@ -180,7 +181,10 @@ mv current.log.tmp current.log
 # Append the saved record
 cat current.log.tmp.saved-record >> current.log
 rm current.log.tmp.saved-record
-# Check that we still see the principal as modified
+# Check that we still see the principal as modified after another write forcing
+# log recovery.
+${kadmin} -l add --random-key --use-defaults dummy@${R} || exit 1
+${kadmin} -l del dummy@${R} || exit 1
 ${kadmin} -l get recovtest@${R} | grep 'Attributes: requires-pre-auth$' > /dev/null || exit 1
 
 # -- foo