Page MenuHomeFreeBSD

Restore local kernel "prog" filtering lost in r332099.
ClosedPublic

Authored by bdrewery on Apr 3 2020, 11:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 20 2024, 5:19 PM
Unknown Object (File)
Oct 16 2024, 2:40 PM
Unknown Object (File)
Oct 16 2024, 2:40 PM
Unknown Object (File)
Oct 16 2024, 2:40 PM
Unknown Object (File)
Oct 16 2024, 2:39 PM
Unknown Object (File)
Oct 16 2024, 2:39 PM
Unknown Object (File)
Oct 16 2024, 2:39 PM
Unknown Object (File)
Oct 16 2024, 2:09 PM
Subscribers

Details

Summary

This behavior is most relevant for ipfw(4) as documented in syslog.conf(5).
The recent addition of property-based regex filters in r359327 is a
fine workaround for this but the behavior was present since 1997 and
documented.

This only restores local matching of the "kernel program". It does not
change the forwarded format at all. On the remote side it will still
be "kernel: ipfw:" and not be parsed as a kernel message. This matches
old behavior.

MFC after: 2 weeks
Relnotes: yes

Test Plan

/etc/syslog.conf:

!-ipfw
*.* @remote.host
!ipfw
*.* /var/log/ipfw

Local shell:

# kldload ipfw
# sysctl net.inet.ip.fw.verbose=1
# ipfw resetlog
# tail -n 1 /var/log/ipfw
Apr  3 16:29:20 c1100-1 kernel: ipfw: All logging counts reset.

This log entry should not be forwarded.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fix handling of no LOG_PID on older releases. It is force enabled on head...

usr.sbin/syslogd/syslogd.c
1159 ↗(On Diff #70199)

This function should also have rfc3164 in the name, it is not generic.

1160 ↗(On Diff #70199)

Why is msg const if we are returning non-const pointers into the message?

1215 ↗(On Diff #70199)

Fix brace style while here?

1628 ↗(On Diff #70199)

Why not gather the kernel app name in printsys() and pass it to logmsg() as app_name, defaulting to "kernel" if no app name is present?

bdrewery marked 3 inline comments as done.

Review updates

usr.sbin/syslogd/syslogd.c
1160 ↗(On Diff #70199)

You're right. I fought with this API and Clang quite a bit. In this function msg is never modified so I'm stubbornly wanting it const.

In the call from logmsg msg is const. I would have to DECONST it there.
From logmsg the procid isn't cared about for kernel messages, but it still needs to be looked at to validate the app_name. In this usage msg is never modified nor is any deep pointer returned.

I'll change it to return an offset instead which satisfies what I was looking for.

1628 ↗(On Diff #70199)

I didn't want to modify the remote message that is sent nor how it is logged and wanted to match old behavior. The "kernel app" matching is very FreeBSD-specific. There's nothing in the specs about this.

For the outgoing message the app is still "kernel" or it won't display like it did before; the message is split into "app" and "msg", there's no way to display something before "app" in the current code.
local:

<109>1 2020-04-06T16:15:46.522515-07:00 c1100-1.shatow.net kernel - - - ipfw: All logging counts reset.

remote:

<109>1 2020-04-06T16:20:18.755703-07:00 home-??? kernel - - - ipfw: All logging counts reset.

local:

Apr  6 16:16:05 c1100-1 kernel: ipfw: All logging counts reset.

Remote:

Apr  6 16:18:49 <security.notice> home-??? kernel: ipfw: All logging counts reset.

If I modified app_name it would become this:

Apr  6 16:18:49 <security.notice> home-??? ipfw: All logging counts reset.

The app name can't have a space from what I can tell but this would still be wrong as the remote side would go from app_name=kernel (pre-rfc5424) to app_name="kernel: ipfw"
Wrong:

<109>1 2020-04-06T16:20:18.755703-07:00 home-??? kernel: ipfw - - - All logging counts reset.

If you're still reading here's the old version of this feature:

/* add kernel prefix for kernel messages */
if (flags & ISKERNEL) {
        snprintf(buf, sizeof(buf), "%s: %s",
            use_bootfile ? bootfile : "kernel", msg);
        msg = buf;
        msglen = strlen(buf);
}

It simply prefixed "kernel:" into the message after the app_name was determined to be "ipfw". It only used prog for the filter check in logmsg

# grep -w prog ../10/usr.sbin/syslogd/syslogd.c
        char prog[NAME_MAX+1];
                prog[i] = msg[i];
        prog[i] = 0;
                if (skip_message(prog, f->f_program, 1))

The message is forwarded was the full msg there without any further distinction between what to tell the remote "app name" was.
The new code treats msg stricly as the message to send and not as containing the app name anymore.

TLDR old (both pre-my-change and pre-5424) and new versions of the code retain same "app name" on remote side.

usr.sbin/syslogd/syslogd.c
1683 ↗(On Diff #70287)

We are assuming here that kernel_app_name is initialized.

1628 ↗(On Diff #70199)

I see, I didn't look at other usages of app_name in logmsg().

bdrewery marked an inline comment as done.

Fix uninitialized kernel_app_name

This revision is now accepted and ready to land.Apr 8 2020, 8:13 PM