[libc-coord] syslog and LOG_KERN - Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility

[Rich Felker]

On Wed, Mar 31, 2021 at 04:27:54PM -0300, Adhemerval Zanella wrote:
> 
> 
> On 27/03/2021 17:07, Dan Raymond wrote:
> > From 93683928886a563a4740e2b42b53752a4a7d431f Mon Sep 17 00:00:00 2001
> > From: Dan Raymond <draymond@foxvalley.net>
> > Date: Sat, 27 Mar 2021 13:51:16 -0600
> > Subject: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
> > 
> 
> Not allowing LOG_KERN by any user process seems to be de facto behavior
> on all systems I am aware of:
> 
>   * FreeBSD and MUSL explicit set to previous log facility (they check
>     if the priority against a mask and since on both LOG_KERN is 0 is
>     set to the previous/default value).
> 
>   * Solaris 11.4 man page explicit says:
> 
>        LOG_KERN      Messages generated by the kernel. These cannot be  gener-
>                      ated by any user processes.
> 
>   * AIX 7.2 is similar, but it seems to provide a special symbol for that
>     (which seems to not have a man page):
> 
>        LOG_KERN      Logs messages generated by the kernel. Kernel processes 
>                      should use the bsdlog routine to generate syslog messages. 
>                      The syntax of bsdlog is identical to syslog. The bsdlog 
>                      messages can only be created by kernel processes and must
>                      be of LOG_KERN priority. The syslog subroutine cannot log 
>                      LOG_KERN facility messages. Instead it will log LOG_USER 
>                      facility messages.
> 
> So before to make glibc an outlier here to fix a very specific issue, I
> would like to check with other implementation the possible security
> implication and whether it make sense to change it.
> 
> Reference: https://sourceware.org/bugzilla/show_bug.cgi?id=3604

It's not a matter of security (this is not a security boundary) but of
interface contract:

    "Values of the priority argument are formed by OR'ing together a
    severity-level value and an optional facility value. If no
    facility value is specified, the current default facility value is
    used."

However:

> 
> > ---
> >  misc/syslog.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/misc/syslog.c b/misc/syslog.c
> > index 2cc63ef287..bb30cd963a 100644
> > --- a/misc/syslog.c
> > +++ b/misc/syslog.c
> > @@ -285,7 +285,7 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
> > 
> >      /* Get connected, output the message to the local logger. */
> >      if (!connected)
> > -        openlog_internal(LogTag, LogStat | LOG_NDELAY, 0);
> > +        openlog_internal(NULL, LogStat | LOG_NDELAY, LogFacility);
> > 
> >      /* If we have a SOCK_STREAM connection, also send ASCII NUL as
> >         a record terminator.  */
> > @@ -299,7 +299,7 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
> >          /* Try to reopen the syslog connection.  Maybe it went
> >             down.  */
> >          closelog_internal ();
> > -        openlog_internal(LogTag, LogStat | LOG_NDELAY, 0);
> > +        openlog_internal(NULL, LogStat | LOG_NDELAY, LogFacility);
> >            }
> > 
> >          if (!connected || __send(LogFile, buf, bufsize, send_flags) < 0)
> > @@ -343,7 +343,7 @@ openlog_internal(const char *ident, int logstat, int logfac)
> >      if (ident != NULL)
> >          LogTag = ident;
> >      LogStat = logstat;
> > -    if (logfac != 0 && (logfac &~ LOG_FACMASK) == 0)
> > +    if ((logfac &~ LOG_FACMASK) == 0)
> >          LogFacility = logfac;
> > 
> >      int retry = 0;

It looks like the proposed patch is not making syslog violate its
interface contract, only fixing glibc's existing violation of the
openlog interface contract where it's ignoring the facility argument
when the value is zero. If my understanding is correct, that would
just bring glibc in line with what musl already does, not make it an
outlier.

Admittedly having to use openlog to set LOG_KERN is hideous because it
involves global state and might break other code in the same process,
but at least it works and it's probably acceptable for the rare
application that has legitimate reason to send LOG_KERN messages
(klogd/syslogd only?).

Another possible solution on the libc side would be redefining
LOG_KERN to something nonzero and remapping it to the value 0 on the
wire. But I suspect this would break syslogd implementations.

Rich