Add insertRule method

[ZoltanFridrich]

Currently, appendRule function allows appending rules either to the end of the last ruleset or after a rule with the given id. This behavior is not very flexible as you cant append a rule to the beginning of any ruleset and you are unable to append a rule into an empty ruleset. To improve this, rulesets can now be referred to by their filename. And instead of changing the appendRule method a new API call InsertRule has been added.

InsertRule method will receive a rule and optional arguments: parent_id (same as parent_id in appendRule), ruleset (this string is used to match a prefix of a ruleset name, basically helps you to choose into which ruleset the rule should be inserted)

After this addition, one new option is added to usbguard append-rule command:
--ruleset (-r) prefix : prefix of a ruleset where the rule should be inserted

• also --after (-a) now accepts 0 as a valid ID. In this case, the rule is appended to the beginning of the file
  This addition allows the user to append new rules at any position in any ruleset, even into an empty ruleset.

The addition of named rulesets also opens more options for enhancements into the future, for example, usbguard list-rules might be able to separate output based on rulesets or print only rules within a separate ruleset.

Resolves #471

[lgtm-com]

This pull request introduces 1 alert when merging 9936e2a into 57022f1 - view on LGTM.com

new alerts:

• 1 for Declaration hides parameter

[muelli]

I wonder whether easier ways exist to achieve "prepend".
Is allowing 0 as a parent id problematic?

[ZoltanFridrich]

I wonder whether easier ways exist to achieve "prepend".
Is allowing 0 as a parent id problematic?

Actually, it shouldn't be problematic at all. However, you need to be able to specify the ruleset anyway as you would not be able to prepend to any ruleset other than the first. I will remove --before option and rather make it such that --after 0 means inserting the rule at the beginning.

[lgtm-com]

This pull request introduces 1 alert when merging 0748f3c into 57022f1 - view on LGTM.com

new alerts:

• 1 for Missing return statement

[muelli]

However, you need to be able to specify the ruleset anyway

hm. I'm a bit lost here.
The DBus interface doesn't require a ruleset, does it? AFAICS, it's appendRule (IN s rule, IN u parent_id, OUT u id);.

And I believe I have successfully used "0" to prepend in the past.
Or was I just lucky and have always had empty rules, anyway?

[ZoltanFridrich]

And I believe I have successfully used "0" to prepend in the past.
Or was I just lucky and have always had empty rules, anyway?

It might have been possible in the past as the underlying code works that way, however with current version it should be impossible as it checks for the presence of rule with parent_id (which will always be missing for 0) and the append-rule fails. If I remember it correctly.

[muelli]

I am confused with the concept of rulesets. The only other place where this is exposed seems to be the org.usbguard.Policy::listRules call: https://usbguard.github.io/documentation/dbus/doc-org.usbguard.Policy.html#gdbus-method-org-usbguard-Policy.listRules
I can't find the term anywhere else.
As such, I wonder how I as a user would know what string to provide. Is the empty string a valid input? Where do I get the other "prefixes" from to make a choice?

[muelli]

however with current version it should be impossible as it checks for the presence of rule with parent_id (which will always be missing for 0) and the append-rule fails. If I remember it correctly.

yeah, I does seem to fail. USBGuard must have changed behaviour.
But I fail to see where the failure is coming from. From looking at

usbguard/src/Library/public/usbguard/RuleSet.cpp

Lines 104 to 105 in 034f378

	else if (parent_id == 0) {
	_rules.insert(_rules.begin(), rule_ptr);

I can see the intention the give "0" the "prepending" semantic. What am I missing?

[ZoltanFridrich]

I can see the intention the give "0" the "prepending" semantic. What am I missing?

It was implemented in RuleSet::appendRule but not in Policy::appendRule from which the RuleSet::appendRule method is called. The Policy::appendRule method checks whether the given id exists within the ruleset, but the rule with id 0 can never exist within a ruleset. Thats why it fails.

[muelli]

Thanks for explaining. I think I see how the code rejects parent rule id 0.
This probably fails:

usbguard/src/Library/public/usbguard/Policy.cpp

Line 78 in 034f378

auto _parent_rule = ruleset->getRule(parent_id);

It seems that the rules.d changes broke the behaviour of prepending by appending with parent rule id 0.

I guess it would be easy to restore the behaviour by checking for "0", just as RuleSet::appendRule does.

[ZoltanFridrich]

I guess it would be easy to restore the behaviour by checking for "0", just as RuleSet::appendRule does.

Yes, it would be, but I wanted to expand the usbguard-cli to work with policy files in rules.d folder.

[ZoltanFridrich]

I have stopped to work on this PR as I am moving to other projects.

This PR is supposed to enhance users ability to append rules into the rule files as the current appendRule method does not provide enough flexibility (cant append at the beginning of a rule file or chose a rule file to append to).

Please, if anybody wants to finish this PR feel free to do so.