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

Create policy for a container_device_t #178

Merged
merged 1 commit into from
May 4, 2022
Merged

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Apr 22, 2022

This policy can be used for kubernetes/container plugins which add
devices to containers.

Signed-off-by: Daniel J Walsh [email protected]

@rhatdan
Copy link
Member Author

rhatdan commented Apr 22, 2022

Replaces: #177

@rhatdan
Copy link
Member Author

rhatdan commented Apr 22, 2022

@mregmi lets work together on this one. Could you try this policy out on your system and tell me what AVCs are created?

@mregmi
Copy link
Contributor

mregmi commented Apr 22, 2022

Thank you. i will test it out today.

@mregmi
Copy link
Contributor

mregmi commented Apr 22, 2022

Hi Dan, i am seeing error related to connectto.

type=AVC msg=audit(1650654069.545:12189): avc: denied { connectto } for pid=2653465 comm="intel_sgx_devic" path="/var/lib/kubelet/device-plugins/kubelet.sock" scontext=system_u:system_r:container_device_t:s0:c310,c693 tcontext=system_u:system_r:container_runtime_t:s0 tclass=unix_stream_socket permissive=0

audit2allow suggests the following
#============= container_device_t ==============
allow container_device_t container_runtime_t:unix_stream_socket connectto;

@rhatdan
Copy link
Member Author

rhatdan commented Apr 22, 2022

That means that kubernetes is not running with the correct label. Can you run restorecon on the kublet and see if it gets relabeled to kublet_exec_t, and then restart it and make sure it is running as kublet_t.

That would change the access to container_device_t connectto kublet_t which should be allowed.

@mregmi
Copy link
Contributor

mregmi commented Apr 22, 2022

yes it was not running with new label. restorecon worked. Thank you.

There are a also couple of things we need for this.
There is also an initcontainer for each plugin and they need access to things in /etc/kubernetes/. Those files are kubernetes_file_t. Do you think we need different labels for initcontainer or we can give access to the above label to container_device_t?

Also one of the plugin we are releasing later this year also needs access to sysfs (files in /sys) which have sysfs_t. the container_device_t would also need access to this. Do we want to add this while we are doing this?

lastly the workloads using these plugins will just need access to /dev (wont need connectto and other things). One way they can work is using the setsebool. do you think it would be better to have a workload specific label with just access to /dev/?

@rhatdan
Copy link
Member Author

rhatdan commented Apr 25, 2022

Can you write this up in a table. When you say access to sysfs_t, are you talking read or write? Also are you mounting in /sys from the host into the container? /sys is usually namespaced.

Are you saying only the init container needs to talk to kublet? And not the general continer?

Could we just run with
container_device_t == container_t + read/write devices?
init == container_device_t + read/write kublet socket?

@mregmi
Copy link
Contributor

mregmi commented Apr 25, 2022

So there are 3 types of the containers for device plugins:

  1. The device plugins (the container_device_t in current patch):
    container_device_t = container_t + r/w /dev (device_t) + r/w kubelet socket + r/w some parts of /sys (sysfs_t)
    Like /dev, /sys is also volume mounted from host

  2. init container
    container_device_init_t = container_t + kubernetes_file_t (because it needs access to /etc/kubernetes/node-feature-discovery )

  3. the third one is the normal containers with end users application. in most cases they just need /dev/ file access.
    container_t + access to /dev (device_t)

Thanks.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 26, 2022 via email

@mregmi
Copy link
Contributor

mregmi commented Apr 26, 2022

Yes lets go with these:

  1. Device plugins: container_device_plugin_t
  2. Init containers: container_device_plugin_init_t
  3. workload: container_device_t

The workload (3) will need both read and write to device_t.
Init container (2) will also need read/write access as it copies some file in that folder
Thanks.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 27, 2022

How does this look?

container_domain_template(container_device)
allow container_device_t device_node:chr_file rw_chr_file_perms;

container_domain_template(container_device_plugin)
container_kublet_stream_connect(container_device_plugin_t)
allow container_device_plugin_t device_node:chr_file rw_chr_file_perms;

container_domain_template(container_device_plugin_init)
container_kublet_stream_connect(container_device_plugin_init_t)
allow container_device_plugin_init_t device_node:chr_file rw_chr_file_perms;
manage_dirs_pattern(container_device_plugin_init_t, kubernetes_file_t, kubernetes_file_t)
manage_files_pattern(container_device_plugin_init_t, kubernetes_file_t, kubernetes_file_t)
manage_lnk_files_pattern(container_device_plugin_init_t, kubernetes_file_t, kubernetes_file_t)

@rhatdan rhatdan force-pushed the main branch 2 times, most recently from 834cadd to b849706 Compare April 27, 2022 19:10
@mregmi
Copy link
Contributor

mregmi commented Apr 27, 2022

one small comment.
container_device_plugin_init_t does not need socket communication with kubelet. so we can remove container_kublet_stream_connect(container_device_plugin_init_t)

container_device_plugin_t will also need rw access to /sys so a rule allowing to sysfs_t might be needed.

