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

modif: keep plugdev group unless nou2f is used #6664

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

northboot
Copy link
Contributor

To make hardware tokens available for ordinary users, some distributions include a udev rule to make the corresponding entry in /dev/... available for users belonging to a specific group.

The options noroot and nogroups currently break this behavior.

This PR implements a fix that checks whether browser-disable-u2f is set to no in order to disable these two options.

@kmk3 kmk3 changed the title Fix U2F for Firefox/Chromium profiles: fix u2f for firefox/chromium Feb 25, 2025
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

To make hardware tokens available for ordinary users, some distributions
include a udev rule to make the corresponding entry in /dev/... available for
users belonging to a specific group.

Which group?

It would likely be better to just whitelist the group(s) if nou2f is not
used.

For example, see:

Also, does the issue still happen with
firejail-git?

@northboot northboot force-pushed the master branch 2 times, most recently from 9ffcbfc to aa0a69a Compare February 25, 2025 03:12
@northboot
Copy link
Contributor Author

northboot commented Feb 25, 2025

Thanks for the feedback. The group in question is "users" on Void Linux.

The issue is also present on the latest build, but based on your example I've implemented a fix to keep that group: 8576b7d

@kmk3 kmk3 changed the title profiles: fix u2f for firefox/chromium modif: keep users group unless nou2f is used Feb 25, 2025
@kmk3
Copy link
Collaborator

kmk3 commented Feb 27, 2025

Thanks for the feedback.

No problem.

The group in question is "users" on Void Linux.

That seems like a pretty generic group name.

What else is it intended to allow access to?

What is the output of the following:

find /dev -group users | LC_ALL=C sort -u

@northboot northboot changed the title modif: keep users group unless nou2f is used modif: keep plugdev group unless noinput is used Mar 3, 2025
@northboot
Copy link
Contributor Author

northboot commented Mar 3, 2025

Void Linux is now using plugdev group for FIDO2 access: void-linux/void-packages#54519.

I've updated and tested the code accordingly, by keeping group plugdev unless noinput is used as a parameter.
noinput is now also ignored in the Firefox profile in case browser-disable-u2f no is set in the firejail config file.

@kmk3
Copy link
Collaborator

kmk3 commented Mar 6, 2025

Void Linux is now using plugdev group for FIDO2 access:
void-linux/void-packages#54519.

Nice, glad to see that it's more specific now.

I've updated and tested the code accordingly, by keeping group plugdev
unless noinput is used as a parameter. noinput is now also ignored in the
Firefox profile in case browser-disable-u2f no is set in the firejail
config file.

Why change from nou2f to noinput?

Giving access to all of /dev/input seems rather broad.

What is the output of the following command?

find /dev -group plugdev | LC_ALL=C sort -u

It's unclear to me where exactly these devices appear in /dev.

Also, note that this PR currently breaks test-seccomp-extra:

@northboot
Copy link
Contributor Author

What is the output of the following command?
find /dev -group plugdev | LC_ALL=C sort -u

The YubiKey appears as the following device nodes:

$ find /dev -group plugdev | LC_ALL=C sort -u
/dev/hidraw1
/dev/hidraw2
$ ls -l /dev/hidraw1
crw-rw---- 1 root plugdev 240, 1 Mar  7 19:53 /dev/hidraw1
$ ls -l /dev/hidraw2
crw-rw---- 1 root plugdev 240, 2 Mar  7 19:53 /dev/hidraw2

Usually permission to these devices is given by the session manager (logind) but udev does also change the group permissions so users without a session manager can still access them.

Why change from nou2f to noinput?

I thought that users who grant access to input devices are also likely ok with firejail also giving them access to plugdev devices, for me they seem very similar.
But I can revert the change and keep group plugdev unless no2uf is set, I think that way there isn't any change needed in firefox-common.profile.

@kmk3
Copy link
Collaborator

kmk3 commented Mar 7, 2025

What is the output of the following command?
find /dev -group plugdev | LC_ALL=C sort -u

The YubiKey appears as the following device nodes:

$ find /dev -group plugdev | LC_ALL=C sort -u
/dev/hidraw1
/dev/hidraw2
$ ls -l /dev/hidraw1
crw-rw---- 1 root plugdev 240, 1 Mar  7 19:53 /dev/hidraw1
$ ls -l /dev/hidraw2
crw-rw---- 1 root plugdev 240, 2 Mar  7 19:53 /dev/hidraw2

Usually permission to these devices is given by the session manager (logind)
but udev does also change the group permissions so users without a session
manager can still access them.

Thanks for the details.

