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.

Sign up for GitHub

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
Open

USB Guard | Depend on it and configure rules #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d31e17
usbguard
monsieuremre Nov 17, 2023
7c8b9b2
30_security-misc.conf
monsieuremre Nov 17, 2023
4cae74d
Update 30_security-misc.conf
monsieuremre Jul 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions debian/control
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

${misc:Depends}
Replaces: anon-gpg-tweaks, swappiness-lowest, tcp-timestamps-disable
Description: Enhances Miscellaneous Security Settings
Expand Down
46 changes: 46 additions & 0 deletions etc/usbguard/rules.d/30_security-misc.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# We allow those that were plugged in before the daemon starts. Everything is blocked as the default. Following rules apply on top of this.

# 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 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

### Take extra measures to ensure security

# Allow only one keyboard to be connected
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.


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

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.

reject with-interface all-of { 03:*:* 05:*:* }
reject with-interface all-of { 03:*:* 06:*:* }
reject with-interface all-of { 03:*:* 07:*:* }
reject with-interface all-of { 03:*:* 08:*:* }
reject with-interface all-of { 03:*:* 09:*:* }
reject with-interface all-of { 03:*:* 0a:*:* }
reject with-interface all-of { 03:*:* 0b:*:* }
reject with-interface all-of { 03:*:* 0d:*:* }
reject with-interface all-of { 03:*:* 0e:*:* }
reject with-interface all-of { 03:*:* 0f:*:* }
reject with-interface all-of { 03:*:* 10:*:* }
reject with-interface all-of { 03:*:* 11:*:* }
reject with-interface all-of { 03:*:* 12:*:* }
reject with-interface all-of { 03:*:* 13:*:* }
reject with-interface all-of { 03:*:* 14:*:* }
reject with-interface all-of { 03:*:* 3c:*:* }
reject with-interface all-of { 03:*:* dc:*:* }
reject with-interface all-of { 03:*:* e0:*:* }
reject with-interface all-of { 03:*:* ef:*:* }
reject with-interface all-of { 03:*:* fe:*:* }
reject with-interface all-of { 03:*:* ff:*:* }

# Allow USB mass storage
# If and only if the USB device only has the mass storage interface and nothing extra
# Suspicious interface combinations with mass storage are blocked
allow with-interface equals { 08:*:* }