Improve klog com_err hook

Remove the code to read a severity from the first byte of format, as
it is an unclear interface and likely unused.  Also stop using the
configured default severity for syslog devices.  Instead, log at error
severity if a code is given, and at informational severity if one is
not.

Pass the formatted message to krb5_klog_syslog() so that it uses the
same format in log files as regular logged messages.

Add krb5_klog_set_context() to allow the context for extended error
messages to be reset, so that KDC plugins can log using the context
object for the realm being served for each request.

Use k5buf for simpler memory management in the hook function.

ticket: 8630

diff --git a/doc/plugindev/general.rst b/doc/plugindev/general.rst
index dff68076233a8ee066e14d0523c46f8c667899f8..174ccd0676ed4a467badc5e2d424bf7cdb362b31 100644 (file)
--- a/doc/plugindev/general.rst
+++ b/doc/plugindev/general.rst
@@ -94,5 +94,25 @@ fictional pluggable interface named fences, for a module named
         return 0;
     }
 
+Logging from KDC and kadmind plugin modules
+-------------------------------------------
+
+Plugin modules for the KDC or kadmind daemons can write to the
+configured logging outputs (see :ref:`logging`) by calling the
+**com_err** function.  The first argument (*whoami*) is ignored.  If
+the second argument (*code*) is zero, the formatted message is logged
+at informational severity; otherwise, the formatted message is logged
+at error severity and includes the error message for the supplied
+code.  Here are examples::
+
+    com_err("", 0, "Client message contains %d items", nitems);
+    com_err("", retval, "while decoding client message");
+
+(The behavior described above is new in release 1.17.  In prior
+releases, the *whoami* argument is included for some logging output
+types, the logged message does not include the usual header for some
+output types, and the severity for syslog outputs is configured as
+part of the logging specification, defaulting to error severity.)
+
 .. _automake: http://www.gnu.org/software/automake/
 .. _libtool: http://www.gnu.org/software/libtool/

diff --git a/src/include/adm_proto.h b/src/include/adm_proto.h
index e99a84d6f86be90c28b84b70e54741154bb3df0a..70a3bdf210628015a79baa294e0306b87f5beb8a 100644 (file)
--- a/src/include/adm_proto.h
+++ b/src/include/adm_proto.h
@@ -48,6 +48,7 @@ typedef struct ___krb5_key_salt_tuple krb5_key_salt_tuple;
 
 /* logger.c */
 krb5_error_code krb5_klog_init(krb5_context, char *, char *, krb5_boolean);
+void krb5_klog_set_context(krb5_context);
 void krb5_klog_close(krb5_context);
 int krb5_klog_syslog(int, const char *, ...)
 #if !defined(__cplusplus) && (__GNUC__ > 2)

diff --git a/src/lib/kadm5/clnt/libkadm5clnt_mit.exports b/src/lib/kadm5/clnt/libkadm5clnt_mit.exports
index 9d1a5734128431501fd6ef7525f377ad1ece40f5..f122b31ab96adf6fd9bf4caaeb9c2b0d04588660 100644 (file)
--- a/src/lib/kadm5/clnt/libkadm5clnt_mit.exports
+++ b/src/lib/kadm5/clnt/libkadm5clnt_mit.exports
@@ -62,6 +62,7 @@ krb5_keysalt_iterate
 krb5_klog_close
 krb5_klog_init
 krb5_klog_reopen
+krb5_klog_set_context
 krb5_klog_syslog
 krb5_string_to_keysalts
 xdr_chpass3_arg

