Page MenuHomeFreeBSD

add altlog_jaillist to syslogd's rc script
AbandonedPublic

Authored by freebsd_igalic.co on Nov 29 2020, 12:00 PM.
Referenced Files
Unknown Object (File)
Sun, Mar 10, 8:50 AM
Unknown Object (File)
Feb 17 2024, 12:07 PM
Unknown Object (File)
Feb 7 2024, 8:28 PM
Unknown Object (File)
Feb 1 2024, 11:29 PM
Unknown Object (File)
Jan 31 2024, 9:50 PM
Unknown Object (File)
Jan 19 2024, 9:36 PM
Unknown Object (File)
Dec 20 2023, 7:08 AM
Unknown Object (File)
Dec 1 2023, 11:12 PM

Details

Reviewers
kevans
yuripv
0mp
pauamma_gundo.com
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
rS FreeBSD src repository - subversion
Lint
Lint Skipped
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
56

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
416

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
54

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 ↗(On Diff #80308)

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

1506 ↗(On Diff #80308)

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

3917 ↗(On Diff #80308)

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

4614 ↗(On Diff #80308)

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

usr.sbin/syslogd/syslogd.8
476

Same sort note.

freebsd_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
56

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

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

Missing period.

290

Missing period.

libexec/rc/rc.d/syslogd
54

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
2158 ↗(On Diff #80374)

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.

koobs added a subscriber: koobs.

Add previous reviewers to ... Reviewers: as this ... needs review :)

pauamma_gundo.com added inline comments.
libexec/rc/rc.conf
290
share/man/man5/rc.conf.5
2189–2192 ↗(On Diff #81917)

If you don't go for this, you still need to s/recommend/recommended/.

And maybe "add ..." instead of "adding .... is recommended", but that's arguably taste.

2193 ↗(On Diff #81917)
This revision now requires changes to proceed.May 26 2022, 2:36 AM
This revision is now accepted and ready to land.May 27 2022, 1:20 AM

it's been a while. update this to bug people

This revision now requires review to proceed.Feb 23 2023, 11:17 PM

I'm afraid this change is a potential security problem.

Ultimately content of the jail is considered fundamentally untrusted, meaning /var/run in the jail can point literally anywhere and in particular can be a symlink outside of the jail. syslog must *not* follow it.

But let's say none of the above was a problem. As this allows a jailed program to talk to host's syslog, it introduces another potential security problem -- what if syslog is exploitable? Furthermore, are there any provisions made to prevent jailed process from faking messages from the main host?

This is not a simple problem and the current change is definitely a no-go.

I also feel inclined to add that any remaining code which mingles with a running jail should be fixed.

Contrary to popular belief the jail subsystem was not thought out very well, I refer you to sample bugs in the area:
https://www.freebsd.org/security/advisories/FreeBSD-SA-21:10.jail_mount.asc
https://www.freebsd.org/security/advisories/FreeBSD-SA-21:05.jail_chdir.asc
https://www.freebsd.org/security/advisories/FreeBSD-SA-21:04.jail_remove.asc

closing this based on @mjg's feedback.
I'm going to have to rethink this completely.