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

USB Guard | Depend on it and configure rules #166

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

monsieuremre
Copy link
Contributor

We should depend on usbguard as a package. When installed on debian, usb guard settings are configured in a way that all devices are rejected, only those that were already plugged in before the daemon starts are allowed.

Normally this is not optimal. We should by default reject everything and only whitelist the devices that we know and trust. But this requires configuration at the user end. So for now, we better leave the debian config as is.

We also add new rules with our config file. These rules do not configure the daemon. They define how the daemon behaves when it is 'applying policy'. The policy is by default blocking everything. We slacken this by allowing devices with mass storage interface, so the users can plug in their external hard drives and usb's. Then we blacklist especially suspicuous storage devices. Those that try to also behave like a keyboard at the same time for example.

Normally this is also not optimal, but it is a big improvement. We don't allow anything else. That means everything else is implicitly blocked. If the user plugs in a keyboard or mouse or anything else, it will be blocked. If these devices are plugged in when the machine starts up, they are allowed. This should not be too inconvenient for the user, because most peripherals are almost always already plugged in when the machine starts up. If not, they can be manually allowed or the system can be rebooted for the device to be allowed after having booted.

@adrelanos
Copy link
Member

@monsieuremre
Copy link
Contributor Author

If we don't depend on the package, there is very little sense in having configuration files installed for it. So which is it going to be, are you planning to drop all the various dependencies?

@adrelanos
Copy link
Member

"Depends: usbguard" will be added in source code repository kicksecure-meta-packages. Maybe to kicksecure-cli-host-packages-recommended.

The config files are good to have here.

related:

@monsieuremre
Copy link
Contributor Author

So @adrelanos what are waiting regarding this?

@adrelanos
Copy link
Member

  1. please removed the Depends:
  2. please add a link to the source: https://usbguard.github.io/documentation/rule-language
  3. the following comment seems wrong.
## Allow all USB devices with mass storage interface
allow with-interface equals { 08:*:* }

This actually does something else as per documentation.

This policy will block any device that isn’t just a mass storage device. Devices with a hidden keyboard interface in a USB flash disk will be blocked. Only devices with a single mass storage interface will be allowed to interact with the operating system.

Best to copy this verbatim?

  1. Where did you find the following one?
reject with-interface all-of { 08:*:* 0a:*:* }

@monsieuremre
Copy link
Contributor Author

Where did you find the following one?

The rules aren't necessarily taken from a source. Some of them I wrote myself. It means: reject a device, that presents itself as a mass stroage device, and a CDC data device, at the same time.

Class codes for different interfaces can be looked up easily here.

@neoniobium
Copy link

Just a remark.

I tried this rules for about 2 months now with various USB devices including hard drives, cameras, microphones and speakers and have not encountered any inconveniences yet.

Copy link

@DimSystem DimSystem left a comment

Choose a reason for hiding this comment

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

Code is also found on the usbguard docs.

Reject devices with suspicious combination of interfaces

A USB flash disk which implements a keyboard or a network interface is very suspicious. The following set of rules forms a policy which allows USB flash disks and explicitly rejects devices with an additional and suspicious (as defined before) interface.

allow with-interface equals { 08:*:* }
reject with-interface all-of { 08:*:* 03:00:* }
reject with-interface all-of { 08:*:* 03:01:* }
reject with-interface all-of { 08:*:* e0:*:* }
reject with-interface all-of { 08:*:* 02:*:* }

The policy rejects all USB flash disk devices with an interface from the HID/Keyboard, Communications and Wireless classes. Please note that blacklisting is the wrong approach and you shouldn’t just blacklist a set of devices and allow the rest. The policy above assumes that blocking is the implicit default. Rejecting a set of devices considered as “bad” is a good approach how to limit the exposure of the OS to such devices as much as possible.

Seems ok to me. Will interfere with like single board computers, but as commented, for default config and average user, seems good.

Here are some slides from blackhat 14, that confirms that sus usb devices most likely contains multiples descriptors.

@monsieuremre
Copy link
Contributor Author

Code is also found on the usbguard docs.

Exactly. This is where the core concept stems from as @adrelanos pointed it out before. I have made some more additions with more of the commonly used interfaces to the list to make it a little more extensive. Which is completely not necessary since they are normally blocked anyway unless we explicitly allow them in the rules. The original list itself is also completely superfluous. This is just a fallback in case the usbguard config is modified to allow by default. Let me fix that and make it more clear to avoid confusion.

Actually let me update the rules to add some more little things from the usb guard documentation.

Also, I will get rid of the combinations because it might introduce confusion, since no explicit rejecting is needed anyway, unless we change the configuration to allow by default, which we do not. I should have made this more clear in the beginning.

Copy link
Contributor

@ArrayBolt3 ArrayBolt3 left a comment

Choose a reason for hiding this comment

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

I like what this is intended to accomplish and think it's a great step forward. I'll give this some testing and report back how it works.


