From dfc7ec92fa47bf4af9054959003755107d7c58c8 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Fri, 22 Jul 2011 21:07:48 -0500 Subject: [PATCH] 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 | 16 ++++++++++++++++ kadmin/kadmin.c | 12 ++++++++++++ lib/kadm5/chpass_s.c | 22 ++++++++++++++-------- lib/kadm5/context_s.c | 34 ++++++++++++++++++++++++++++++++-- lib/kadm5/create_s.c | 22 ++++++++++++++-------- lib/kadm5/delete_s.c | 13 ++++++++----- lib/kadm5/get_princs_s.c | 14 +++++++++----- lib/kadm5/get_s.c | 12 ++++++++---- lib/kadm5/kadm5_err.et | 2 ++ lib/kadm5/modify_s.c | 11 +++++++---- lib/kadm5/private.h | 1 + lib/kadm5/randkey_s.c | 11 +++++++---- lib/kadm5/rename_s.c | 14 +++++++++----- 13 files changed, 139 insertions(+), 45 deletions(-) diff --git a/kadmin/kadmin-commands.in b/kadmin/kadmin-commands.in index 28d2e9b4c..b79882498 100644 --- a/kadmin/kadmin-commands.in +++ b/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 = "?" diff --git a/kadmin/kadmin.c b/kadmin/kadmin.c index 6e31828af..7c4576752 100644 --- a/kadmin/kadmin.c +++ b/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) { diff --git a/lib/kadm5/chpass_s.c b/lib/kadm5/chpass_s.c index 3d27c8d7f..359f322df 100644 --- a/lib/kadm5/chpass_s.c +++ b/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); } diff --git a/lib/kadm5/context_s.c b/lib/kadm5/context_s.c index 8cd58b1a8..a6bd7b887 100644 --- a/lib/kadm5/context_s.c +++ b/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 diff --git a/lib/kadm5/create_s.c b/lib/kadm5/create_s.c index 632575856..d89c6f228 100644 --- a/lib/kadm5/create_s.c +++ b/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; diff --git a/lib/kadm5/delete_s.c b/lib/kadm5/delete_s.c index 7f8f537b0..20180d6dd 100644 --- a/lib/kadm5/delete_s.c +++ b/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); } diff --git a/lib/kadm5/get_princs_s.c b/lib/kadm5/get_princs_s.c index 55c8f2e98..a351e4390 100644 --- a/lib/kadm5/get_princs_s.c +++ b/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){ diff --git a/lib/kadm5/get_s.c b/lib/kadm5/get_s.c index c40c70d04..b224d507b 100644 --- a/lib/kadm5/get_s.c +++ b/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); diff --git a/lib/kadm5/kadm5_err.et b/lib/kadm5/kadm5_err.et index 35c620e72..f6343cbb2 100644 --- a/lib/kadm5/kadm5_err.et +++ b/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" diff --git a/lib/kadm5/modify_s.c b/lib/kadm5/modify_s.c index aa555da28..347ac2ed1 100644 --- a/lib/kadm5/modify_s.c +++ b/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); } diff --git a/lib/kadm5/private.h b/lib/kadm5/private.h index 168b473d4..dcf9ab4bd 100644 --- a/lib/kadm5/private.h +++ b/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; diff --git a/lib/kadm5/randkey_s.c b/lib/kadm5/randkey_s.c index f5eb71225..a83b3f48d 100644 --- a/lib/kadm5/randkey_s.c +++ b/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); } diff --git a/lib/kadm5/rename_s.c b/lib/kadm5/rename_s.c index 08351290c..2f6076b42 100644 --- a/lib/kadm5/rename_s.c +++ b/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);