Page MenuHomeFreeBSD

add altlog_jaillist to syslogd's rc script
Needs ReviewPublic

Authored by me_igalic.co on Nov 29 2020, 12:00 PM.

Details

Reviewers
None
Group Reviewers
manpages
rc
Summary

syslogd has an altlog_proglist stanza, which can place the log socket
into a program's chroot directory, so it can still log.

This patch adds an equivalent to that for jails.
If used with jails, we lookup the each jail's "path" with jls appending
it to the sockfile and, now also privsockfile.

Caveats:

  • syslogd must be restarted
  • -H must be added to syslogd_flags before this is actually useful
Test Plan

install script in /etc/rc.d/syslogd
restart syslog, it still works, phew

create a jail with syslogd_enable="NO" and add altlog_jaillist="jail-name" to the host's rc.conf
verify that the jail logs to the host.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

I think this generally LGTM; you'll need to also add a default and description for altlog_jaillist in ^/libexec/rc.conf (preferably near the other altlog_*)

libexec/rc/rc.d/syslogd
55

This line seems to be indented with spaces, and subsequent lines with a mix of tabs -- these should be entirely tabs

address @kevans's review; and document! fix almost all igor & mandoc concerns, except for this one:

mandoc: usr.sbin/syslogd/syslogd.8:281:14: STYLE: no blank before trailing delimiter: Li kernel:
yuripv added inline comments.
usr.sbin/syslogd/syslogd.8
415

Should not these be documented in rc.conf(5) instead? As it is, rc variables are not related to syslogd daemon.

great idea!

libexec/rc/rc.d/syslogd
53

s/priviledged/privileged/

address @dch and @yuripv review, moving rc.conf settings to rc.conf(5) and fixing most style issues

i've now finally gotten around to testing this change ^_^;; and i'm quite happy with the results…

…except when the jail is -red, it doesn't show up in jls -a output… it did with my libioc jails…
(aaaand, it's too late to figure out why)

So, another test round.
This works fine with syslogd_flags="-s" and syslogd_flags="-ss"; however, only when we add -H to the flags, do the syslog entries become more useful, because we get the host.hostname of the jail in our logs.
so I'll add that to that to the rc.conf man page.

add explanation that "-H" is needed for altlog_jaillist

share/man/man5/rc.conf.5
1380

i think you want Ns before . (might need to escape it as \&. if mandoc lint whines).

1506

i think you want Ns before . (might need to escape it as \&. if mandoc lint whines).

3915

same Ns note, so you don't break the path in parts.

4611–4612

SEE ALSO is sorted by section first, move it below.

usr.sbin/syslogd/syslogd.8
438

Same sort note.

me_igalic.co edited the summary of this revision. (Show Details)

address @yuripv's review: by removing additional lints from rc.conf.5

jls -a returns an error on system startup; should we silence it with 2>/dev/null?

libexec/rc/rc.d/syslogd
55

jls -aj foo path doesn't return anything useful if the jail isn't running, so we should probably silence the error here.

me_igalic.co marked an inline comment as done.
0mp added inline comments.
libexec/rc/rc.conf
289

Missing period.

289–290

Missing period.

libexec/rc/rc.d/syslogd
53

Comments seems to be indented in a slightly different way in this file. Could you new comments in a similar fashion to maintain a uniform style?

share/man/man5/rc.conf.5
2157

I think this line is unnecessary in this patch. We may think about adding section separators like this in a separate commit for all the other sections we have in this file.

address @0mp's comments.