# Explicitly reject any interface that is not documented and/or defined by USB.org
# Note: Most probably superfluous
reject with-interface none-of { 01:*:* 02:*:* 03:*:* 04:*:* 05:*:* 06:*:* 07:*:* 08:*:* 09:*:* 0a:*:* 0b:*:* 0d:*:* 0e:*:* 0f:*:* 10:*:* 11:*:* 12:*:* 13:*:* 14:*:* 3c:*:* dc:*:* e0:*:* ef:*:* fe:*:* ff:*:*}
Copy link
Contributor

Choose a reason for hiding this comment

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

According to https://usb.org/defined-class-codes, there is no base class of 04, so that should be removed from the list. There also is a base class 00 for devices where class information is supposed to be determined from their interface descriptors, so that maybe should be added here. (If we're blocking everything by default anyway and this is superfluous, it probably won't harm security at least.)

Also, you seem to be missing a space before the closing } here. Not sure if that will result in a syntax error or not.

allow with-interface one-of { 03:00:01 03:01:01 } if !allowed-matches(with-interface one-of { 03:00:01 03:01:01 })

# Allow only one mouse to be connected
allow with-interface one-of { 03:00:02 03:01:02 } if !allowed-matches(with-interface one-of { 03:00:02 03:01:02 })
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the USB HID spec (https://usb.org/sites/default/files/hid1_11.pdf), there's a couple of invalid rules here - 03:00:01 and 03:00:02 will never exist, the only 03:00:* devices that should exist are 03:00:00 devices. Quoting the spec:

A variety of protocols are supported HID devices. The bInterfaceProtocol member of an Interface descriptor only has meaning if the bInterfaceSubClass member declares that the device supports a boot interface, otherwise it is 0.

bInterfaceProtocol is this last byte of the triplet, while bInterfaceSubClass is the byte in the middle. When the byte in the middle is 00, it is not declaring that the device supports a boot interface, thus the last byte will be 00.

Worthy of note, this rule will prohibit mice and keyboards that don't have boot interface support (I think my Logitech MX devices fall into this category), and it will probably prohibit HID devices other than mice and keyboards such as touchscreens. I don't know how common mice and keyboards without boot interface support are though. We should probably document how to disable this rule for people with problematic devices.

allow with-interface one-of { 03:00:02 03:01:02 } if !allowed-matches(with-interface one-of { 03:00:02 03:01:02 })

# Explicitly reject any device with a mouse/keyboard interface in combination with some other interface
# Mouses and keyboards should only have one interface for all legitimate use cases
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, mouse and keyboard devices might legitimately have two or even three interfaces, in the case of a wireless mouse+keyboard combo with a single receiver. The Microsoft Sculpt desktop set probably falls into this category, it consists of a keyboard, mouse, and touchpad, all separate but all sharing one receiver. So this second line of the comment isn't really accurate from a factual standpoint.

The second line of the comment also fails to reflect what the rules do. It prohibits mixing HID and other interfaces, it notably does not prohibit mixing multiple HID interfaces together. This is actually good IMO, but it's not what the comment says, so I think this second line should just be removed, or perhaps replaced with:

# Mice and keyboards should likely never have non-HID interfaces provided alongside them

If this line is kept, mouses should change to mice here also.

# Explicitly reject any device with a mouse/keyboard interface in combination with some other interface
# Mouses and keyboards should only have one interface for all legitimate use cases
reject with-interface all-of { 03:*:* 02:*:* }
reject with-interface all-of { 03:*:* 04:*:* }
Copy link
Contributor

Choose a reason for hiding this comment

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

Here again there is no base class of 04, so this line should be removed. Also, this list misses the 00 and 01 base classes at the start.

# Note: Most probably superfluous
reject with-interface none-of { 01:*:* 02:*:* 03:*:* 04:*:* 05:*:* 06:*:* 07:*:* 08:*:* 09:*:* 0a:*:* 0b:*:* 0d:*:* 0e:*:* 0f:*:* 10:*:* 11:*:* 12:*:* 13:*:* 14:*:* 3c:*:* dc:*:* e0:*:* ef:*:* fe:*:* ff:*:*}

### Allow all mouses and keyboards, in a sense, so the user can conveniently change them without restrating the daemon.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo fixes: mouses -> mice, restrating -> restarting

@@ -30,6 +30,7 @@ Depends: adduser,
python3,
secure-delete,
sudo,
usbguard,
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned previously, this needs to be removed from debian/control so that it can be shipped by a metapackage.

@ArrayBolt3
Copy link
Contributor

I tested this PR on physical hardware with the functional changes requested above. Surprisingly, my Logitech MX Mechanical Mini keyboard worked - upon closer examination it appears that it identifies itself as being a keyboard and a mouse that have boot interface support, and then also identifies itself as an HID device without boot interface support. Not sure how that works, but I'm guessing the keyboard and mouse bits were allowed while the "extra" device (probably for advanced configuration) was disallowed. A USB flash drive also worked just fine. I didn't test things that I expected should be rejected due to the limited number of USB ports in my system and devices available to me at the time.

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