* [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-27 20:07 [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility Dan Raymond
@ 2021-03-31 15:19 ` Dan Raymond
2021-03-31 19:27 ` syslog and LOG_KERN - " Adhemerval Zanella
2021-04-09 17:37 ` [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility Adhemerval Zanella
2 siblings, 0 replies; 8+ messages in thread
From: Dan Raymond @ 2021-03-31 15:19 UTC (permalink / raw)
To: libc-alpha
Can I get a review on this please?
On 3/27/2021 2:07 PM, 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
>
> ---
> 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* syslog and LOG_KERN - Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-27 20:07 [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility Dan Raymond
2021-03-31 15:19 ` Dan Raymond
@ 2021-03-31 19:27 ` Adhemerval Zanella
2021-03-31 19:44 ` [libc-coord] " Alan Coopersmith
2021-04-01 15:21 ` Rich Felker
2021-04-09 17:37 ` [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility Adhemerval Zanella
2 siblings, 2 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2021-03-31 19:27 UTC (permalink / raw)
To: Dan Raymond, libc-alpha, Rich Felker, libc-coord
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
> ---
> 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;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [libc-coord] syslog and LOG_KERN - Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-31 19:27 ` syslog and LOG_KERN - " Adhemerval Zanella
@ 2021-03-31 19:44 ` Alan Coopersmith
2021-04-01 1:21 ` ***SPAM***Re: " Dan Raymond
2021-04-01 15:21 ` Rich Felker
1 sibling, 1 reply; 8+ messages in thread
From: Alan Coopersmith @ 2021-03-31 19:44 UTC (permalink / raw)
To: libc-coord, Adhemerval Zanella, Dan Raymond, libc-alpha, Rich Felker
On 3/31/21 12:27 PM, 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.
The Solaris implementation is similar to FreeBSD & MUSL - LOG_KERN is 0,
so appears the same to syslog() as not specifying a facility and letting
the default value be used.
--
-Alan Coopersmith- alan.coopersmith@oracle.com
Oracle Solaris Engineering - https://blogs.oracle.com/alanc
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ***SPAM***Re: [libc-coord] syslog and LOG_KERN - Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-31 19:44 ` [libc-coord] " Alan Coopersmith
@ 2021-04-01 1:21 ` Dan Raymond
0 siblings, 0 replies; 8+ messages in thread
From: Dan Raymond @ 2021-04-01 1:21 UTC (permalink / raw)
To: Alan Coopersmith, libc-coord, Adhemerval Zanella, libc-alpha,
Rich Felker
On 3/31/2021 1:44 PM, Alan Coopersmith wrote:
> On 3/31/21 12:27 PM, Adhemerval Zanella wrote:
>> 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.
>
> The Solaris implementation is similar to FreeBSD & MUSL - LOG_KERN is 0,
> so appears the same to syslog() as not specifying a facility and letting
> the default value be used.
It's a fair point that even with this patch the user can't explicitly
specify LOG_KERN during a call to syslog(). To use LOG_KERN they must
call openlog() first and set it as the default facility. That's a
little clumsy but it is good enough to fix the klogd implementation in
busybox. What is the alternative? To rewrite klogd so it bypasses
syslog() altogether and writes directly to the syslogd socket? That
seems inefficient and doesn't really achieve any security. If klogd can
do this any user process can do it too.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [libc-coord] syslog and LOG_KERN - Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-31 19:27 ` syslog and LOG_KERN - " Adhemerval Zanella
2021-03-31 19:44 ` [libc-coord] " Alan Coopersmith
@ 2021-04-01 15:21 ` Rich Felker
2021-04-06 16:47 ` [PATCH] fix calls to openlog() with LOG_KERN facility (Bug 3604) Dan Raymond
1 sibling, 1 reply; 8+ messages in thread
From: Rich Felker @ 2021-04-01 15:21 UTC (permalink / raw)
To: libc-coord; +Cc: Dan Raymond, libc-alpha
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fix calls to openlog() with LOG_KERN facility (Bug 3604)
2021-04-01 15:21 ` Rich Felker
@ 2021-04-06 16:47 ` Dan Raymond
0 siblings, 0 replies; 8+ messages in thread
From: Dan Raymond @ 2021-04-06 16:47 UTC (permalink / raw)
To: Adhemerval Zanella, Alan Coopersmith, Rich Felker; +Cc: libc-alpha, libc-coord
On 4/1/2021 9:21 AM, Rich Felker wrote:
> On 3/31/2021 7:21 PM, Dan Raymond wrote:
>> On 3/31/2021 1:44 PM, Alan Coopersmith wrote:
>>> On 3/31/21 12:27 PM, Adhemerval Zanella wrote:
>>>> 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.
>>>
>>> The Solaris implementation is similar to FreeBSD & MUSL - LOG_KERN
>>> is 0,
>>> so appears the same to syslog() as not specifying a facility and
>>> letting
>>> the default value be used.
>>
>> It's a fair point that even with this patch the user can't explicitly
>> specify LOG_KERN during a call to syslog(). To use LOG_KERN they
>> must call openlog() first and set it as the default facility. That's
>> a little clumsy but it is good enough to fix the klogd implementation
>> in busybox. What is the alternative? To rewrite klogd so it
>> bypasses syslog() altogether and writes directly to the syslogd
>> socket? That seems inefficient and doesn't really achieve any
>> security. If klogd can do this any user process can do it too.
>
> 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:
>
> 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.
Adhemerval, have you had a chance to look into this?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility
2021-03-27 20:07 [PATCH] Bug 3604: fix calls to openlog() with LOG_KERN facility Dan Raymond
2021-03-31 15:19 ` Dan Raymond
2021-03-31 19:27 ` syslog and LOG_KERN - " Adhemerval Zanella
@ 2021-04-09 17:37 ` Adhemerval Zanella
2 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2021-04-09 17:37 UTC (permalink / raw)
To: libc-alpha, Dan Raymond
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
I agree with Rich remarks [1] that openlog (LOG_KERN) is not a security
boundary and making syslog using it after a previously openlog honors the
contract interface.
I also checked with my tst-syslog.c, and the patch does make KERN_LOG
usage without regressions.
And I really think that another proposed solution of redefining LOG_KERN
to a non-zero value and remapping it when actually creating the message
is cumbersome is much complex.
So the patch look good, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
[1] https://sourceware.org/pipermail/libc-alpha/2021-April/124650.html
[2] https://patchwork.sourceware.org/project/glibc/patch/20210409173031.1508079-1-adhemerval.zanella@linaro.org/
>
> ---
> 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;
^ permalink raw reply [flat|nested] 8+ messages in thread