Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

file ownership and permissions, RPM consistency with installer, SETUID bit... #6619

Open
stormi opened this issue Aug 8, 2019 · 2 comments

Comments

@stormi
Copy link

commented Aug 8, 2019

Bug report summary

I'm working a on RPM package of netdata for XCP-ng (https://github.com/xcp-ng/xcp), as the release manager and main RPM packager. File ownership and permission issues have caught my attention so I've compared netdata-installer.sh with the RPM spec file netdata.spec.in and found a lot of differences, and packaging issues.

I've listed everything in the table below. There's a lot in one issue, some topics addressed here may be good candidates for spawning separate issues. It is dense, but I believe most of my comments are relevant (I'm quite experienced in RPM packaging). Some might not be relevant due to lack of knowledge of the context: I won't mind if you tell me so, but I'll defend my point of view if I think it's important.

OS / Environment

XCP-ng 8.0 / CentOS 7

Netdata version (ouput of netdata -V)

master

Description

Here's the table with all my findings. Do not miss the last columns, that github does not show unless you scroll horizontally.

Better readability of that table at https://framabin.org/p/?85b258768e5eefae#ZBKXImy9hWDRR21Na9VtGEv4yO9yjvONOwXUdDQdDFw=

Path netdata-installer.sh netdata.spec.in (RPM installed on XCP-ng and CentOS 7) Suggested Comments
/etc/netdata (dir) owner root.netdata
mode 0755
idem owner root.root
mode 0755
standard practice is root.root, and the directory is world-readable so the netdata group ownership is not useful
/etc/netdata/edit-config owner root.netdata
mode 0755
owner netdata.netdata
mode 0755
owner root.root
mode 0755
/etc/netdata/netdata.conf owner netdata.netdata
mode 0664
owner root.netdata
mode 0644
owner root.root
mode 0644
why does the installer script give full ownership to netdata user? Is netdata supposed to write into that file? And then why doesn't the RPM set the same ownership and permissions? If netdata is not supposed to write to that file, root.root would be more appropriate
all dirs in /etc/netdata owner root.netdata
mode 0755
(if I read the installation script well)
owner netdata.netdata
mode 0755
owner root.root
mode 0755
no benefit in specifying the netdata group with file mode 0755, so suggesting the standard root.root ownership for those directories
/usr/lib/systemd/system/netdata.service (not checked) owner netdata.netdata
mode 0750
owner root.root
mode 0644
All service files in /usr/lib/systemd/system must belong to root.root and have mode 0644. There's no reason to differ. And above of all no user but root should have write rights over anything in /usr/lib.
/usr/lib64/netdata (dir) owner root.netdata
mode 0755
idem owner root.root
mode 0755
no benefit in group ownership to netdata on this directory. Every single other directory in /usr/lib64 on my system belongs to root.root.
/usr/lib64/netdata/conf.d (dir) and subdirectories owner root.netdata
mode 0755
owner netdata.netdata
mode 0755
owner root.root
mode 0755
no benefit in group ownership to netdata on this directory. Every single other directory in /usr/lib64 on my system belongs to root.root. In the RPM, it's even worse: netdata has write rights! No user but root should have write rights over anything in /usr/lib64
files in /usr/lib64/netdata/conf.d owner root.netdata
mode 0640
owner netdata.netdata
mode 0750
? I understand that there's a will to not let the world read those files, so OK for root.netdata and 0640 if that's really needed. Else root.root 0644 would be more appropriate. In the RPM the files are executable for no reason and writeable by the netdata user, which once again should never happen in /usr/lib64.
/usr/libexec/netdata (dir) owner root.netdata
mode 0755
owner netdata.netdata
mode 0755
owner root.root
mode 0755
No reason to not use root.root here: not meant to be modified, world-readable. In the RPM, again too many rights given to netdata user over the files.
directories in /usr/libexec/netdata owner root.netdata
mode 0755
owner root.netdata
mode 0750
owner root.root
mode 0755
in the result of the installer, no reason to use the netdata group instead of root? In the result of the RPM installation, the directories are not world-readable anymore. We're talking only about the directories here. The files are addressed below.
*.sh and *.py in /usr/libexec/netdata owner root.netdata
mode 0755
owner netdata.netdata
mode 0755
owner root.root
mode 0755
No benefit in attributing the world-readable files to the netdata group. Too many rights given to user netdata over the files in the RPM.
most .plugin files in /usr/libexec/netdata/plugins.d owner root.netdata
mode 0750
owner netdata.netdata
mode 0755
? root.netdata 0750 looks fine if we want no other user than netdata to run the plugins. But then why not do it for the .sh and .py files too? Else root.root 0755 would be more standard. Note: the case of plugins with special capabilities or SETUID bit is addressed below. Here we're talking about the others.
.../plugins.d/apps.plugin owner root.netdata

mode 0750 + cap_dac_read_search, cap_sys_ptrace=ep

OR

mode 4750 (SETUID bit)
owner root.netdata
mode 0550 + cap_dac_read_search, cap_sys_ptrace=ep
same as in netdata-installer.sh why 0550 instead of 0750 in the RPM? Note : in this situation I understand the need for root.netdata in conjunction with the capabilities or SETUID.
.../plugins.d/freeipmi.plugin owner root.netdata
mode 4750 (SETUID bit)
owner root.netdata
mode 4550
+ cap_setuid=ep
use mroe specific capabilities instead for better security? Why 4550 in the RPM? I'm also not fond of SETUID bits especially when the WEB user has the rights to execute the plugins as seems to be the case in the RPM. Couldn't more specific capabilities be used? Why does the RPM both set the SETUID bit AND the cap_setuid capability?
.../plugins.d/ioping.plugin owner root.netdata
mode 4750 (SETUID bit)
owner root.netdata
mode 0755
use capabilities instead for better security? Why does it have the SETUID bit set in the installer but not in the RPM?
.../plugins.d/nfacct.plugin owner root.netdata
mode 4750 (SETUID bit)
I haven't built it, but I see nothing in the RPM to give it the SETUID bit use capabilities instead for better security? Is the SETUID bit really needed if it's not set in the RPM? Could capabilities be used instead to reduce the attack surface?
.../plugins.d/perf.plugin owner root.netdata
mode 4750 (SETUID bit)
owner root.netdata
mode 4750 (SETUID bit)
+ cap_setuid=ep
use more specific capabilities instead for better security? Why does the RPM both set the SETUID bit AND the cap_setuid capability?
.../plugins.d/xenstat.plugin owner root.netdata
mode 4750 (SETUID bit)
owner root.netdata
mode 0755
use more specific capabilities instead for better security? SETUID or capabilities missing in the RPM for that plugin to work (tested)
.../plugins.d/cgroup-network owner root.netdata
mode 4750 (SETUID bit)
owner root.netdata
mode 0755
use more specific capabilities instead for better security? SETUID or capabilities missing in the RPM? update: I build without the %with_ns macro set, that's why the SETUID bit was not set. I wonder if the file should have been excluded from the resulting RPM though.
.../plugins.d/cgroup-network-helper.sh owner root.netdata
mode 0550
owner root.netdata
mode 0755
use more specific capabilities instead for better security? 0550 vs 0755. Update: default mode 0755 instead of the wanted 0550 probably due to the fact that I'm building without %with_ns. Maybe the file should be excluded from the resulting RPM though.
/usr/sbin/netdata (not checked) owner netdata.netdata!
mode 0755
owner root.root
mode 0755
executable in /usr/sbin writable by the netdata user, which is also the web user if installed via the RPM. Ouch.

stormi added a commit to xcp-ng-rpms/netdata that referenced this issue Aug 8, 2019

Try to fix file ownership and modes
We can't leave so many files in /usr/lib64, /usr/sbin, etc., owned by
the netdata user.

See upstream bug report netdata/netdata#6619

This may cause issues if netdata expects to be able to write in /etc

@paulkatsoulakis paulkatsoulakis self-assigned this Aug 9, 2019

@paulkatsoulakis paulkatsoulakis added this to the v1.17 milestone Aug 9, 2019

@paulkatsoulakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Thanks for your report @stormi, i 'll review your findings and let you know !

@paulkatsoulakis

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Hello @stormi, at a first glance i can definitely acknowledge the gap between the .RPM and netdata-installer.sh, this is a BUG and i will get to it directly.

Regarding the recommendations of what ownership and perms should be, i 'll discuss this with the team and let you know. Thanks again for taking the effort to provide this feedback!

@gpapamathND gpapamathND added the size:3 label Aug 16, 2019

@gpapamathND gpapamathND added this to Not Planned in Core via automation Aug 18, 2019

@gpapamathND gpapamathND moved this from Not Planned to To do in Core Aug 18, 2019

@paulkatsoulakis paulkatsoulakis moved this from To do to In progress in Core Aug 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.