netblue30 / firejail

Linux namespaces and seccomp-bpf sandbox
https://firejail.wordpress.com
GNU General Public License v2.0
5.58k stars 555 forks source link

Allow U2F security keys by default #5280

Open hackerb9 opened 1 year ago

hackerb9 commented 1 year ago

Is your feature request related to a problem? Please describe.

Yes, there is a problem. Firejail works so well that it is easy to forget that it is installed. Then, when someone (okay, it was me) buys a U2F security key (Yubico, Solokeys, etc) they will be frustrated that it does not work in their browser. It took me a while to figure it out, but luckily, I wasn't the only one who had installed Firejail a long time ago and forgot about it: a web search turned up Firejail among the possible reasons why U2F might not work in Firefox. The advice given (to whitelist everything) was incorrect, but it at least got me pointed in the right direction.

Describe the solution you'd like

Please set the default for browse-disable-u2f to no.

Describe alternatives you've considered

Not applicable.

Additional context

While it makes sense to block USB access by default, security keys are a special case. They increase security and complement the protections given by Firejail. While I haven't done a poll, I would guess that the great majority of individual Firejail users who have U2F keys would want to be able to use them in their web browser. (Firejail users who do not have U2F keys would not be affected by this change other than that if they ever get a U2F key, it will work.)

A default of 'no' still allows anyone who wants to disable U2F to simply change it to 'yes'. But, I am hard pressed to imagine a use case where anyone would find that beneficial. I had originally thought perhaps public kiosks or for large deployments at a corporation, but in both cases, you would either choose to have no USB ports (to prevent Windows driver exploits) or you'd want U2F to work so that people can authenticate securely.

rusty-snake commented 1 year ago

Duplicate of #4295

rusty-snake commented 1 year ago

Firejail users who do not have U2F keys would not be affected by this change other than that if they ever get a U2F key, it will work.

They will be affected. I don't have any u2f hardware and --nou2f changes ls -l /dev | wc -l for me.

The problem is that nou2f is named a bit misleading, it should be better named nohidraw.



And this change (enumerate bad devices instead of enumerate good devices) will not affect other users? https://github.com/netblue30/firejail/blob/06d3fd05814da2bbf1f9f30a722092a562cf16b2/etc/profile-a-l/firefox-common.profile#L55

rusty-snake commented 1 year ago

They increase security

Well yes they increase security, but it's another kind of security than firejail provides.

hackerb9 commented 1 year ago

They increase security

Well yes they increase security, but it's another kind of security than firejail provides.

I agree and add that it would be helpful if Firejail worked by default with U2F keys as they provide a form of security that Firejail does not.

Thank you for correcting me that this change would affect users who do not use U2F keys. How serious of a problem is it?

When you say it changes "hidraw", do you mean that setting browse-disable-u2f to no actually allows the browser to access any USB Human Interface Device in Firejail? While I expected there would be the usual security/convenience tradeoff, that sounds like a huge hole. And, of course, I would not want the default changed until Firejail has a way to limit access to only U2F keys.

rusty-snake commented 1 year ago

When you say it changes "hidraw", do you mean that setting browse-disable-u2f to no actually allows the browser to access any USB Human Interface Device in Firejail?

Yes, w/o nou2f any HID device file in /dev is bind-mounted into the sandbox (in my case my optical mouse). The only permission checks then are the default check of the kernel.

In the past it was called nousb IIRC.

it would be helpful if Firejail worked by default with U2F keys

Fully agreed, the only reason why it is blocked by default are the costs.

How serious of a problem is it?

There are two issues:

  1. nou2f allows more than u2f keys
  2. private-dev needs to be dropped (or you have no real advantage for new users over setting browse-disable-u2f by hand)
    • Information leak of available (outside of the sandbox) but blocked devices. (Admittedly this is also leaked via sysfs until we get disable-sys.inc)
    • All devices not explicitly blocked are allowed (tty*, nvme*, nvram, rtc?, tpm*, vhost-*, vcs*, ...). (Permission check of the kernel still apply)
    • Any device plugged in after the sandbox is started is allowed (permission checks of kernel still apply) even if there is a matching no*.
rusty-snake commented 1 year ago

So to allow U2F keys by default in browser we would need

  1. checks that only U2F keys are allowed. I thing the list of vendors for U2F keys is fairly limited so you could check them, AFAIK USB devices have something comparable to the MAC of a network card.
  2. Find a way to push new devices into the sandbox with private-dev.
hackerb9 commented 1 year ago

it would be helpful if Firejail worked by default with U2F keys

Fully agreed, the only reason why it is blocked by default are the costs.

Now that I understand that Firejail would allow access to all USB devices, I completely agree with the decision to keep U2F keys off by default.

I hope that the name of the option can be changed to something like, browser-allow-all-usb-access. When I flipped the browse-disable-u2f toggle on my machine, I had mistakenly presumed it only affected U2F.

hackerb9 commented 1 year ago
  1. checks that only U2F keys are allowed. I thing the list of vendors for U2F keys is fairly limited so you could check them, AFAIK USB devices have something comparable to the MAC of a network card.