container.fc Outdated
/usr/local/s?bin/kubelet.* -- gen_context(system_u:object_r:container_runtime_exec_t,s0)
/usr/s?bin/hyperkube.* -- gen_context(system_u:object_r:container_runtime_exec_t,s0)
/usr/local/s?bin/hyperkube.* -- gen_context(system_u:object_r:container_runtime_exec_t,s0)
/usr/s?bin/kubelet.* -- gen_context(system_u:object_r:kublet_exec_t,s0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be kubelet_exec_t?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing.

@mythi
Copy link

mythi commented Apr 28, 2022

one small comment. container_device_plugin_init_t does not need socket communication with kubelet. so we can remove container_kublet_stream_connect(container_device_plugin_init_t)

container_device_plugin_t will also need rw access to /sys so a rule allowing to sysfs_t might be needed.

with some devices, the initContainer is used to write to /sys (e.g., to enable sriov_numvfs for PCIe device)

@mregmi
Copy link
Contributor

mregmi commented Apr 28, 2022

@rhatdan I also saw this error when testing the latest policy.

type=AVC msg=audit(1651188022.040:40076): avc: denied { read write } for pid=1185191 comm="cp" name="intel-sgx-epchook" dev="sda4" ino=614477480 scontext=system_u:system_r:container_device_plugin_init_t:s0:c178,c513 tcontext=system_u:object_r:container_file_t:s0:c351,c450 tclass=file permissive=1

type=AVC msg=audit(1651188022.058:40077): avc: denied { setattr } for pid=1185191 comm="cp" name="intel-sgx-epchook" dev="sda4" ino=614477480 scontext=system_u:system_r:container_device_plugin_init_t:s0:c178,c513 tcontext=system_u:object_r:container_file_t:s0:c351,c450 tclass=file permissive=1

audit2allow gives this warning.

#============= container_device_plugin_init_t ==============

#!!!! This avc is a constraint violation.  You would need to modify the attributes of either the source or target types to allow this access.
#Constraint rule:
#       mlsconstrain file { ioctl read lock execute execute_no_trans } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { write setattr append unlink link rename } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { create relabelto } ((h1 dom h2 -Fail-)  and (l2 eq h2)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { relabelfrom } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED

#       Possible cause is the source level (s0:c178,c513) and target level (s0:c351,c450) are different.
allow container_device_plugin_init_t container_file_t:file { read setattr write };

@rhatdan
Copy link
Member Author

rhatdan commented Apr 29, 2022

This means that you ran a container with one label which created content and then attempted to read/write the content from a different. If the content needs to be shared between multiple containers running in different Pods. then the container_file_t content has to be labeled as s0, not with a private label for the container.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 29, 2022

with some devices, the initContainer is used to write to /sys (e.g., to enable sriov_numvfs for PCIe device)

Added, does it need to set sysctls as well or just this one field.

@mregmi
Copy link
Contributor

mregmi commented Apr 29, 2022

I think this line dev_rw_sysfs((container_device_plugin_t)
is also needed for container_device_plugin_init_t

I will debug the above container_file_t issue.

Copy link
Contributor

@mregmi mregmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after a quick fix related to the things i commented. the labels works for the plugins. i think we can merge after fixing those small issues.

container.te Outdated
# communicate with kublet
container_domain_template(container_device_plugin)
allow container_device_plugin_t device_node:chr_file rw_chr_file_perms;
dev_rw_sysfs((container_device_plugin_t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a typo. should be dev_rw_sysfs(container_device_plugin_t)

container.te Outdated
# modify kublet configuration
container_domain_template(container_device_plugin_init)
allow container_device_plugin_init_t device_node:chr_file rw_chr_file_perms;
dev_rw_sysfs((container_device_plugin_t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be dev_rw_sysfs(container_device_plugin_init_t)

container.te Outdated
')


type kublet_exec_t;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in container.fc, it was changed to kubelet_exec_t. so these needs to be changed too. i saw module load error without this change.

@rhatdan
Copy link
Member Author

rhatdan commented May 2, 2022

@mregmi @Walnux PTAL

@mregmi
Copy link
Contributor

mregmi commented May 2, 2022

@rhatdan only one comment at dev_rw_sysfs lines. second one should be dev_rw_sysfs(container_device_plugin_init_t).
Other than that it looks good. I tested it on our system.

@mregmi
Copy link
Contributor

mregmi commented May 3, 2022

The latest push does not seem to have any change.

Also create policy for container_device_plugin_t and
container_device_plugin_init_t.

This policy can be used for kubernetes/container plugins which add
devices to containers.

Signed-off-by: Daniel J Walsh <[email protected]>
@mregmi
Copy link
Contributor

mregmi commented May 3, 2022

LGTM. Thank you @rhatdan for helping out with this issue.

@rhatdan rhatdan changed the title [WIP] Create policy for a container_device_t Create policy for a container_device_t May 4, 2022
@rhatdan rhatdan merged commit 15c20d7 into containers:main May 4, 2022
Tal-or added a commit to k8stopologyawareschedwg/deployer that referenced this pull request Aug 22, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

Signed-off-by: Talor Itzhak <[email protected]>
Tal-or added a commit to k8stopologyawareschedwg/deployer that referenced this pull request Aug 22, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

Signed-off-by: Talor Itzhak <[email protected]>
Tal-or added a commit to k8stopologyawareschedwg/deployer that referenced this pull request Aug 22, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

We gradually want to depracate the custom SELinux context
and use this one instead.

Signed-off-by: Talor Itzhak <[email protected]>
Tal-or added a commit to k8stopologyawareschedwg/deployer that referenced this pull request Aug 22, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

We gradually want to depracate the custom SELinux context
and use this one instead.

Signed-off-by: Talor Itzhak <[email protected]>
Tal-or added a commit to k8stopologyawareschedwg/deployer that referenced this pull request Aug 22, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

We gradually want to depracate the custom SELinux context
and use this one instead.

Signed-off-by: Talor Itzhak <[email protected]>
ffromani pushed a commit to ffromani/deployer that referenced this pull request Dec 16, 2024
The `container_device_plugin_t` label type, allows communication with
`kubelet_t` context: containers/container-selinux#178.

The PodResourceAPI socket is an object created by Kubelet so it inherents
the same process context, i.e. `kubelet_t`.

We gradually want to depracate the custom SELinux context
and use this one instead.

Signed-off-by: Talor Itzhak <[email protected]>
(cherry picked from commit 170037a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants