[apparmor] [PATCH] utils: Don't enforce ordering of dbus rule attributes

[Tyler Hicks]

On 02/12/2017 01:30 PM, Christian Boltz wrote:
> Hello,
> 
> Am Mittwoch, 8. Februar 2017, 23:56:27 CET schrieb Tyler Hicks:
>> https://launchpad.net/bugs/1628286
>>
>> The utils were enforcing that the dbus rule attributes were strictly
>> ordered in the following fashion:
>>
>>  bus -> path -> interface -> member -> peer
>>
>> However, the parser has always accepted the attributes in any order.
>> If the system contained a profile which did not use the strict
>> ordering enforced by the utils, the utils would refuse to operate at
>> all.
>>
>> This patch eases the restriction on the ordering at the expense of the
>> utils no longer being able to detect and reject a single attribute
>> that is repeated multiple times. In that situation, only the last
>> occurrence of the attribute will be honored by the utils.
> 
> Also note that writing (in "clean" mode) a dbus rule back to a profile 
> will only include the last match. This has the advantage of accidently 
> fixing the profile, and the disadvantage of altering the profile without 
> any notice.
> 
>> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
>> Cc: Christian Boltz <apparmor at cboltz.de>
>> ---
>>  utils/apparmor/rule/dbus.py            | 12 ++++++------
>>  utils/test/test-dbus.py                |  6 ++++++
>>  utils/test/test-parser-simple-tests.py |  8 +++-----
>>  3 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/utils/apparmor/rule/dbus.py b/utils/apparmor/rule/dbus.py
>> index 60f1ecf..58dc7b5 100644
>> --- a/utils/apparmor/rule/dbus.py
>> +++ b/utils/apparmor/rule/dbus.py
>> @@ -40,11 +40,11 @@ RE_FLAG         =
>> '(?P<%s>(\S+|"[^"]+"|\(\s*\S+\s*\)|\(\s*"[^"]+"\)\s*))'    # s
>> RE_DBUS_DETAILS  = re.compile(
> 
> It's probably a good idea to add something like
>     # XXX this regex will allow repeated parameters, last one wins
>     # XXX (the parser will reject such rules)
> 
> 
>> diff --git a/utils/test/test-dbus.py b/utils/test/test-dbus.py
>> index 5b676bc..f1bcb25 100644
>> --- a/utils/test/test-dbus.py
>> +++ b/utils/test/test-dbus.py
>> @@ -89,6 +89,10 @@ class DbusTestParse(DbusTest):
> 
>> +       ('dbus bus=system path=/foo/bar bus=session,'
> 
> Please add
>     # XXX bus= specified twice, last one wins
> to make clear this test is about suboptimal behaviour
> 
>> +       ('dbus bus=1 bus=2 bus=3 bus=4 bus=5 bus=6,'
> 
> This deserves the same comment ;-) (well, s/twice/multipe times/)
> 
> 
> With these comments added,
>     Acked-by: Christian Boltz <apparmor at cboltz.de>

Thanks for the reviews! I've made the changes that you requested here.

> 
> 
> As I already stated in the bugreport, having a parse_dbus_rule() in 
> libapparmor would be even better ;-)

Agreed. I hope we can get to that point someday.

Tyler

> 
> 
> Regards,
> 
> Christian Boltz
> 
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170215/3041e93a/attachment.pgp>