diff --git a/src/lib/kadm5/logger.c b/src/lib/kadm5/logger.c
index ce79fabf753dc177b70e2bf293f3a173a9933f70..e113af9d20bd200fd31c6d52b2a6d3054a4e991c 100644 (file)
--- a/src/lib/kadm5/logger.c
+++ b/src/lib/kadm5/logger.c
@@ -173,142 +173,39 @@ klog_com_err_proc(const char *whoami, long int code, const char *format, va_list
 #endif
     ;
 
+/*
+ * Write com_err() messages to the configured logging devices.  Ignore whoami,
+ * as krb5_klog_init() already received a whoami value.  If code is nonzero,
+ * log its error message (retrieved using err_context) and the formatted
+ * message at error severity.  If code is zero, log the formatted message at
+ * informational severity.
+ */
 static void
 klog_com_err_proc(const char *whoami, long int code, const char *format, va_list ap)
 {
-    char        outbuf[KRB5_KLOG_MAX_ERRMSG_SIZE];
-    int         lindex;
-    const char  *actual_format;
-    int         log_pri = -1;
-    char        *cp;
-    char        *syslogp;
+    struct k5buf buf;
+    const char *emsg;
 
-    if (whoami == NULL || format == NULL)
+    if (format == NULL)
         return;
 
-    /* Make the header */
-    snprintf(outbuf, sizeof(outbuf), "%s: ", whoami);
-    /*
-     * Squirrel away address after header for syslog since syslog makes
-     * a header
-     */
-    syslogp = &outbuf[strlen(outbuf)];
+    k5_buf_init_dynamic(&buf);
 
-    /* If reporting an error message, separate it. */
     if (code) {
-        const char *emsg;
-        outbuf[sizeof(outbuf) - 1] = '\0';
-
-        emsg = krb5_get_error_message (err_context, code);
-        strncat(outbuf, emsg, sizeof(outbuf) - 1 - strlen(outbuf));
-        strncat(outbuf, " - ", sizeof(outbuf) - 1 - strlen(outbuf));
+        /* Start with the error message and a separator. */
+        emsg = krb5_get_error_message(err_context, code);
+        k5_buf_add(&buf, emsg);
         krb5_free_error_message(err_context, emsg);
+        k5_buf_add(&buf, " - ");
     }
-    cp = &outbuf[strlen(outbuf)];
 
-    actual_format = format;
-    /*
-     * This is an unpleasant hack.  If the first character is less than
-     * 8, then we assume that it is a priority.
-     *
-     * Since it is not guaranteed that there is a direct mapping between
-     * syslog priorities (e.g. Ultrix and old BSD), we resort to this
-     * intermediate representation.
-     */
-    if ((((unsigned char) *format) > 0) && (((unsigned char) *format) <= 8)) {
-        actual_format = (format + 1);
-        switch ((unsigned char) *format) {
-        case 1:
-            log_pri = LOG_EMERG;
-            break;
-        case 2:
-            log_pri = LOG_ALERT;
-            break;
-        case 3:
-            log_pri = LOG_CRIT;
-            break;
-        default:
-        case 4:
-            log_pri = LOG_ERR;
-            break;
-        case 5:
-            log_pri = LOG_WARNING;
-            break;
-        case 6:
-            log_pri = LOG_NOTICE;
-            break;
-        case 7:
-            log_pri = LOG_INFO;
-            break;
-        case 8:
-            log_pri = LOG_DEBUG;
-            break;
-        }
-    }
+    /* Add the formatted message. */
+    k5_buf_add_vfmt(&buf, format, ap);
 
-    /* Now format the actual message */
-    vsnprintf(cp, sizeof(outbuf) - (cp - outbuf), actual_format, ap);
+    if (k5_buf_status(&buf) == 0)
+        krb5_klog_syslog(code ? LOG_ERR : LOG_INFO, "%s", buf.data);
 
-    /*
-     * Now that we have the message formatted, perform the output to each
-     * logging specification.
-     */
-    for (lindex = 0; lindex < log_control.log_nentries; lindex++) {
-        /* Omit messages marked as LOG_DEBUG for non-syslog outputs unless we
-         * are configured to include them. */
-        if (log_pri == LOG_DEBUG && !log_control.log_debug &&
-            log_control.log_entries[lindex].log_type != K_LOG_SYSLOG)
-            continue;
-
-        switch (log_control.log_entries[lindex].log_type) {
-        case K_LOG_FILE:
-        case K_LOG_STDERR:
-            /*
-             * Files/standard error.
-             */
-            if (fprintf(log_control.log_entries[lindex].lfu_filep, "%s\n",
-                        outbuf) < 0) {
-                /* Attempt to report error */
-                fprintf(stderr, log_file_err, whoami,
-                        log_control.log_entries[lindex].lfu_fname);
-            }
-            else {
-                fflush(log_control.log_entries[lindex].lfu_filep);
-            }
-            break;
-        case K_LOG_CONSOLE:
-        case K_LOG_DEVICE:
-            /*
-             * Devices (may need special handling)
-             */
-            if (DEVICE_PRINT(log_control.log_entries[lindex].ldu_filep,
-                             outbuf) < 0) {
-                /* Attempt to report error */
-                fprintf(stderr, log_device_err, whoami,
-                        log_control.log_entries[lindex].ldu_devname);
-            }
-            break;
-        case K_LOG_SYSLOG:
-            /*
-             * System log.
-             */
-            /*
-             * If we have specified a priority through our hackery, then
-             * use it, otherwise use the default.
-             */
-            if (log_pri >= 0)
-                log_pri |= log_control.log_entries[lindex].lsu_facility;
-            else
-                log_pri = log_control.log_entries[lindex].lsu_facility |
-                    log_control.log_entries[lindex].lsu_severity;
-
-            /* Log the message with our header trimmed off */
-            syslog(log_pri, "%s", syslogp);
-            break;
-        default:
-            break;
-        }
-    }
+    k5_buf_free(&buf);
 }
 
 /*
@@ -662,6 +559,13 @@ krb5_klog_init(krb5_context kcontext, char *ename, char *whoami, krb5_boolean do
     return((log_control.log_nentries) ? 0 : ENOENT);
 }
 
+/* Reset the context used by the com_err hook to retrieve error messages. */
+void
+krb5_klog_set_context(krb5_context kcontext)
+{
+    err_context = kcontext;
+}
+
 /*
  * krb5_klog_close()    - Close the logging context and free all data.
  */

diff --git a/src/lib/kadm5/srv/libkadm5srv_mit.exports b/src/lib/kadm5/srv/libkadm5srv_mit.exports
index 804eba16abb859061620db6cd30f0ffafcb4c4f2..64ad5dd69ec7f6902f5589dd7403c848f2059638 100644 (file)
--- a/src/lib/kadm5/srv/libkadm5srv_mit.exports
+++ b/src/lib/kadm5/srv/libkadm5srv_mit.exports
@@ -71,6 +71,7 @@ krb5_keysalt_iterate
 krb5_klog_close
 krb5_klog_init
 krb5_klog_reopen
+krb5_klog_set_context
 krb5_klog_syslog
 krb5_string_to_keysalts
 master_db