Security - rc/init scripts #1873
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is Nathan's work to run all "rc" scripts (except pmcd) as pcp:pcp instead of root:root).
Reinstate some changes for systemd that were lost from Nathan's patch
set.
But more importantly, rework the structure of the rc scripts so they
can run as pcp:pcp for all platforms, not just those using systemd.
Specifically:
src/pmlogger
+ install rc_pmlogger in $PCP_SYSCONF_DIR/pmlogger/rc (will be run
as pcp:pcp, not root:root)
+ change pmlogger.service to run with User=pcp Group=pcp and execute
$PCP_SYSCONF_DIR/pmlogger/rc
+ add rc_wrapper installed in $PCP_RC_DIR/pmlogger to capture control
for init(1), insserv(1), etc. and for non-systemd platforms, run
$PCP_SYSCONF_DIR/pmlogger/rc as pcp:pcp
src/pmproxy
+ install rc_pmproxy in $PCP_SYSCONF_DIR/pmproxy/rc (will be run as
pcp:pcp, not root:root)
+ change pmproxy.service to run with User=pcp Group=pcp and execute
$PCP_SYSCONF_DIR/pmproxy/rc
+ add rc_wrapper installed in $PCP_RC_DIR/pmproxy to capture control
for init(1), insserv(1), etc. and for non-systemd platforms, run
$PCP_SYSCONF_DIR/pmproxy/rc as pcp:pcp
src/pmie
+ install rc_pmie in $PCP_SYSCONF_DIR/pmie/rc (will be run as
pcp:pcp, not root:root)
+ change pmie.service to run with User=pcp Group=pcp and execute
$PCP_SYSCONF_DIR/pmie/rc
+ add rc_wrapper installed in $PCP_RC_DIR/pmie to capture control
for init(1), insserv(1), etc. and for non-systemd platforms, run
$PCP_SYSCONF_DIR/pmie/rc as pcp:pcp
$PCP_SYSCONF_DIR/pcp/pmlogger may contain more than control files, e.g. $PCP_SYSCONF_DIR/pmlogger/rc that needs to be kept and not moved aside.
Need to cherry-pick pmproxy.options and pmproxy.conf, not all the $PCP_SYSCONF_DIR/pmproxy directory, as it may contain other files or scripts, e.g. $PCP_SYSCONF_DIR/pmproxy/rc that need to be kept and not moved aside.
If we're using runuser(1), we need -s /bin/sh because there is (by default) no login shell for the pcp user.
Intended for pmie and pmlogger to replace culling of $PCP_TMP_DIR/pmie and $PCP_TMP_DIR/pmlogger map directories in rc scripts.
Conflicts: qa/group reserved block of tests
Relieves rc scripts from any need to clean the map file directories.
modified: src/pmie/src/pmie.c
modified: src/pmlogger/src/ports.c
Only root:root processes write here, so it does not need to be pcp:pcp.
…/pmproxy directory
This routine is mostly used by PMDAs and pm* tools to create log files to capture diagnostic information. There were a number of ad hoc approaches to trying to save the previous version of a the log file logname in logname.prev ... these were being done in shell scripts run under various user id's and subject to some security concerns. The libpcp routine pmOpenLog now assumes responsibility for salvaging the previous logname, if any, in logname.prev. The shell scripts are not longer responsible for this activity.
Specifically user and group ownership and modes.
In the "special" symlink case, the link needs to not be a numeric PID, but the last path component of the link needs to be a numeric PID. root@bozo-vm:/var/lib/pcp/pmdas/simple# ls -l /var/lib/pcp/tmp/pmlogger total 4 -rw-r--r-- 1 pcp pcp 76 Dec 28 11:43 3416689 lrwxrwxrwx 1 pcp pcp 33 Dec 28 11:43 primary -> /var/lib/pcp/tmp/pmlogger/3416689 So primary -> /var/..../PID, not primary -> PID as the code had previously assumed.
Needed to move log files for PMDAs used in this test to someplace that is more easily writeable.
/run (default parent of $PCP_RUNE_DIR) is likely to be a tmpfs mounted empty at reboot. We can no longer ensure $PCP_RUN_DIR is set up from our "rc" scripts as some of them no longer run as root:root. tmpfiles.d(5) provides a mechanism to address this scenario that is more useful for us than the systemd unit config mechanism.
Mostly this is removing .log and .log.prev files that are hidden by .gitignore.
Fallout from some rc scripts no longer run as root (pmie and pmlogger in particular).
This is now done in pmOpenLog() and the suffix us .prev not .prior like everywhere else.
…correctly Track changes into the tarball packaging scripts.
For at least OpenBSD 7.4 (vm37) there is no setpriv(1) or runuser(1), so fallback to su(1).
For systems that don't have setpriv(1) or runuser(1), and where su(1) cannot be depended on (pam, su config, different command line options), fallback to our own runaspcp(1) to run shell commands from rc scripts are pcp:pcp
And in particular don't need mkdir for $PCP_LOG_DIR here.
No longer useful, the add_* scripts are much better.
From /proc/net/snmp Ip: ... OutTransmits seen on vm26 (CentOS Stream9) after recent update.
…ange
modified: qa/872.out
modified: qa/linux/proc_net_netstat
Add qa/1481 (new) to check tarball postinstall against qa/src/permslist for any discrepancies for the "odd" cases ... e.g. dirs owned pcp:pcp
Conflicts:
build/tar/postinstall.tail
qa/group
Accidential overlap between main and security branches.
Not needed, and won't work as intended now this scripts is run as the user "pcp" not "root". The dir creation is done elsewhere and the map file cleanup is now done by pmi as user "pcp" via __pmCleanMapDir().
natoscott
reviewed
Jan 29, 2024
There was a problem hiding this comment.
Hi @kmcdonell - its looking good. I'll add a few small nits inline shortly, but one higher level topic is I think the tmpfiles.d file needs to live elsewhere. AIUI the /etc location is for sysadmins, and as packagers we are supposed to use the /usr/lib location ... (PCP_SYSTEMDTMPFILES_DIR)
$ rpm -ql pcp |grep tmpf
/etc/tmpfiles.d/pcp.conf
/usr/lib/tmpfiles.d/pcp.conf
The first is new of course, the latter we generate during the RPM builds (for rpm-ostree). It'd be ideal if we can wrangle this content to be all in the second file.
I guess that means we need to shift this to be generated in the rpm/deb packaging rather than at this top level.
natoscott
reviewed
Jan 29, 2024
- move it from /etc/tmpfiles.d to /usr/lib/tmpfiles.d (where it belongs) - rename the installed file from pcp.conf to pcp-run-pcp.conf (to avoid conflicts with pcp.conf that's generated by rpm packaging on some platforms) - d -> D (so it is removed when PCP is (package) removed)
|
@natoscott re. tmpfiles.d ... we can just place the /run/pcp one in /usr/lib/tmpfiles.d/pcp-run-pcp.conf (avoids name conflict with rpm-generated ones) ... since /usr/lib/tmpfiles.d can exist without systemd, I'd prefer to use the explicit pathname rather than $(PCP_SYSTEMDTMPFILES_DIR) (which may translate to badness on a non-systemd platform). |
@KenJ yep, sounds good. Another option might be to share a name with the pcp-reboot-init service? |
As per discussion in performancecopilot#1873 and matches the name of the pcp-reboot-init script that does the same thing in the absence of systemd-tmpfiles(8).
|
@natoscott OK, since the tmpfiles.d conf and the pcp-reboot-init script are effectively alternate ways of getting the same job done, I'm happy to rename the conf file. This part is done in commits 1f28451aae and 97f85835. |
Thanks Nathan.
modified: man/man1/runaspcp.1
modified: man/man3i/__pmcleanmapdir.3
return ...; break; => return ...;
This is "proc_net_snmp:" NOT "proc_net_netstat:".
Changed from a prefix to a dir with mktemp(1), but refs to $tmp not changed in the script as a whole.
Directory should exist and chown is not needed, as this script runs as root from systemd.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A bunch of changes to provide improved security, especially for PCP's rc/init/systemd scripts