You probably already know this, but Yubico used to have their own udev rules, 70-u2f.rules. I believe that now the list of vendors and their U2F keys is kept in (/lib/udev/hwdb.d/20-usb-vendor-model.hwdb), but I'm not exactly sure which rules and TAGS get applied to them. On my machine, my keys get tagged security-device and the property ID_FIDO_TOKEN gets set to 1.

Output from udevadm monitor -p -t security-device ``` UDEV [139854.852974] add /devices/pci0000:00/0000:00:14.0/usb2/2-2/2-2:1.0/0003:0483:A2CA.0014/hidraw/hidraw4 (hidraw) ACTION=add DEVPATH=/devices/pci0000:00/0000:00:14.0/usb2/2-2/2-2:1.0/0003:0483:A2CA.0014/hidraw/hidraw4 SUBSYSTEM=hidraw DEVNAME=/dev/hidraw4 SEQNUM=4226 USEC_INITIALIZED=139854852625 ID_FIDO_TOKEN=1 ID_SECURITY_TOKEN=1 ID_PATH=pci-0000:00:14.0-usb-0:2:1.0 ID_PATH_TAG=pci-0000_00_14_0-usb-0_2_1_0 ID_FOR_SEAT=hidraw-pci-0000_00_14_0-usb-0_2_1_0 MAJOR=243 MINOR=4 DEVLINKS=/dev/solosecure--4 /dev/solo /dev/solo--4 TAGS=:seat:security-device:uaccess: CURRENT_TAGS=:seat:security-device:uaccess: ```
hackerb9 commented 1 year ago

A-ha! If you don't want to rely on udev existing, you can examine the HID report for the declared usage of a device to determine if it is a FIDO key. Here's how systemd does it in fido_id_desc.c:

Click to see fido_id_desc.c ```C /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* Inspired by Andrew Lutomirski's 'u2f-hidraw-policy.c' */ #include #include #include #include #include "fido_id_desc.h" #define HID_RPTDESC_FIRST_BYTE_LONG_ITEM 0xfeu #define HID_RPTDESC_TYPE_GLOBAL 0x1u #define HID_RPTDESC_TYPE_LOCAL 0x2u #define HID_RPTDESC_TAG_USAGE_PAGE 0x0u #define HID_RPTDESC_TAG_USAGE 0x0u /* * HID usage for FIDO CTAP1 ("U2F") and CTAP2 security tokens. * https://fidoalliance.org/specs/fido-u2f-v1.0-ps-20141009/fido-u2f-u2f_hid.h-v1.0-ps-20141009.txt * https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#usb-discovery * https://www.usb.org/sites/default/files/hutrr48.pdf */ #define FIDO_FULL_USAGE_CTAPHID 0xf1d00001u /* * Parses a HID report descriptor and identifies FIDO CTAP1 ("U2F")/CTAP2 security tokens based on their * declared usage. * A positive return value indicates that the report descriptor belongs to a FIDO security token. * https://www.usb.org/sites/default/files/documents/hid1_11.pdf (Section 6.2.2) */ int is_fido_security_token_desc(const uint8_t *desc, size_t desc_len) { uint32_t usage = 0; for (size_t pos = 0; pos < desc_len; ) { uint8_t tag, type, size_code; size_t size; uint32_t value; /* Report descriptors consists of short items (1-5 bytes) and long items (3-258 bytes). */ if (desc[pos] == HID_RPTDESC_FIRST_BYTE_LONG_ITEM) { /* No long items are defined in the spec; skip them. * The length of the data in a long item is contained in the byte after the long * item tag. The header consists of three bytes: special long item tag, length, * actual tag. */ if (pos + 1 >= desc_len) return -EINVAL; pos += desc[pos + 1] + 3; continue; } /* The first byte of a short item encodes tag, type and size. */ tag = desc[pos] >> 4; /* Bits 7 to 4 */ type = (desc[pos] >> 2) & 0x3; /* Bits 3 and 2 */ size_code = desc[pos] & 0x3; /* Bits 1 and 0 */ /* Size is coded as follows: * 0 -> 0 bytes, 1 -> 1 byte, 2 -> 2 bytes, 3 -> 4 bytes */ size = size_code < 3 ? size_code : 4; /* Consume header byte. */ pos++; /* Extract the item value coded on size bytes. */ if (pos + size > desc_len) return -EINVAL; value = 0; for (size_t i = 0; i < size; i++) value |= (uint32_t) desc[pos + i] << (8 * i); /* Consume value bytes. */ pos += size; if (type == HID_RPTDESC_TYPE_GLOBAL && tag == HID_RPTDESC_TAG_USAGE_PAGE) { /* A usage page is a 16 bit value coded on at most 16 bits. */ if (size > 2) return -EINVAL; /* A usage page sets the upper 16 bits of a following usage. */ usage = (value & 0x0000ffffu) << 16; } if (type == HID_RPTDESC_TYPE_LOCAL && tag == HID_RPTDESC_TAG_USAGE) { /* A usage is a 32 bit value, but is prepended with the current usage page if * coded on less than 4 bytes (that is, at most 2 bytes). */ if (size == 4) usage = value; else usage = (usage & 0xffff0000u) | (value & 0x0000ffffu); if (usage == FIDO_FULL_USAGE_CTAPHID) return 1; } } return 0; } ```
rusty-snake commented 1 year ago

