From f62b00e33cff67151bb3da5cb62027502da32f5f Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Thu, 27 Dec 2018 11:58:26 +1100 Subject: [PATCH] kadm5: improve kadm5 hook logging (#397) Centralize logging for kadm5 hook failure, log successful hook loading, better logging on hook load failures and on platforms that do not support dlopen(). --- lib/kadm5/chpass_s.c | 7 ++-- lib/kadm5/create_s.c | 7 ++-- lib/kadm5/delete_s.c | 7 ++-- lib/kadm5/modify_s.c | 7 ++-- lib/kadm5/randkey_s.c | 7 ++-- lib/kadm5/rename_s.c | 7 ++-- lib/kadm5/server_hooks.c | 71 ++++++++++++++++++++++++---------------- lib/kadm5/setkey3_s.c | 7 ++-- 8 files changed, 57 insertions(+), 63 deletions(-) diff --git a/lib/kadm5/chpass_s.c b/lib/kadm5/chpass_s.c index 0a76e0575..2e4999290 100644 --- a/lib/kadm5/chpass_s.c +++ b/lib/kadm5/chpass_s.c @@ -56,11 +56,8 @@ chpass_principal_hook(kadm5_server_context *context, stage, code, princ, flags, n_ks_tuple, ks_tuple, password); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "chpass hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "chpass", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/create_s.c b/lib/kadm5/create_s.c index 0340dc144..d6c30444d 100644 --- a/lib/kadm5/create_s.c +++ b/lib/kadm5/create_s.c @@ -120,11 +120,8 @@ create_principal_hook(kadm5_server_context *context, ret = hook->hook->create(context->context, hook->data, stage, code, princ, mask, password); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "create hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "create", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/delete_s.c b/lib/kadm5/delete_s.c index 55236fb5b..d315e792c 100644 --- a/lib/kadm5/delete_s.c +++ b/lib/kadm5/delete_s.c @@ -51,11 +51,8 @@ delete_principal_hook(kadm5_server_context *context, ret = hook->hook->delete(context->context, hook->data, stage, code, princ); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "delete hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "delete", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/modify_s.c b/lib/kadm5/modify_s.c index 27d9f5dae..2e13fd9a0 100644 --- a/lib/kadm5/modify_s.c +++ b/lib/kadm5/modify_s.c @@ -52,11 +52,8 @@ modify_principal_hook(kadm5_server_context *context, ret = hook->hook->modify(context->context, hook->data, stage, code, princ, mask); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "modify hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "modify", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/randkey_s.c b/lib/kadm5/randkey_s.c index 61b390a58..83fce48d1 100644 --- a/lib/kadm5/randkey_s.c +++ b/lib/kadm5/randkey_s.c @@ -51,11 +51,8 @@ randkey_principal_hook(kadm5_server_context *context, ret = hook->hook->randkey(context->context, hook->data, stage, code, princ); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "randkey hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "randkey", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/rename_s.c b/lib/kadm5/rename_s.c index 78f28a3f1..2e10e0a4b 100644 --- a/lib/kadm5/rename_s.c +++ b/lib/kadm5/rename_s.c @@ -52,11 +52,8 @@ rename_principal_hook(kadm5_server_context *context, ret = hook->hook->rename(context->context, hook->data, stage, code, source, target); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "rename hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "rename", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; } diff --git a/lib/kadm5/server_hooks.c b/lib/kadm5/server_hooks.c index 5a3cc0c5c..46c14e495 100644 --- a/lib/kadm5/server_hooks.c +++ b/lib/kadm5/server_hooks.c @@ -31,6 +31,8 @@ */ #include "kadm5_locl.h" + +#ifdef HAVE_DLFCN_H #include #ifndef RTLD_NOW @@ -44,21 +46,37 @@ #ifndef RTLD_GROUP #define RTLD_GROUP 0 #endif +#endif /* HAVE_DLFCN_H */ + +void +_kadm5_s_set_hook_error_message(kadm5_server_context *context, + krb5_error_code ret, + const char *op, + const struct kadm5_hook *hook, + enum kadm5_hook_stage stage) +{ + assert(ret != 0); + + krb5_set_error_message(context->context, ret, + "%s hook `%s' failed %s-commit", + op, hook->name, + stage == KADM5_HOOK_STAGE_PRECOMMIT ? "pre" : "post"); +} /* * Load kadmin server hooks. */ -#ifdef HAVE_DLOPEN - kadm5_ret_t _kadm5_s_init_hooks(kadm5_server_context *ctx) { krb5_context context = ctx->context; char **hooks; - size_t i; void *handle = NULL; struct kadm5_hook_context *hook_context = NULL; +#ifdef HAVE_DLOPEN struct kadm5_hook_context **tmp; + size_t i; +#endif kadm5_ret_t ret = KADM5_BAD_SERVER_HOOK; hooks = krb5_config_get_strings(context, NULL, @@ -66,6 +84,7 @@ _kadm5_s_init_hooks(kadm5_server_context *ctx) if (hooks == NULL) return 0; +#ifdef HAVE_DLOPEN for (i = 0; hooks[i] != NULL; i++) { const char *hookpath = hooks[i]; kadm5_hook_init_t hook_init; @@ -81,8 +100,8 @@ _kadm5_s_init_hooks(kadm5_server_context *ctx) hook_init = dlsym(handle, "kadm5_hook_init"); if (hook_init == NULL) { - krb5_warnx(context, "didn't find `kadm5_hook_init' symbol in `%s':" - " %s", hookpath, dlerror()); + krb5_warnx(context, "didn't find kadm5_hook_init symbol in `%s': %s", + hookpath, dlerror()); ret = KADM5_BAD_SERVER_HOOK; goto fail; } @@ -91,7 +110,7 @@ _kadm5_s_init_hooks(kadm5_server_context *ctx) if (ret == 0 && hook == NULL) ret = KADM5_BAD_SERVER_HOOK; if (ret) { - krb5_warn(context, ret, "initialize of hook `%s' failed", hookpath); + krb5_warn(context, ret, "initialization of hook `%s' failed", hookpath); goto fail; } @@ -100,16 +119,18 @@ _kadm5_s_init_hooks(kadm5_server_context *ctx) else if (hook->version > KADM5_HOOK_VERSION_V1) ret = KADM5_NEW_SERVER_HOOK_VERSION; if (ret) { - krb5_warnx(context, "version of loaded hook `%s' is %u" - " (supported versions %u to %u)", hookpath, hook->version, + krb5_warnx(context, "%s: version of loaded hook `%s' by vendor `%s' is %u" + " (supported versions are %u to %u)", + hookpath, hook->name, hook->vendor, hook->version, KADM5_HOOK_VERSION_V1, KADM5_HOOK_VERSION_V1); hook->fini(context, data); goto fail; } if (hook->init_context != krb5_init_context) { - krb5_warnx(context, "loaded hook `%s' is not linked against " - "this version of Heimdal", hookpath); + krb5_warnx(context, "%s: loaded hook `%s' by vendor `%s' (API version %u)" + "is not linked against this version of Heimdal", + hookpath, hook->name, hook->vendor, hook->version); hook->fini(context, data); goto fail; } @@ -135,9 +156,17 @@ _kadm5_s_init_hooks(kadm5_server_context *ctx) ctx->hooks[ctx->num_hooks] = hook_context; hook_context = NULL; ctx->num_hooks++; - } + krb5_warnx(context, "Loaded kadm5 hook `%s' by vendor `%s' (API version %u)", + hook->name, hook->vendor, hook->version); + } return 0; +#else + krb5_warnx(context, "kadm5 hooks configured, but platform " + "does not support dynamic loading"); + ret = KADM5_BAD_SERVER_HOOK; + goto fail; +#endif /* HAVE_DLOPEN */ fail: _kadm5_s_free_hooks(ctx); @@ -153,7 +182,8 @@ fail: void _kadm5_s_free_hooks(kadm5_server_context *ctx) { - int i; +#ifdef HAVE_DLOPEN + size_t i; for (i = 0; i < ctx->num_hooks; i++) { if (ctx->hooks[i]->hook->fini != NULL) @@ -164,20 +194,5 @@ _kadm5_s_free_hooks(kadm5_server_context *ctx) free(ctx->hooks); ctx->hooks = NULL; ctx->num_hooks = 0; +#endif /* HAVE_DLOPEN */ } - -# else /* !HAVE_DLOPEN */ - -kadm5_ret_t -_kadm5_s_init_hooks(kadm5_server_context *ctx) -{ - return 0; -} - -void -_kadm5_s_free_hooks(kadm5_server_context *ctx) -{ - return 0; -} - -#endif /* !HAVE_DLOPEN */ diff --git a/lib/kadm5/setkey3_s.c b/lib/kadm5/setkey3_s.c index 91df57f2d..09814a028 100644 --- a/lib/kadm5/setkey3_s.c +++ b/lib/kadm5/setkey3_s.c @@ -56,11 +56,8 @@ setkey_principal_hook(kadm5_server_context *context, flags, n_ks_tuple, ks_tuple, n_keys, keyblocks); if (ret != 0) { - krb5_prepend_error_message(context->context, ret, - "setkey hook `%s' failed %scommit", - hook->hook->name, - stage == KADM5_HOOK_STAGE_PRECOMMIT - ? "pre" : "post"); + _kadm5_s_set_hook_error_message(context, ret, "setkey", + hook->hook, stage); if (stage == KADM5_HOOK_STAGE_PRECOMMIT) break; }