Allow for journald logs filtering on a per-unit basis #24058

qdeslandes · 2022-07-18T21:35:07Z

Implement mechanism to filter logs ingested by journald using regexes (#6432).

Implement for two new keywords in unit files (in Service section):

LogIncludeRegex=
LogExcludeRegex=

When journald will process logs on a unit with LogIncludeRegex defined, it will continue processing if the log message matches the regex, and discard it if it doesn't. The opposite behaviour is used for LogExcludeRegex. If none is defined, then all log messages are processed as usual. If both are defined, then the log message must match LogIncludeRegex but not LogExcludeRegex to be processed.

flowchart TD
    A{{Is LogIncludeRegex defined?}}
    B{{Does LogIncludeRegex matches?}}
    C{{Is LogExcludeRegex defined?}}
    D{{Does LogExcludeRegex matches?}}
    F>Discard log]
    E>Ingest log]

    style A fill:#f5f3f4,stroke:#219ebc,stroke-width:3px
    style B fill:#f5f3f4,stroke:#219ebc,stroke-width:3px
    style C fill:#f5f3f4,stroke:#219ebc,stroke-width:3px
    style D fill:#f5f3f4,stroke:#219ebc,stroke-width:3px
    style E fill:#f5f3f4,stroke:#219ebc,stroke-width:3px
    style F fill:#f5f3f4,stroke:#219ebc,stroke-width:3px


    A -- YES --> B -- YES --> C -- YES --> D
    A -- NO --> C
    B -- NO --> F
    C -- NO --> E
    D -- NO --> E
    D -- YES --> F

Both regexes are read from the unit file (or from D-Bus or systemctl set-property) and stored within the unit's cgroup xattr for journald to read.

Changes in this PR have been tested using mkosi qemu. Sending logs to journald using logger, systemd-cat and StandardOutput=journal. This change is feature complete, I will add a last commit to update integration tests.

Based on #25394

DaanDeMeyer · 2022-07-19T08:49:26Z

Seems there's a few CI failures (I think when PCRE2 is missing?)

../../src/systemd/src/journal/journald-context.c:520:17: error: unknown type name 'pcre2_code'
                pcre2_code **compiled_regex) {
                ^
../../src/systemd/src/journal/journald-context.c:568:29: error: no member named 'log_include_regex' in 'struct ClientContext'
                        &c->log_include_regex,
                         ~  ^
../../src/systemd/src/journal/journald-context.c:569:29: error: no member named 'log_include_compiled' in 'struct ClientContext'
                        &c->log_include_compiled);
                         ~  ^
../../src/systemd/src/journal/journald-context.c:576:29: error: no member named 'log_exclude_regex' in 'struct ClientContext'
                        &c->log_exclude_regex,
                         ~  ^
../../src/systemd/src/journal/journald-context.c:577:29: error: no member named 'log_exclude_compiled' in 'struct ClientContext'
                        &c->log_exclude_compiled);
                         ~  ^

DaanDeMeyer

Did an initial pass, approach looks good but some comments.

man/systemd.exec.xml

src/core/cgroup.c

src/journal/journald-context.c

src/journal/journald-context.h

src/journal/journald-context.c

src/journal/journald-context.h

bluca · 2022-07-20T17:41:01Z

Please add tests for this new feature

src/basic/alloc-util.h

src/journal/journald-context.c

poettering

somehow i forgot to post these comments when i wrote them the other day, i hope they still apply to the current version of the PR

docs/TRANSIENT-SETTINGS.md

man/systemd.exec.xml

src/core/cgroup.c

docs/TRANSIENT-SETTINGS.md

src/shared/pcre2-util.c

src/journal/journald-context.c

man/systemd.exec.xml

src/core/load-fragment.c

src/journal/journald-context.c

src/shared/pcre2-util.c

src/test/test-load-fragment.c

DaanDeMeyer

Looks great, a few comments

docs/TRANSIENT-SETTINGS.md

src/core/load-fragment.c

src/test/test-load-fragment.c

src/journal/journald-client.c

src/journal/journald-context.c

src/journal/journald-stream.c

src/journal/journald-syslog.c

src/journal/journald-native.c

src/journal/journald-client.c

.vscode/settings.json

src/journal/journald-syslog.c

DaanDeMeyer

Some more comments

man/systemd.exec.xml

src/core/execute.c

src/core/load-fragment.c

src/journal/journald-context.c

src/core/load-fragment.c

src/core/cgroup.c

lgtm-com · 2022-07-26T14:27:52Z

This pull request introduces 1 alert when merging 1bf9ace into 5f4ccb0 - view on LGTM.com

new alerts:

1 for Comparison result is always the same

evverx · 2022-07-26T15:16:40Z

As far as I can tell this feature isn't compatible with log namespaces at this point.

systemd-run --property 'LogFilterRegex=GREP ME'  bash -c 'echo GREP ME; echo TEST'
Running as unit: run-r323cc3e560cf41148bbcdc2fa9d23631.service

journalctl -u run-r323cc3e560cf41148bbcdc2fa9d23631.service
Jul 26 15:11:15 C systemd[1]: Started run-r323cc3e560cf41148bbcdc2fa9d23631.service.
Jul 26 15:11:15 C bash[201]: GREP ME
Jul 26 15:11:15 C systemd[1]: run-r323cc3e560cf41148bbcdc2fa9d23631.service: Deactivated successfully.

systemd-run --property 'LogFilterRegex=GREP ME' --property LogNamespace=wat bash -c 'echo GREP ME; echo TEST'
Running as unit: run-r135e7b4d02e74f41bc745ab4c81159c5.service

