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

modify usbguard_allow_* rules to use new match-all keyword #5055

Merged
merged 2 commits into from
Dec 10, 2019

Conversation

vojtapolasek
Copy link
Collaborator

@vojtapolasek vojtapolasek commented Dec 5, 2019

Description:

Usbguard_allow_hid and usbguard_allow_hub now use the new match-all keyword provided by Usbguard rule syntax.
Both rules DO NOT check for actual content of /etc/usbguard/rules.conf file, they just check if the file is not empty or is not missing. If file is empty or missing, they add respective lines. In other cases, intentional modification by administrator is assumed and therefore rule passes. This is reflected by warning in rule.yml.
OCIL, OVAL and Bash remediations are modified.
Also a new rule usbguard_allow_hid_and_hub is created because combining these two separate rules turns out to be difficult. Oval, rule file, Bash remediation and tests are created for this new rule. All three rules use common OVAL check and therefore only one test is provided.

Rationale:

IMPORTANT: usbguard_allow_* rules should be understood primarily as convenience
Rules. They help administrators to ease management of machines.
The match-all keyword allows better matching of USB devices, for example
match-all { 03:: will match HID devices with possibly multiple HID interfaces but will not match devices including any other USB class interface
This was not reasonably easy to do before.

@yuumasato yuumasato self-assigned this Dec 5, 2019
Copy link
Contributor

@dahaic dahaic left a comment

Choose a reason for hiding this comment

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

Sorry, this won't be as simple as that - these two rules need to be able to work together - when both are enabled, it should result in allow with-interface match-all { 03:*:* 09:*:*}

@yuumasato
Copy link
Member

Sorry, this won't be as simple as that - these two rules need to be able to work together - when both are enabled, it should result in allow with-interface match-all { 03:*:* 09:*:*}

Nice catch. An use case that should be allowed is devices with any combination of 03:*:* and 09:*:*.
In this case, having them as separate rules will make remediation and check complicated. As there can be interaction with any other existing rules.

Merging them and treating as a single config seems the sane approach.
The individual rules can still exist, for folks who only want to check for keyboards or hubs.

Copy link
Contributor

@dahaic dahaic left a comment

Choose a reason for hiding this comment

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

OK, after internal discussions, based on this rules being considered "convenience" (it does not bring anything security related to the table - that's provided by the default USBGuard installation), we have decided to change the behavior of the rule a bit:

  • OVAL will fail when file is empty. Otherwise it is expected that user made deliberate changes and we are OK with it.
  • Remediation will allow devices with any combination of 03:*:* and 09:00:*
  • Description will be explicit about this rule being convenience, not security requirement.

@dahaic
Copy link
Contributor

dahaic commented Dec 9, 2019

Also, I have checked there is SRG mapped to this rule - SRG-OS-000114-GPOS-00059. This reads as The operating system must uniquely identify peripherals before establishing a connection.

This should be mapped to the service_usbguard_enabled, not to the convenience functions making holes in the strict default.

…yword

oval DOES NOT CHECK for actual values, only if file is empty or completely missing
taking into account possible modifications by administrator
created new shared oval for usbguard_allow_* rules
this rule covers a possibility when devices having both HID and hub USB class need to be allowed
this rule also contains tests for the shared oval
combinations of usbguard_allow_hid and usbguard_allow_hub are replaced in profiles by usbguard_allow_hid_and_hub
@dahaic
Copy link
Contributor

dahaic commented Dec 10, 2019

Thank you @vojtapolasek , looks great!

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.

4 participants