From: Greg Hudson Date: Thu, 4 Jan 2018 16:01:28 +0000 (-0500) Subject: Improve klog com_err hook X-Git-Tag: krb5-1.17-beta1~196 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=09cbda11a4f220db1810485123851b4f2d89dd55;p=thirdparty%2Fkrb5.git 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 dff6807623..174ccd0676 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 e99a84d6f8..70a3bdf210 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 9d1a573412..f122b31ab9 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 ce79fabf75..e113af9d20 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 804eba16ab..64ad5dd69e 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