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

Record and restore SELinux context for mocked /dev nodes #222

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Conversation

martinpitt
Copy link
Owner

If libselinux is available, record the original node SELinux context into an internal __DEVCONTEXT property, and restore it in umockdev-run. This property can also be set via the API.

Fixes #220

@martinpitt
Copy link
Owner Author

@ikerexxe : Do you mind having a quick look? Not necessarily on the implementation details (although any review there is much appreciated), but wheter this suits your use case? You can test the packit COPR rpms even if you want. Thanks!

@martinpitt martinpitt mentioned this pull request Dec 14, 2023
@martinpitt martinpitt marked this pull request as draft December 14, 2023 15:58
@martinpitt
Copy link
Owner Author

Meh, COPR builders have devices with a wholly different SELinux context, their /dev/null is unconfined_u:object_r:user_tmp_t:s0 -- likely an unpacked chroot tarball with static files?

@martinpitt
Copy link
Owner Author

That's better. I'll deal with this nixos failure tomorrow, but @ikerexxe you probably care most about Fedora or C9S?

@ikerexxe
Copy link

That's better. I'll deal with this nixos failure tomorrow, but @ikerexxe you probably care most about Fedora or C9S?

Yes, Fedora or C9S is good for me. Unfortunately, I don't have any FIDO2 keys with me right now so I won't be able to test it until Monday 🤦

@martinpitt
Copy link
Owner Author

@ikerexxe : No worries. I'll sort out the nixos failure today in the meantime. BTW, @allisonkarlitskaya is very interested in mocking a FIDO2 key for testing -- is any of your test code public by any chance?

@martinpitt

This comment was marked as resolved.

If libselinux is available, record the original node SELinux context
into an internal `__DEVCONTEXT` property, and restore it in
`umockdev-run`. This property can also be set via the API.

Fixes #220
@ikerexxe
Copy link

I don't think it's working as expected, but I might be doing something wrong.

If I run the command before applying these changes I get the following selinux labels for the recordings:

ls -lZ
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0  8262 dic 18 09:36 yk.ioctl
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0  4081 dic 18 09:36 yk.script
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0 10936 dic 18 09:36 yk.umockdev

As a reference, the device has the following labels:

ls -lZ /dev/hidraw8 
crw-rw----+ 1 root root system_u:object_r:usb_device_t:s0 241, 8 dic 18 09:25 /dev/hidraw8

So, if I update the umockdev package and record the communication I'd expect to see system_u:object_r:usb_device_t:s0 as labels, but I get the same as before:

ls -lZ
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0  8262 dic 18 09:38 selinux.ioctl
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0  4336 dic 18 09:38 selinux.script
-rw-r--r--. 1 ipedrosa ipedrosa unconfined_u:object_r:user_home_t:s0 11087 dic 18 09:38 selinux.umockdev

Do I need to set any additional option in the CLI? It's not clear to me. By the way, I'm testing it using Fedora 39.

@martinpitt
Copy link
Owner Author

martinpitt commented Dec 18, 2023

I'm not sure why you refer to the SELinux contexts of the recordings files. That is entirely a functionality between the shell and how you handle the files. They will be in git etc, which doesn't preserve SELinux context anyway. It also shouldn't matter. The point of this PR, and the subject of your bug report was the SELinux context of the emulated /dev/* nodes.

To validate, I checked that the build log correctly builds with SELinux support:

Library libselinux found: YES
Check usable header "selinux/selinux.h" : YES

I installed the rpms as described. With umockdev-record /dev/null it outputs

E: __DEVCONTEXT=system_u:object_r:null_device_t:s0

Does that work for you? I.e. do you get __DEVCONTEXT in your recording? How do the emulated devices look like?

@ikerexxe
Copy link

I'm not sure why you refer to the SELinux contexts of the recordings files. That is entirely a functionality between the shell and how you handle the files. They will be in git etc, which doesn't preserve SELinux context anyway. It also shouldn't matter. The point of this PR, and the subject of your bug report was the SELinux context of the emulated /dev/* nodes.

I shouldn't have done the testing on the Monday morning. Forget that comment.

Does that work for you? I.e. do you get __DEVCONTEXT in your recording? How do the emulated devices look like?

Yes, that's working fine. If I run it for /dev/null I get E: __DEVCONTEXT=system_u:object_r:null_device_t:s0. Whereas if I do it for the actual FIDO2 key I get E: __DEVCONTEXT=system_u:object_r:usb_device_t:s0, which is the expected output.

@martinpitt
Copy link
Owner Author

@ikerexxe 🤣 We are all ready for some EOY holidays 😁 Thanks for testing!

@martinpitt martinpitt merged commit a08e391 into main Dec 18, 2023
35 checks passed
@martinpitt martinpitt deleted the selinux branch December 18, 2023 10:14
@martinpitt
Copy link
Owner Author

@ikerexxe Do you want/need a release with this right away, or are the COPR packages enough for now? I'd still like to discuss your other issue #221.

@ikerexxe
Copy link

@ikerexxe 🤣 We are all ready for some EOY holidays 😁 Thanks for testing!

Only two days and a half of work and I won't work again until next year 😄

@ikerexxe Do you want/need a release with this right away, or are the COPR packages enough for now? I'd still like to discuss your other issue #221.

Let's finish #221 first. I don't think it makes any sense for us, as we need the two functionalities.

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.

Set SELinux context
2 participants