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

Add initial policy type for wayland #381

Merged
merged 1 commit into from
Apr 26, 2024
Merged

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Apr 24, 2024

No description provided.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan
Copy link
Member Author

rhatdan commented Apr 24, 2024

@dougsland @alexlarsson PTAL

@dougsland
Copy link
Collaborator

lgtm

@dougsland
Copy link
Collaborator

CI/CD timeout, re-triggering the jobs.

@alexlarsson
Copy link
Collaborator

I'll let @martinezjavier comment here, I haven't had time to look at the issues.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 25, 2024

Note this will not fix the problem, it only sets up a type which we can gather AVCs off of and then update the policy for the access required to leak wayland through the QM.
Setup the QM quadlets that will use the wayland socket with

SecurityLabelFileType=qm_container_wayland_t

And then run the tests in permissive mode. Gather up the AVC's and then I will help you add allow rules to the qm.if file to support wayland.

@martinezjavier
Copy link

Note this will not fix the problem, it only sets up a type which we can gather AVCs off of and then update the policy for the access required to leak wayland through the QM. Setup the QM quadlets that will use the wayland socket with

SecurityLabelFileType=qm_container_wayland_t

I believe this is a typo and you meant SecurityLabelType=qm_container_wayland_t instead ?

And SecurityLabelFileType= should be set to SecurityLabelFileType=qm_container_wayland_file_t ?

And then run the tests in permissive mode. Gather up the AVC's and then I will help you add allow rules to the qm.if file to support wayland.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 25, 2024

Sorry it should be

SecurityLabelType=qm_container_wayland_t

We need to set the process label.

@dougsland
Copy link
Collaborator

Looks everything is correct now. @martinezjavier ack as well?

Fixes #391

@dougsland
Copy link
Collaborator

cc @sandrobonazzola I believe this is interesting for Sandro's team if I am not wrong.

@dougsland
Copy link
Collaborator

maybe interesting to you as well @aesteve-rh

@sandrobonazzola
Copy link
Collaborator

yes, interesting. @telemaco would be interested as well

@martinezjavier
Copy link

Looks everything is correct now. @martinezjavier ack as well?

As @rhatdan mentioned, this PR is only the starting point to allow labeling a container with a wayland workload. I'm getting the list of rules that are needed and will share here once I managed to get the simple-shell wayland compositor running with SELinux in enforcing mode.

@martinezjavier
Copy link

martinezjavier commented Apr 26, 2024

I've pushed https://github.com/martinezjavier/qm/tree/wayland-rules that contains the rules I added on top of @rhatdan's patch. There are a lot of rules so I don't know if I'm doing something wrong but with that the only missing AVC that is reported is the following:

$ audit2allow -a 
#============= qm_t ==============
allow qm_t unconfined_t:process transition;

AFAIU this is due the simple-shell container running as an unconfined user (digital-cockpit). So I tried to map the user to a different role as explained in https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/using_selinux/managing-confined-and-unconfined-users_using-selinux but that didn't help and I sill see the mentioned AVC denial.

$ usermod -G wheel -Z system_u digital-cockpit

$ restorecon -R -F -v /var/lib/digital-cockpit
Relabeled /var/lib/digital-cockpit from system_u:object_r:var_lib_t:s0 to system_u:object_r:user_home_dir_t:s0
Relabeled /var/lib/digital-cockpit/.bash_logout from system_u:object_r:var_lib_t:s0 to system_u:object_r:user_home_t:s0
Relabeled /var/lib/digital-cockpit/.bashrc from system_u:object_r:var_lib_t:s0 to system_u:object_r:user_home_t:s0
Relabeled /var/lib/digital-cockpit/.bash_profile from system_u:object_r:var_lib_t:s0 to system_u:object_r:user_home_t:s0
$semanage login -l

Login Name           SELinux User         MLS/MCS Range        Service

__default__          unconfined_u         s0-s0:c0.c1023       *
digital-cockpit      system_u             s0-s0:c0.c1023       *
root                 unconfined_u         s0-s0:c0.c1023       *

@rhatdan
Copy link
Member Author

rhatdan commented Apr 26, 2024

allow qm_t unconfined_t:process transition;
Never we don't want to allow qm_t to execute unconfined_t processes.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 26, 2024

Only AVC messages about qm_*_t should be added, but please pass them by me.

@dougsland
Copy link
Collaborator

dougsland commented Apr 26, 2024

Going to merge the initial patch from @rhatdan as we all acked. Let's merge the next improvement in second patch so we can have the initial support in tree already. Thanks!

@dougsland dougsland merged commit f19543f into containers:main Apr 26, 2024
7 checks passed
@martinezjavier
Copy link

allow qm_t unconfined_t:process transition; Never we don't want to allow qm_t to execute unconfined_t processes.

Yes, you mentioned that already and the reason why I attempted to solve that by changing the digital-cockpit user to not be an unconfined user instead of adding an allow rule for that AVC.

@martinezjavier
Copy link

martinezjavier commented Apr 26, 2024

Only AVC messages about qm_*_t should be added, but please pass them by me.

Don't worry that we will always pass by you before attempting to add any rule. The qm_wayland_t rules that I identified are the following:

	# Rules for wayland containers
	allow $1_container_wayland_t $1_file_t:chr_file { open map };
	allow $1_container_wayland_t $1_file_t:lnk_file read;
	dev_rw_dri($1_container_wayland_t)
	allow $1_container_wayland_t $1_file_t:dir { add_name write };
	allow $1_container_wayland_t $1_file_t:file { create write };
	allow $1_container_wayland_t sysfs_t:file { open read };
	allow $1_container_wayland_t var_run_t:dir { add_name remove_name write };
	allow $1_container_wayland_t var_run_t:file { create getattr lock open read write };
	allow $1_container_wayland_t var_run_t:sock_file { create write getattr unlink };
	allow $1_container_wayland_t $1_file_t:sock_file write;
	allow $1_container_wayland_t $1_t:unix_stream_socket connectto;
	allow $1_container_wayland_t unconfined_t:unix_stream_socket connectto;
	allow $1_container_wayland_t $1_t:dbus send_msg;

But as said, that is not enough due the fact that the DBus user session is started in the QM container right now. We are exploring with @telemaco if either that setup can happen in the wayland container as well or if the wayland compositor could be run as root.

Either of those options would avoid the need to add additional qm_t rules.

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.

5 participants