URL:
https://github.com/SSSD/sssd/pull/5477
Title: #5477: DEBUG: Use () in program identifier instead of []
sigv commented:
"""
Initially #5431 was talking about the case:
- SSSD is built with --with-syslog=syslog
- syslog was trying to parse [] part of debug_prg_name as a PID
- the result was seen in journalctl output as malformed SYSLOG_IDENTIFIER and SYSLOG_PID
(and this was the issue)
This specific case should be resolved with 1823353 as this patch removes
ident==debug_prg_name in case "--with-syslog=syslog" and default is used
instead. So instead of sssd[sudo] you should see sssd_sudo (no [])
All correct on the summary. The reason for the improper `SYSLOG_IDENTIFIER` and
`SYSLOG_PID` was that systemd-journald was parsing syslog back into journald. It was
picking up misformated lines and mis-parsing the `TAG[PID]` combination there.
The mitigation is partially a combination of 00e7b1ada3d1c1071eac79b65c17cd2701c2ae6a and
18233532b72e62452eac6886652fa633ba055d8c with one removing `sssd[` from one place and
adding `sssd[` in another. I did close that opened #5431 issue ticket on the tracker here
as a result, as it was determined to be a flaw in historical versions.
then, I guess, syslog gets wrong part of the TAG as a PID?
What is practical issue / consequences?
This is where me being interested in switching `--with-syslog=syslog` to
`--with-syslog=journald` comes into play. As of now, by default in Ubuntu (and I would
guess Debian as well), `/var/log/syslog` gets forwarded to from journald, with rsyslog. As
you can see from various sources, rsyslog utilizes BSD syslog format as in RFC 3164
instead of the RFC 5424 format.
However for Ubuntu Core Development Team brought up the fact that on Ubuntu
`/var/log/syslog` output as `sssd[sudo][1234]:` after the current proposed changes. If
someone is working with the syslog output then they will have a bad time when trying to
parse PID, which with a greedy (default) regexp for `/\[.*\]/` or
`.split('[')[0].split(']')[0]` results with `"[sudo][1234]"`.
So strictly speaking replacement of [] with () doesn't look as a
proper solution.
The informational RFC 3164 from year 2001 was not a perfectly formal standard and there
are adaptations to it. Parsers generally care about the `:` and `[` characters as
separators in the start of "content". The goal here is to avoid `sudo` bit
leaking into `PID`.
I don't see how "[]" in APP-NAME violates rfc5424
(please correct me if I'm wrong)
In RFC 5424 both `sssd[sudo]` and `sssd(sudo)` could be considered valid indeed. However,
RFC 3164 is still in use in the wild. And usual parsers there handle `[]` in a particular
way, as PID separators.
Frankly, I'm tempted to just use sd_journal_send() with
"SYSLOG_IDENTIFIER=sssd[%s]", program_invocation_short_name.
`sssd[sssd_sudo]` still has a `[` character. That is the concern. If you want, then it
could be replaced with `SYSLOG_IDENTIFIER=sssd(%s)` and `program_invocation_short_name`
instead of `debug_prg_name`.
But at that point, as you have `sssd(sssd_sudo)` you might as well just use
`program_invocation_short_name` (`sssd_sudo`) directly.
Not sure if this proposal is improving the sitation as a result.
This way we will not change format of logs in files and will use a
process name as an SYSLOG_IDENTIFIER, and I think any reasonable syslog implementation
should be able to cope with it.
There is a change in any way. The change proposed by this PR just swaps `[` characters
with `(` characters. If someone was parsing them previously, they can keep the same logic
they did by swapping the two.
"any reasonable syslog implementation" does not deal with the current code due
to the (old) RFC 3164 expecting `[` to separate PIDs, and causes pain. Does `be` report
the domain in its program name even? The change to process name causes more change than
just swapping `[` to `(`.
I don't know why is this difference.
The difference is in how rsyslog picks up the journald message and ships it to BSD-style
syslog (as the old RFC 3164).
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5477#issuecomment-772763558