Make kadm5_lock() and unlock work, and add kadmin commands for them.

The libkadm5 functions hdb_open() and close around all HDB ops.  This
meant the previous implementation of kadm5_lock() and unlock would
always result in a core dump.  Now we hdb_open() for write in
kadm5_lock() and hdb_close() in kadm5_unlock(), with all kadm5_s_*()
functions now not opening nor closing the HDB when the server context
keep_open flag is set.

Also, there's now kadmin(8) lock and unlock commands.  These are there
primarily as a way to test the kadm5_lock()/unlock() operations, but
MIT's kadmin.local also has lock/unlock commands, and these can be
useful for scripting (though they require much care).

kadmin/kadmin-commands.in

@@ -459,6 +459,22 @@ command = {
 	max_args = "1"
 	help = "Check the realm (if not given, the default realm) for configuration errors."
 }
+command = {
+	name = "lock"
+	function = "lock"
+	argument = ""
+	min_args = "0"
+	max_args = "0"
+	help = "Lock the database for writing (use with care)."
+}
+command = {
+	name = "unlock"
+	function = "unlock"
+	argument = ""
+	min_args = "0"
+	max_args = "0"
+	help = "Unlock the database."
+}
 command = {
 	name = "help"
 	name = "?"

kadmin/kadmin.c

@@ -112,6 +112,18 @@ exit_kadmin (void *opt, int argc, char **argv)
     return 0;
 }
 
+int
+lock(void *opt, int argc, char **argv)
+{
+    return kadm5_lock(kadm_handle);
+}
+
+int
+unlock(void *opt, int argc, char **argv)
+{
+    return kadm5_unlock(kadm_handle);
+}
+
 static void
 usage(int ret)
 {

lib/kadm5/chpass_s.c

@@ -50,9 +50,11 @@ change(void *server_handle,
     int existsp = 0;
 
     memset(&ent, 0, sizeof(ent));
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    return ret;
+    }
 
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ,
 				      HDB_F_DECRYPT|HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
@@ -145,7 +147,8 @@ change(void *server_handle,
 out2:
     hdb_free_entry(context->context, &ent);
 out:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     return _kadm5_error_code(ret);
 }
 
@@ -193,9 +196,11 @@ kadm5_s_chpass_principal_with_key(void *server_handle,
     kadm5_ret_t ret;
 
     memset(&ent, 0, sizeof(ent));
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    return ret;
+    }
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ, 0,
 				      HDB_F_GET_ANY|HDB_F_ADMIN_DATA, &ent);
     if(ret == HDB_ERR_NOENTRY)
@@ -244,6 +249,7 @@ kadm5_s_chpass_principal_with_key(void *server_handle,
 out2:
     hdb_free_entry(context->context, &ent);
 out:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     return _kadm5_error_code(ret);
 }

lib/kadm5/context_s.c

@@ -39,16 +39,46 @@ static kadm5_ret_t
 kadm5_s_lock(void *server_handle)
 {
     kadm5_server_context *context = server_handle;
+    kadm5_ret_t ret;
 
-    return context->db->hdb_lock(context->context, context->db, HDB_WLOCK);
+    if (context->keep_open) {
+	/*
+	 * We open/close around every operation, but we retain the DB
+	 * open if the DB was locked with a prior call to kadm5_lock(),
+	 * so if it's open here that must be because the DB is locked.
+	 */
+	assert( context->db->lock_count > 0 );
+	return KADM5_ALREADY_LOCKED;
+    }
+
+    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+    if (ret)
+	return ret;
+
+    ret = context->db->hdb_lock(context->context, context->db, HDB_WLOCK);
+    if (ret)
+	return ret;
+
+    context->keep_open = 1;
+    return 0;
 }
 
 static kadm5_ret_t
 kadm5_s_unlock(void *server_handle)
 {
     kadm5_server_context *context = server_handle;
+    kadm5_ret_t ret;
 
-    return context->db->hdb_unlock(context->context, context->db);
+    if (!context->keep_open)
+	return KADM5_NOT_LOCKED;
+
+    (void) context->db->hdb_close(context->context, context->db);
+
+    context->keep_open = 0;
+    ret = context->db->hdb_unlock(context->context, context->db);
+    if (ret)
+	return ret;
+    return 0;
 }
 
 static void

lib/kadm5/create_s.c

@@ -131,11 +131,14 @@ kadm5_s_create_principal_with_key(void *server_handle,
     if (ret)
 	goto out;
 
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	goto out;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    goto out;
+    }
     ret = context->db->hdb_store(context->context, context->db, 0, &ent);
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     if (ret)
 	goto out;
     kadm5_log_create (context, &ent.entry);
@@ -183,11 +186,14 @@ kadm5_s_create_principal(void *server_handle,
     if (ret)
 	goto out;
 
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	goto out;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    goto out;
+    }
     ret = context->db->hdb_store(context->context, context->db, 0, &ent);
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     if (ret)
 	goto out;
 

lib/kadm5/delete_s.c

