Add label to rules

[tweksteen]

This PR adds a new attribute to the Rule class named "label", as suggested in #283. This attribute can be used to store further background on a rule. For instance, allow id 045e:00db label "user-preference". It is then possible use the command usbguard list-rules -l "user-preference" to only return the rules matching this label.

Worth noticing:

• The IPC definition of listRules has been simplified to only return vector<Rule> (there is currently no client for the default_target attribute, both list-rules and dbus discard that value). If required, this could be implemented in another IPC method).
• The "query" parameter is used directly to store the label. This could be renamed to avoid misunderstandings. I also explored the option of creating a query rule and performing the matching via appliesTo but this changes the semantic of the rule (i.e. any rule without the label would not match anymore). Happy to change this point if another approach makes more sense.
• I dropped the check that a response is necessarily provided (with only optional/repeated fields, this might not be the case). Ideally, this should be implemented within payloadToMessage by verifying the returned value of ParseFromString. I would also suggest moving all the required field to optional at some point (reasoning at why messge type remove 'required,optional'? protocolbuffers/protobuf#2497).

[coveralls]

Coverage decreased (-0.1%) to 57.188% when pulling e6b2d21 on tweksteen:add_label_to_rules into f011198 on USBGuard:master.

[dkopecek]

This PR adds a new attribute to the Rule class named "label", as suggested in #283. This attribute can be used to store further background on a rule. For instance, allow id 045e:00db label "user-preference". It is then possible use the command usbguard list-rules -l "user-preference" to only return the rules matching this label.

Thanks again, this looks like an useful usability improvement.

Worth noticing:

* The IPC definition of listRules has been simplified to only return `vector<Rule>` (there is currently no client for the default_target attribute, both list-rules and dbus discard that value). If required, this could be implemented in another IPC method).

Ok, that makes sense.

* The "query" parameter is used directly to store the label. This could be renamed to avoid misunderstandings. I also explored the option of creating a query rule and performing the matching via appliesTo but this changes the semantic of the rule (i.e. any rule without the label would not match anymore). Happy to change this point if another approach makes more sense.

I have to look at the code again here, as I don't remember what was the intention. I think the query was supposed to be used to filter the result if needed.

* I dropped the check that a response is necessarily provided (with only optional/repeated fields, this might not be the case). Ideally, this should be implemented within `payloadToMessage` by verifying the returned value of ParseFromString. I would also suggest moving all the required field to optional at some point (reasoning at [protocolbuffers/protobuf#2497](https://github.com/protocolbuffers/protobuf/issues/2497)).

Ok, I've read the linked issue and agree with dropping the required/optional annotations.

[dkopecek]

* i.e. any rule without the label would not match anymore

Is this a problem? Maybe I'm not seeing the problem here. I would say this is how it should behave. i.e. list-rules should list rules that match on the attributes of the query rule:

query: match label "foo"

rule "allow id 1234:4568 label "foo" (matches)
rule "allow id 1234:4568" (does not match)
rule "allow id 1234:4568 label "bar" (does not match)
...

[tweksteen]

Is this a problem? Maybe I'm not seeing the problem here. I would say this is how it should behave. i.e. list-rules should list rules that match on the attributes of the query rule:

It is not a problem for list-rules, which would work as expected. But it is a problem for the standard rule matching when evaluating if a device should be authorised or denied. In this case, the device rule does not contain that label and therefore does not match any rule anymore. (This was implemented within the appliesTo method of RulePrivale.cpp, there might be another way to make that work).

[dkopecek]

Is this a problem? Maybe I'm not seeing the problem here. I would say this is how it should behave. i.e. list-rules should list rules that match on the attributes of the query rule:

It is not a problem for list-rules, which would work as expected. But it is a problem for the standard rule matching when evaluating if a device should be authorised or denied. In this case, the device rule does not contain that label and therefore does not match any rule anymore. (This was implemented within the appliesTo method of RulePrivale.cpp, there might be another way to make that work).

Ok, I see that now. To be honest I don't like that the list rules query parameter is now reserved and interpreted only as the label attribute value. But let's not block this feature on such a minor detail. We can meditate on it and deal with it later :-) Maybe there should be some sort of context parameter that would tell the API how to handle the "appliesTo" logic. Or the "device" and "match" rule targets [1] could be used to parametrize the applicability logic, e.g. a "match ..." rule when applied to a "device ..." rule would not invoke the matching on label attributes.

[1] https://github.com/USBGuard/usbguard/blob/master/src/Library/public/usbguard/Rule.hpp#L59

[dkopecek]

LG(enough)TM