-
Notifications
You must be signed in to change notification settings - Fork 12
[Centos9] Add support for prometheus node-exporter container #54
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
[Centos9] Add support for prometheus node-exporter container #54
Conversation
The Monitoring chart in Rancher can be used with SELinux enabled, however with the container-selinux policy installed the node-exporter container inherits container_t, which is not allowed to run several tasks.
This commit adds a new type prom_node_exporter_t along with the required rules to allow node-exporter to run with least permissions.
|
cc @ca-hu for review |
|
done with the review :) ftr i did not have a test system so i did not check if permissions are missing, but i believe you can do that better than me anyways :) |
| ############################################################################ | ||
| require { | ||
| type container_runtime_t; | ||
| type prom_node_exporter_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might not need this type prom_node_exporter_t; in the require block since it is defined below
| # type prom_node_exporter_t # | ||
| # target: prometheus-node-exporter container for Rancher monitoring chart # | ||
| ############################################################################ | ||
| require { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- nitpick: above
gen_requireis used, hererequire. should not make a difference, but maybe for consistency - not only for this PR, but generally in this file: i am a bit confused why there are many duplications stated in the
gen_requiressections. they should not be needed i think. not sure if you want to split the file up in the future or what the plan is/was here? usually its just one block at the top
| dev_list_sysfs(prom_node_exporter_t) | ||
| dev_read_sysfs(prom_node_exporter_t) | ||
| files_read_etc_symlinks(prom_node_exporter_t) | ||
| init_read_state(prom_node_exporter_t) | ||
| kernel_read_network_state(prom_node_exporter_t) | ||
| kernel_read_network_state_symlinks(prom_node_exporter_t) | ||
| kernel_read_proc_files(prom_node_exporter_t) | ||
| kernel_read_proc_symlinks(prom_node_exporter_t) | ||
| kernel_read_software_raid_state(prom_node_exporter_t) | ||
| libs_read_lib_files(prom_node_exporter_t) | ||
| selinux_read_security_files(prom_node_exporter_t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense as the node_exporter reads system state, ok
| corenet_tcp_bind_generic_node(prom_node_exporter_t) | ||
| corenet_tcp_bind_generic_port(prom_node_exporter_t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, ok
| class fifo_file write; | ||
| } | ||
| type prom_node_exporter_t; | ||
| container_domain_template(prom_node_exporter_t, container) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be
container_domain_template(prom_node_exporter, container)
otherwise you will get this (note the _t_t suffix):
seinfo -x -a container_domain | grep prom_node
prom_node_exporter_t_t
| class fd use; | ||
| class fifo_file write; | ||
| } | ||
| type prom_node_exporter_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you fix the container_domain_template you dont need this line anymore
| allow container_runtime_t prom_node_exporter_t:dir { open read search }; | ||
| allow container_runtime_t prom_node_exporter_t:file { getattr open read }; | ||
| allow container_runtime_t prom_node_exporter_t:key { create search setattr view }; | ||
| allow container_runtime_t prom_node_exporter_t:lnk_file { getattr read }; | ||
| allow container_runtime_t prom_node_exporter_t:process { noatsecure rlimitinh siginh sigkill signal transition }; | ||
| allow prom_node_exporter_t container_runtime_t:fd use; | ||
| allow prom_node_exporter_t container_runtime_t:fifo_file write; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think these are not needed if you fix the container_domain_template line because they should be already included
you can check with sesearch -A -s container_runtime_t -t prom_node_exporter_t
| allow prom_node_exporter_t self:dir { getattr search }; | ||
| allow prom_node_exporter_t self:file { open read }; | ||
| allow prom_node_exporter_t self:lnk_file read; | ||
| allow prom_node_exporter_t self:netlink_route_socket { bind create getattr getopt nlmsg_read read write }; | ||
| allow prom_node_exporter_t self:process fork; | ||
| allow prom_node_exporter_t self:tcp_socket { accept bind create getattr listen read setopt write }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably these as well
| allow prom_node_exporter_t self:netlink_route_socket { bind create getattr getopt nlmsg_read read write }; | ||
| allow prom_node_exporter_t self:process fork; | ||
| allow prom_node_exporter_t self:tcp_socket { accept bind create getattr listen read setopt write }; | ||
| container_runtime_typebounds(prom_node_exporter_t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting that this is needed, did you see a nnp_transition? if so i did not find the denial, could you paste it for my curiousity? thanks :)
| type prom_node_exporter_t; | ||
| container_domain_template(prom_node_exporter_t, container) | ||
| virt_sandbox_domain(prom_node_exporter_t) | ||
| allow container_runtime_t prom_node_exporter_t:dir { open read search }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final nitpick, might make sense to do interfaces first and allow rules afterwards just for consistency
|
sorry, forgot to click "submit review", so you probably only saw my last comment.... |
Targeted Env: Centos9
Context: The Monitoring chart in Rancher can be used with SELinux enabled, however with the container-selinux policy installed (comes with k3s-selinux and rancher-selinux), the
node-exportercontainer inheritscontainer_t, which is not allowed to run several tasks. This makes the Monitoring app to stop running.Changes: This PR introduces the addition of a new type
prom_node_exporter_talong with the required rules to allow node-exporter to run with the least permissions.Example of AVC denials that come along the assignment of
prom_node_exporter_tto thenode-exportercontainer, and that are addressed in this PR.AVCs