browser-allow-all-usb-access

Renaming has always the problem with backward compatibility and outdated tutorials, maybe a warning in the description would be better.

hackerb9 commented 1 year ago

browser-allow-all-usb-access

Renaming has always the problem with backward compatibility and outdated tutorials, maybe a warning in the description would be better.

That works for me.

kmk3 commented 1 year ago

@rusty-snake commented on Jul 31:

Firejail users who do not have U2F keys would not be affected by this change other than that if they ever get a U2F key, it will work.

They will be affected. I don't have any u2f hardware and --nou2f changes ls -l /dev | wc -l for me.

The problem is that nou2f is named a bit misleading, it should be better named nohidraw.

Well, you're in luck, as I decided to do so after seeing #5329 and thinking that bluetooth headsets could maybe be blocked by nou2f (which would be rather confusing). As apparently HIDs are not limited to USB devices. From Linux's Documentation/hid/hidraw.rst:

The hidraw driver provides a raw interface to USB and Bluetooth Human Interface Devices (HIDs). It differs from hiddev in that reports sent and received are not parsed by the HID parser, but are sent to and received from the device unmodified.

A benefit of hidraw is that its use by userspace applications is independent of the underlying hardware type. Currently, hidraw is implemented for USB and Bluetooth. In the future, as new hardware bus types are developed which use the HID specification, hidraw will be expanded to add support for these new bus types.

Most of the changes that would be needed are done, but I need to clarify some things for the docs (I'll probably open a question for it).


And this change (enumerate bad devices instead of enumerate good devices) will not affect other users?

https://github.com/netblue30/firejail/blob/06d3fd05814da2bbf1f9f30a722092a562cf16b2/etc/profile-a-l/firefox-common.profile#L55

~I can see why BROWSER_DISABLE_U2F would disable nou2f, but why disable private-dev? Are there U2F keys that show up in paths other than /dev/hidraw*?~

Nevermind, after some digging I get it now:

That was added on commit 32c366911 ("Browsers: private-dev conditional with BROWSER_DISABLE_U2F", 2020-01-21), which references #3170, which leads to #3513 and this comment:

@SkewedZeppelin commented on Jul 16, 2020:

iirc the private-dev is disabled only to allow u2f dongles be connected at will you can remove that conditional for more security, but you then have to have your dongle connected before launching the browser/sandbox I personally prefer the latter, but too many issues were filed that u2f wasn't working.

kmk3 commented 1 year ago

@rusty-snake commented on Aug 2:

browser-allow-all-usb-access

Renaming has always the problem with backward compatibility and outdated tutorials, maybe a warning in the description would be better.

We could add a new one and deprecate the current one. For example:

Or even:

To make grepping for all occurrences of nohidraw easier.

Now considering the above comment about private-dev, maybe there could be a separate toggle for private-dev in browsers? So that users can choose between security/usability. For example:

If the reasoning for each is explained on firejail.config and if they're referenced in the description of --nohidraw, I think that it would be clear enough.

rusty-snake commented 1 year ago

separate toggle

Sounds good. I was thinking about merging both conditions into one config option (i.e. deny, allow and allow-restricted).

We could add a new one and deprecate the current one

What would be the behaviour if both are present? IMHO we should error out then and let the user fix the configuration.

hackerb9 commented 1 year ago

So to allow U2F keys by default in browser we would need

  1. checks that only U2F keys are allowed. I think the list of vendors for U2F keys is fairly limited so you could check them, AFAIK USB devices have something comparable to the MAC of a network card.
  1. Find a way to push new devices into the sandbox with private-dev.

I have wrangled together an example, based @amluto's u2f-hidraw-policy, showing how Firejail can solve number 1.

Please download isu2f.c from here: https://gist.github.com/hackerb9/4abd1484c48c857b4c4b73f4fc9c8e7d

It is a simple command line program that checks for U2F keys using just the USB report descriptor. It does not require a database of USB IDs, nor does it depend on extra libraries, udev, or systemd. It works best if sysfs is mounted on /sys, but it can read the report descriptor from any file.

Here is an example run:

$ cc isu2f.c -o isu2f

$ ./isu2f /sys/bus/usb/devices/*/*/report_descriptor
/sys/bus/usb/devices/3-3:1.0/0003:04F3:0C5B.0002: Nope, not a U2F_TOKEN
/sys/bus/usb/devices/3-3:1.1/0003:04F3:0C5B.0003: Nope, not a U2F_TOKEN
/sys/bus/usb/devices/3-4:1.0/0003:0483:A2CA.0006: Yup, this is a U2F_TOKEN