Why change from nou2f to noinput?

I thought that users who grant access to input devices are also likely ok
with firejail also giving them access to plugdev devices, for me they seem
very similar. But I can revert the change and keep group plugdev unless
no2uf is set, I think that way there isn't any change needed in
firefox-common.profile.

I see what you mean, but for the sake of consistency I'd just use nou2f for
this for now.

Also, do the security keys actually show up in /dev/input? I thought they only
appeared in /dev/hidraw or in a more specific path.

Ideally they would indeed appear in a more specific path (like /dev/tpm for TPM
devices) or use a dedicated group (is plugdev used for all removable usb
devices?).

Anyway, feel free to open a discussion about nou2f, noinput and security
keys.

@northboot
Copy link
Contributor Author

I have now switched back to nou2f. I did some testing, including whether it is detecting the YubiKey when the sandbox is already launched and if it works when pcscd is running, both working fine.

Also, do the security keys actually show up in /dev/input? I thought they only
appeared in /dev/hidraw or in a more specific path.

Only in /dev/hidraw

Ideally they would indeed appear in a more specific path (like /dev/tpm for TPM
devices) or use a dedicated group (is plugdev used for all removable usb
devices?).

At least on my system, only some devices are accessed via plugdev.
For example, my keyboard is using plugdev and my webcam is using input (both USB).
Systems running Xorg as root may not require their user(s) to be in input group, but I think plugdev is still required in some cases.

The build pipeline still fails, do you know how to further debug this?
This is the output of firejail --debug --seccomp.block-secondary pwd:

[...]
Installing /run/firejail/mnt/seccomp/seccomp.namespaces.32 seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.namespaces seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.block_secondary seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.protocol seccomp filter
[...]

@northboot northboot changed the title modif: keep plugdev group unless noinput is used modif: keep plugdev group unless nou2f is used Mar 8, 2025
To make hardware tokens available for ordinary users, some distributions
include a udev rule to make the corresponding entry in /dev available
for users belonging to a specific group.

In the case of Void Linux, it now uses the `plugdev` group for FIDO2
access[1] and when using a YubiKey, it appears as the following device
nodes:

    $ find /dev -group plugdev | LC_ALL=C sort -u
    /dev/hidraw1
    /dev/hidraw2
    $ ls -l /dev/hidraw1 /dev/hidraw2
    crw-rw---- 1 root plugdev 240, 1 Mar  7 19:53 /dev/hidraw1
    crw-rw---- 1 root plugdev 240, 2 Mar  7 19:53 /dev/hidraw2

[1] void-linux/void-packages#54519
@kmk3
Copy link
Collaborator

kmk3 commented Mar 10, 2025

The build pipeline still fails, do you know how to further debug this? This
is the output of firejail --debug --seccomp.block-secondary pwd:

[...]
Installing /run/firejail/mnt/seccomp/seccomp.namespaces.32 seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.namespaces seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.block_secondary seccomp filter
Installing /run/firejail/mnt/seccomp/seccomp.protocol seccomp filter
[...]

Comparing a sucessful run against a failed run (see TESTING ERROR 12), it
seems that the test expects to find exactly 9 complementary groups for some
reason (and this PR makes there be 10), so I just changed it to 10.

From https://github.com/netblue30/firejail/actions/runs/13737961110/job/38423882671:

2025-03-08T13:13:05.4452512Z TESTING: noroot (test/seccomp-extras/noroot.exp)
2025-03-08T13:13:06.4990443Z cat /proc/self/gid_map | wc -l
2025-03-08T13:13:06.4993809Z runner@fv-az740-277:~/work/firejail/firejail/test/seccomp-extra$ 
2025-03-08T13:13:06.4995695Z </test/seccomp-extra$ cat /proc/self/gid_map | wc -l
2025-03-08T13:13:06.5009115Z 9

From https://github.com/netblue30/firejail/actions/runs/13731504189/job/38423927322:

2025-03-08T13:15:41.8655542Z TESTING: noroot (test/seccomp-extras/noroot.exp)
[...]
2025-03-08T13:15:42.9204330Z runner@fv-az1055-883:~/work/firejail/firejail/test/seccomp-extra$ 
2025-03-08T13:15:42.9205444Z </test/seccomp-extra$ cat /proc/self/gid_map | wc -l
2025-03-08T13:15:42.9218954Z 10
2025-03-08T13:15:52.9224059Z runner@fv-az1055-883:~/work/firejail/firejail/test/seccomp-extra$ TESTING ERROR 12

Also added some details to the commit message; feel free to edit it if you
want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants