-
Notifications
You must be signed in to change notification settings - Fork 629
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update selinux label from container_file_t to nfs_t #1965
Conversation
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.
Thanks
@@ -12,7 +12,7 @@ if [ "$#" -ne 1 ]; then | |||
fi | |||
|
|||
NAME="$1" | |||
expected="context=system_u:object_r:container_file_t:s0" | |||
expected="context=system_u:object_r:nfs_t:s0" |
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.
Could you add a comment to explain why nfs_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.
Shall we use the same comment as in description?
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.
Also when compared with 9p on qemu, it also using nfs_t. So I think we're using this label as all those mounts are remote mounts.
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.
SGTM
@@ -14,7 +14,7 @@ if [ -d /sys/fs/selinux ]; then | |||
for line in $(grep -n virtiofs </etc/fstab | cut -d':' -f1); do | |||
OPTIONS=$(awk -v line="$line" 'NR==line {print $4}' /etc/fstab) | |||
if [[ ${OPTIONS} != *"context"* ]]; then | |||
sed -i -e "$line""s/comment=cloudconfig/comment=cloudconfig,context=\"system_u:object_r:container_file_t:s0\"/g" /etc/fstab | |||
sed -i -e "$line""s/comment=cloudconfig/comment=cloudconfig,context=\"system_u:object_r:nfs_t:s0\"/g" /etc/fstab |
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.
Could you add a comment to explain why nfs_t
?
@@ -14,7 +14,7 @@ mkdir -p /mnt/lima-rosetta | |||
|
|||
#Check selinux is enabled by kernel | |||
if [ -d /sys/fs/selinux ]; then | |||
mount -t virtiofs vz-rosetta /mnt/lima-rosetta -o context="system_u:object_r:container_file_t:s0" | |||
mount -t virtiofs vz-rosetta /mnt/lima-rosetta -o context="system_u:object_r:nfs_t:s0" |
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.
Could you add a comment to explain why nfs_t
?
When using vz & virtiofs, initially container_file_t selinux label was considered which works perfectly for container work loads but it might break for other work loads if the process is running with different label. Also these are the remote mounts from the host machine, so keeping the label as nfs_t fits right. Package container-selinux by default adds rules for nfs_t context which allows container workloads to work as well. Signed-off-by: T K Chandra Hasan <t.k.chandra.hasan@ibm.com>
6f16f21
to
81a4904
Compare
@@ -14,7 +14,16 @@ if [ -d /sys/fs/selinux ]; then | |||
for line in $(grep -n virtiofs </etc/fstab | cut -d':' -f1); do | |||
OPTIONS=$(awk -v line="$line" 'NR==line {print $4}' /etc/fstab) | |||
if [[ ${OPTIONS} != *"context"* ]]; then | |||
sed -i -e "$line""s/comment=cloudconfig/comment=cloudconfig,context=\"system_u:object_r:container_file_t:s0\"/g" /etc/fstab | |||
########################################################################################## | |||
## When using vz & virtiofs, initially container_file_t selinux label |
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.
indentation seems broken
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.
Lint looks ok 🤔
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.
Sorry, looks like just a glitch of github web UI
When using vz & virtiofs, initially container_file_t selinux label was considered which works perfectly for container work loads but it might break for other work loads if the process is running with different label. Also these are the remote mounts from the host machine, keeping the label as nfs_t fits right. Package "container-selinux" by default adds rules for nfs_t context which allows container workloads to work as well.