@@ -43,10 +43,12 @@ kadm5_s_delete_principal(void *server_handle, krb5_principal princ)
     hdb_entry_ex ent;
 
     memset(&ent, 0, sizeof(ent));
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret) {
-	krb5_warn(context->context, ret, "opening database");
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret) {
+	    krb5_warn(context->context, ret, "opening database");
+	    return ret;
+	}
     }
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ,
 				      HDB_F_DECRYPT|HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
@@ -70,6 +72,7 @@ kadm5_s_delete_principal(void *server_handle, krb5_principal princ)
 out2:
     hdb_free_entry(context->context, &ent);
 out:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     return _kadm5_error_code(ret);
 }

lib/kadm5/get_princs_s.c

@@ -85,10 +85,13 @@ kadm5_s_get_principals(void *server_handle,
     struct foreach_data d;
     kadm5_server_context *context = server_handle;
     kadm5_ret_t ret;
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret) {
-	krb5_warn(context->context, ret, "opening database");
-	return ret;
+
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret) {
+	    krb5_warn(context->context, ret, "opening database");
+	    return ret;
+	}
     }
     d.exp = expression;
     {
@@ -100,7 +103,8 @@ kadm5_s_get_principals(void *server_handle,
     d.princs = NULL;
     d.count = 0;
     ret = hdb_foreach(context->context, context->db, HDB_F_ADMIN_DATA, foreach, &d);
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     if(ret == 0)
 	ret = add_princ(&d, NULL);
     if(ret == 0){

lib/kadm5/get_s.c

@@ -127,13 +127,17 @@ kadm5_s_get_principal(void *server_handle,
     hdb_entry_ex ent;
 
     memset(&ent, 0, sizeof(ent));
-    ret = context->db->hdb_open(context->context, context->db, O_RDONLY, 0);
-    if(ret)
-	return ret;
+
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDONLY, 0);
+	if(ret)
+	    return ret;
+    }
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ,
 				      HDB_F_DECRYPT|HDB_F_ALL_KVNOS|
 				      HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     if(ret)
 	return _kadm5_error_code(ret);
 

lib/kadm5/kadm5_err.et

@@ -63,3 +63,5 @@ error_code DECRYPT_USAGE_NOSUPP,	"Given usage of kadm5_decrypt() not supported"
 error_code POLICY_OP_NOSUPP,	"Policy operations not supported"
 error_code KEEPOLD_NOSUPP,	"Keep old keys option not supported"
 error_code AUTH_GET_KEYS,	"Operation requires `get-keys' privilege"
+error_code ALREADY_LOCKED,	"Database already locked"
+error_code NOT_LOCKED,		"Database not locked"

lib/kadm5/modify_s.c

@@ -52,9 +52,11 @@ modify_principal(void *server_handle,
     if((mask & KADM5_POLICY) && strcmp(princ->policy, "default"))
 	return KADM5_UNK_POLICY;
 
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    return ret;
+    }
     ret = context->db->hdb_fetch_kvno(context->context, context->db,
 				      princ->principal, HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
     if(ret)
@@ -97,7 +99,8 @@ modify_principal(void *server_handle,
 out2:
     hdb_free_entry(context->context, &ent);
 out:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     return _kadm5_error_code(ret);
 }
 

lib/kadm5/private.h

@@ -92,6 +92,7 @@ typedef struct kadm5_server_context {
     /* */
     kadm5_config_params config;
     HDB *db;
+    int keep_open;
     krb5_principal caller;
     unsigned acl_flags;
     kadm5_log_context log_context;

lib/kadm5/randkey_s.c

@@ -54,9 +54,11 @@ kadm5_s_randkey_principal(void *server_handle,
     kadm5_ret_t ret;
 
     memset(&ent, 0, sizeof(ent));
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    return ret;
+    }
     ret = context->db->hdb_fetch_kvno(context->context, context->db, princ,
 				      HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
     if(ret)
@@ -122,6 +124,7 @@ out3:
 out2:
     hdb_free_entry(context->context, &ent);
 out:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     return _kadm5_error_code(ret);
 }

lib/kadm5/rename_s.c

@@ -48,13 +48,16 @@ kadm5_s_rename_principal(void *server_handle,
     memset(&ent, 0, sizeof(ent));
     if(krb5_principal_compare(context->context, source, target))
 	return KADM5_DUP; /* XXX is this right? */
-    ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
-    if(ret)
-	return ret;
+    if (!context->keep_open) {
+	ret = context->db->hdb_open(context->context, context->db, O_RDWR, 0);
+	if(ret)
+	    return ret;
+    }
     ret = context->db->hdb_fetch_kvno(context->context, context->db,
 				      source, HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent);
     if(ret){
-	context->db->hdb_close(context->context, context->db);
+	if (!context->keep_open)
+	    context->db->hdb_close(context->context, context->db);
 	goto out;
     }
     ret = _kadm5_set_modifier(context, &ent.entry);
@@ -103,7 +106,8 @@ kadm5_s_rename_principal(void *server_handle,
     ret = context->db->hdb_remove(context->context, context->db, oldname);
     ent.entry.principal = oldname;
 out2:
-    context->db->hdb_close(context->context, context->db);
+    if (!context->keep_open)
+	context->db->hdb_close(context->context, context->db);
     hdb_free_entry(context->context, &ent);
 out:
     return _kadm5_error_code(ret);