journalctl -u run-r135e7b4d02e74f41bc745ab4c81159c5.service --namespace wat
Jul 26 15:12:25 C bash[205]: GREP ME
Jul 26 15:12:25 C bash[205]: TEST

I think it should be documented as well (if it's intentional) and it would be great if it was possible to cover that with tests.

evverx · 2022-07-26T15:39:03Z

Also my fuzz targets didn't get far

Jul 26 15:33:31 C systemd-journald[1160]: File /var/log/journal/89048b4b503f455f861e1cda6e724d83.wat/system.journal corrupted or uncleanly shut down, renaming and replacing.
Jul 26 15:33:31 C systemd-journald[1160]: ../src/journal/journald-native.c:263:13: runtime error: null pointer passed as argument 1, which is declared to never be null
Jul 26 15:33:31 C systemd-journald[1160]:     #0 0x48c225 in server_process_entry ../src/journal/journald-native.c:263
Jul 26 15:33:31 C systemd-journald[1160]:     #1 0x48d258 in server_process_native_message ../src/journal/journald-native.c:321
Jul 26 15:33:31 C systemd-journald[1160]:     #2 0x438213 in server_process_datagram ../src/journal/journald-server.c:1393
Jul 26 15:33:31 C systemd-journald[1160]:     #3 0x7f273d1c2324 in source_dispatch ../src/libsystemd/sd-event/sd-event.c:3602
Jul 26 15:33:31 C systemd-journald[1160]:     #4 0x7f273d1cabbb in sd_event_dispatch ../src/libsystemd/sd-event/sd-event.c:4186
Jul 26 15:33:31 C systemd-journald[1160]:     #5 0x7f273d1cbd1f in sd_event_run ../src/libsystemd/sd-event/sd-event.c:4247
Jul 26 15:33:31 C systemd-journald[1160]:     #6 0x40925e in main ../src/journal/journald.c:114
Jul 26 15:33:31 C systemd-journald[1160]:     #7 0x7f273b24043f in __libc_start_call_main (/lib64/libc.so.6+0x4043f)
Jul 26 15:33:31 C systemd-journald[1160]:     #8 0x7f273b2404ef in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x404ef)
Jul 26 15:33:31 C systemd-journald[1160]:     #9 0x408684 in _start (/usr/lib/systemd/systemd-journald+0x408684)
Jul 26 15:33:31 C systemd-journald[1160]: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/journal/journald-native.c:263:13 in

evverx · 2022-07-26T16:56:00Z

Looking at #6432 (comment) it appears this PR doesn't cover use cases where as far as I understand the idea is not to keep sensitive data in journald but to be able to forward them via syslog (or whatever) elsewhere. I don't know if use cases like that are supposed to be covered though. It would be great if that could be documented as well (because right now the documentation doesn't mention that LogFilterRegex affects forwarding in any way).

evverx · 2022-07-26T23:15:20Z

Having finished reading that thread I wonder why this setting can't be used to filter out messages coming from systemd itself? It came up quite a few times there as far as I can see and it's kind of weird that LogFilterRegex works like that. Anyway I think it should be documented as well.

DaanDeMeyer

Some nits but LGTM

src/journal/journald-client.c

src/core/cgroup.c

yuwata

Please add assertions about each function argument.

src/basic/nulstr-util.c

src/basic/nulstr-util.h

src/core/cgroup.c

src/journal/journald-client.c

src/core/cgroup.c

src/core/load-fragment.c

src/core/dbus-execute.c

src/journal/journald-client.c

Add function set_make_nulstr() to create a nulstr out of a set. Behave the same way as strv_make_nulstr().

Define new unit parameter (LogFilterPatterns) to filter logs processed by journald. This option is used to store a regular expression which is carried from PID1 to systemd-journald through a cgroup xattrs: `user.journald_log_filter_patterns`.

Parse DBus structure send by LogFilterPatterns to print it in systemctl show.

Use LogFilterPatterns from the unit's cgroup xattr in order to keep or discard log messages before writing them to the journal. When a log message is discarded, it won't be written to syslog, console... either. When a native, syslog, or standard output log message is received, systemd-journald will process it if it matches against at least one allowed pattern (if any) and none of the denied patterns (if any).

Add integration tests for journald's log filtering feature.

yuwata

LGTM.

DaanDeMeyer · 2022-12-15T12:58:47Z

TEST-04-JOURNAL passed on the CentOS CIs, only the repart integration test failed because of a different issue unrelated to this PR

poettering

sorry for the late review, but somehow i never actually sent these review points. they still apply on the merged version, any chance you can still adjust the implmenetation in a follow-up commit?

src/test/test-nulstr-util.c

src/core/cgroup.c

src/core/load-fragment.c

qdeslandes · 2023-01-06T08:28:07Z

sorry for the late review, but somehow i never actually sent these review points. they still apply on the merged version, any chance you can still adjust the implmenetation in a follow-up commit?

Done! #25954

Fix followup comments on PR #24058: - Use `mempcpy_safe()`. - Remove unused `pcre2_code` variable. - Use `static const` when relevant.

Fix followup comments on PR systemd#24058: - Use `mempcpy_safe()`. - Remove unused `pcre2_code` variable. - Use `static const` when relevant.

github-actions bot added documentation journal labels Jul 18, 2022

DaanDeMeyer requested changes Jul 20, 2022

View reviewed changes

DaanDeMeyer added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jul 20, 2022

DaanDeMeyer reviewed Jul 21, 2022

View reviewed changes

src/basic/alloc-util.h Outdated Show resolved Hide resolved

DaanDeMeyer reviewed Jul 21, 2022

View reviewed changes

qdeslandes requested a review from DaanDeMeyer July 22, 2022 15:44

poettering requested changes Jul 22, 2022

View reviewed changes