krb5 commit: Improve klog com_err hook

Greg Hudson ghudson at mit.edu
Wed Jan 17 18:15:59 EST 2018


https://github.com/krb5/krb5/commit/09cbda11a4f220db1810485123851b4f2d89dd55
commit 09cbda11a4f220db1810485123851b4f2d89dd55
Author: Greg Hudson <ghudson at mit.edu>
Date:   Thu Jan 4 11:01:28 2018 -0500

    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

 doc/plugindev/general.rst                   |   20 ++++
 src/include/adm_proto.h                     |    1 +
 src/lib/kadm5/clnt/libkadm5clnt_mit.exports |    1 +
 src/lib/kadm5/logger.c                      |  150 +++++----------------------
 src/lib/kadm5/srv/libkadm5srv_mit.exports   |    1 +
 5 files changed, 50 insertions(+), 123 deletions(-)

diff --git a/doc/plugindev/general.rst b/doc/plugindev/general.rst
index dff6807..174ccd0 100644
--- 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 e99a84d..70a3bdf 100644
--- 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 9d1a573..f122b31 100644
--- 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 ce79fab..e113af9 100644
--- 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 804eba1..64ad5dd 100644
--- 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


More information about the cvs-krb5 mailing list