search

martinpitt / umockdev

Mock hardware devices for creating unit tests and bug reporting
https://launchpad.net/umockdev
GNU Lesser General Public License v2.1
308 stars 54 forks source link

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

Closed martinpitt closed 8 months ago

martinpitt commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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

ikerexxe commented 8 months ago

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 :facepalm:

martinpitt commented 8 months ago

@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 commented 8 months ago

I cannot reproduce the nix failure in a local container run, and I know almost nothing about how the nix packaging/building works. So I'll do some experimental pushes to hopefully gather better logs.

update: it was just a rare flake apparently. I keep the extra logging for the time being.

ikerexxe commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@ikerexxe :rofl: We are all ready for some EOY holidays :grin: Thanks for testing!

martinpitt commented 8 months ago

@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 commented 8 months ago

@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 :smile:

@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.