[apparmor] DBus rule syntax for subject and peer components
Jamie Strandboge
jamie at canonical.com
Thu Jun 13 01:34:15 UTC 2013
On 06/12/2013 06:36 PM, Seth Arnold wrote:
> On Wed, Jun 12, 2013 at 03:42:34PM -0500, Jamie Strandboge wrote:
>>> So, here's a first shot at Proposal #4:
>>> [...]
>>> /usr/bin/gnome-screensaver {
>>> # Ignore file and accessibility bus access for this excercise
>>> file,
>>> dbus bus=accessibility,
>>>
>>> # sarnold> I think we could remove this entirely; for any bus with
>>> # bus= rules later on, add this rule implicitly. We can do this with
>>> # any of the proposals..
>>> # Talks to system and session buses
>>> # dbus bus={system,session} peer=(name=org.freedesktop.DBus) (send receive),
>>>
>>> dbus send {
>>> bus=system {
>>> peer {
>>> name=org.freedesktop.ConsoleKit
>>> path=/org/freedesktop/ConsoleKit/Manager
>>> interface=org.freedesktop.ConsoleKit.Manager
>>> },
>>> peer name=org.freedesktop.Accounts {
>>> {
>>> path=/org/freedesktop/Accounts
>>> interface=org.freedesktop.Accounts
>>> },
>>> {
>>> path=/org/freedesktop/Accounts/User*
>>> interface=org.freedesktop.DBus.Properties
>>> },
>>> }
>>> }
>>> bus=session {
>>> peer {
>>> name=org.gnome.SessionManager
>>> path=/org/gnome/SessionManager/Presence
>>> interface=org.freedesktop.DBus.Properties
>>> },
>>> peer {
>>> path=/org/gtk/vfs/mounttracker
>>> interface=org.gtk.vfs.MountTracker
>>> },
>>> peer {
>>> name=org.gnome.Shell
>>> path=/org/gnome/Shell
>>> interface=org.freedesktop.DBus.Properties
>>> },
>>> }
>>> }
>>>
>>> # Receives messages on the session bus
>>> dbus acquire bus=session {
>>> subj name=org.gnome.ScreenSaver,
>>> }
>>>
>>> dbus receive bus=session {
>>> subj path=/org/gnome/ScreenSaver {
>>> interface=org.freedesktop.DBus.Properties,
>>> interface=org.gnome.ScreenSaver {
>>> peer label=/usr/bin/gnome-settings-daemon,
>>> peer name=com.canonical.indicator.session,
>>> }
>>> }
>>> }
>>> }
>>>
>>
>> I really don't like Proposal #4 (no offense, I also really don't like
>> my own proposal #2). I think I don't like it because all the nested
>> brackets seem quite messy (especially when considering child and
>> sibling profiles) and, like John mentioned, nesting peer under subject
>> doesn't feel right. Furthermore, I don't like the feel of grouping
>> things under a single access. Granted, something like this:
>
> I'm not wedded to my proposal, but I really like that the horribly
> repetitive bits of dbus are written just once. Perhaps I abused this
> in my proposal -- I don't want to give the impression that only one
> 'receive' or 'send' block would be allowed.
>
I didn't mean to imply that you gave this impression. :) The
hierarchical mechanism doesn't feel intuitive as a profile author and is
harder for me to read (surely I would get used to it, but I'm betting
that at least some other policy auditors would have a similar feeling).
See below for why...
> Nesting peer under subject (or the equivalent nesting subject under
> peer) does feel strange, now that you and John have pointed it out. In
> my mind, it was just a hierarchical mechanism to allow one piece of data
> in one place to apply to multiple pieces of data somewhere else.
>
> It feels a lot like these file rules, from evince:
>
> owner /{,var/}run/user/*/dconf/ w,
> owner /{,var/}run/user/*/dconf/user rw,
>
> /**.[bB][mM][pP] rw,
> /**.[dD][jJ][vV][uU] rw,
> /**.[dD][vV][iI] rw,
> /**.[gG][iI][fF] rw,
> /**.[jJ][pP][gG] rw,
> ...
>
> Some of the duplication has been factored out of each set of rules, but
> not all of it, and in different ways. I think the same could be done
> with Proposal #4 -- a different amount of commonality factoring might
> be appropriate in some situations than in other situations.
>
>> /** r,
>> /tmp/** w,
>>
>> isn't totally conceptually different than:
>>
>> dbus receive {
>> bus=session
>> }
>> dbus send {
>> bus=session
>> peer name=org.freedesktop.Accounts
>> }
>>
>> but it isn't nearly as intuitive as a policy author where I tend to
>> look at the file path, capability, subj, peer, etc first, and then
>> consider the access, but Proposal #4 really turns this around in an
>> awkward way for me (note, I don't care that the access is first or
>> not-- I just object to nesting under the access). Currently, I am able
>> to look an audit message and write a corresponding dbus rule. This is
>> preserved in Proposal #3.
>
> I think this is also preserved in my proposal: if you do not want to do
> any factoring, you can simply mechanically transfer the audit message
> into the profile. The three rules given here were written without any
> factoring:
>
> dbus receive bus=session subj path=/org/gnome/ScreenSaver
> interface=org.freedesktop.DBus.Properties,
> dbus receive bus=session subj path=/org/gnome/ScreenSaver
> interface=org.gnome.ScreenSaver
> peer label=/usr/bin/gnome-settings-daemon,
> dbus receive bus=session subj path=/org/gnome/ScreenSaver
> interface=org.gnome.ScreenSaver
> peer name=com.canonical.indicator.session,
>
>
> The tricky part of this compared to the more regular #3 is that
> 'subject' and 'peer' influence the meaning of what's to come but with
> the (optional) braces enclosing the wrong things. To rewrite the three
> rules above with their optional braces, but still in three rules, it'd
> look like this:
>
> dbus receive bus=session subj { path=/org/gnome/ScreenSaver
> interface=org.freedesktop.DBus.Properties },
> dbus receive bus=session subj { path=/org/gnome/ScreenSaver
> interface=org.gnome.ScreenSaver
> peer { label=/usr/bin/gnome-settings-daemon } },
> dbus receive bus=session subj { path=/org/gnome/ScreenSaver
> interface=org.gnome.ScreenSaver
> peer { name=com.canonical.indicator.session } },
>
> (Newlines are fairly arbitrary here; the only real reasoning here was to
> put 'peer' on a new line on its own so it is easy to spot, but it isn't
> mandatory by any means.)
>
> Anyway, I admit that the braces are in the "wrong spots" for these
> mechanical transcriptions, with braces. You probably wouldn't write them
> for cases without factoring... and whether or not you could like them
> even with factoring is a good question. :)
>
> These rules could also be written in either of these ways, by putting
> 'peer' on the outside of the hierarchy:
>
> dbus receive bus=session subj path=/org/gnome/ScreenSaver
> interface=org.freedesktop.DBus.Properties,
> dbus receive bus=session peer label=/usr/bin/gnome-settings-daemon
> subj path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver,
> dbus receive bus=session peer name=com.canonical.indicator.session
> subj path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver,
>
> dbus receive bus=session subj { path=/org/gnome/ScreenSaver
> interface=org.freedesktop.DBus.Properties } ,
> dbus receive bus=session peer { label=/usr/bin/gnome-settings-daemon
> subj { path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver } },
> dbus receive bus=session peer { name=com.canonical.indicator.session
> subj { path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver } },
>
> Here's the profile, re-written to use less factoring. It'd be up to the
> profile author to decide what is most clear when writing the profile:
>
> /usr/bin/gnome-screensaver {
> # Ignore file and accessibility bus access for this excercise
> file,
> dbus bus=accessibility,
>
> # sarnold> I think we could remove this entirely; for any bus with
> # bus= rules later on, add this rule implicitly. We can do this with
> # any of the proposals..
> # Talks to system and session buses
> # dbus bus={system,session} peer=(name=org.freedesktop.DBus) (send receive),
>
> dbus send bus=system {
> peer name=org.freedesktop.ConsoleKit
> path=/org/freedesktop/ConsoleKit/Manager
> interface=org.freedesktop.ConsoleKit.Manager,
> peer name=org.freedesktop.Accounts
> path=/org/freedesktop/Accounts
> interface=org.freedesktop.Accounts,
> peer name=org.freedesktop.Accounts
> path=/org/freedesktop/Accounts/User*
> interface=org.freedesktop.DBus.Properties,
> }
>
> dbus send bus=session {
> peer name=org.gnome.SessionManager
> path=/org/gnome/SessionManager/Presence
> interface=org.freedesktop.DBus.Properties,
> peer path=/org/gtk/vfs/mounttracker
> interface=org.gtk.vfs.MountTracker,
> peer name=org.gnome.Shell
> path=/org/gnome/Shell
> interface=org.freedesktop.DBus.Properties,
> }
>
> # Receives messages on the session bus
> dbus acquire bus=session {
> subj name=org.gnome.ScreenSaver,
> }
>
> dbus receive bus=session {
> subj path=/org/gnome/ScreenSaver
> interface=org.freedesktop.DBus.Properties,
> subj path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver {
> peer label=/usr/bin/gnome-settings-daemon,
> peer name=com.canonical.indicator.session,
> }
> }
> }
>
> It feels less noisy without all the lines with only "}" on them, but
> there's more duplication, and to my eye, visual density there.
>
I admit this is better but personally, as a profile writer, the
hierarchical mechanism feels foreign and out of place. Eg, historically
I group different types of access together (eg, abstractions at the top,
capability rules next, network rules after that, followed by file rules
(dbus rules would likely be similarly grouped together). Then with
files, I group by path, so @{PROC} rules tend to be together, @{HOME}
rules together, /etc together, program paths together, and so on. With
abstractions and policy groups, we group similar things together-- X,
gnome, user-tmp, unity-hud, unity-menu, etc. I learned this from examing
existing policy from years ago. To me, this type of thing helps organize
the profile and helps with readability and auditability. At no point do
I group by access-- ie, the 'r's aren't all grouped together, the 'k's
all grouped together, etc. Proposal #4's hierarchical mechanism promotes
these (IMO unnatural) groupings for send, receive and acquire.
Also, this still allows for nesting peer under subj, which feels wrong
to me and the not-as-but-still-somewhat-deeply nesting makes this harder
to read and I imagine even more so with child and sibling profiles. I
feel like I should learn a new way to write rules as opposed to dbus/IPC
rules feeling like an extension of the language.
>>> So here's proposal #3 with linewraps where I think I'd want to put them to
>>> try to make them long-term legible. It's still shorter than my "factoring"
>>> approach above, but the density still makes my eyes glaze over a bit. :)
>> [...]
>> I think the multiline approach for Proposal #3 is not a bad idea, but
>> also not a blocker on pushing the single line approach now, and
>> bringing multiline later.
>
> The way the parser is written, we actually get both versions for free.
> Newlines mean very nearly nothing. (They serve as whitespace, and
> they cause line number counters to be bumped, but they aren't used for
> interpreting rules.) This would continue to be the case regardless of
> which propsal we select, simply because it is easier to write the parser
> to not care about newlines.
>
> So if you like #3 with newlines most of all, you get that the same day
> you get #3 written all on one line. :)
>
I thought this was the case or if it wasn't would be easy to fix. Glad
to hear we would get it for free. :)
>> Talking to John and Tyler today, I too am not opposed to using blocking
>> mechanisms instead which I think captures some of what Seth is going
>> for, and looks cleaner. Eg:
>>
>> dbus [<bus>] [subj { <subject> } ] [acquire],
>> dbus [<bus>] [subj { <subject> } ] [peer { <peer> }] [send | receive],
>>
>> /usr/bin/gnome-screensaver {
>> # Ignore file and accessibility bus access for this excercise
>> file,
>> dbus bus=accessibility,
>>
>> # Talks to system and session buses
>> dbus bus={system,session} peer { name=org.freedesktop.DBus } (send receive),
>>
>> # Sends messages on the system bus
>> dbus bus=system peer { name=org.freedesktop.ConsoleKit path=/org/freedesktop/ConsoleKit/Manager interface=org.freedesktop.ConsoleKit.Manager } send,
>> dbus bus=system peer { name=org.freedesktop.Accounts path=/org/freedesktop/Accounts interface=org.freedesktop.Accounts } send,
>> dbus bus=system peer { name=org.freedesktop.Accounts path=/org/freedesktop/Accounts/User* interface=org.freedesktop.DBus.Properties } send,
>>
>> # Receives messages on the session bus
>> dbus bus=session subj { name=org.gnome.ScreenSaver } acquire,
>> dbus bus=session subj { path=/org/gnome/ScreenSaver interface=org.freedesktop.DBus.Properties } receive,
>> # Be selective because the Lock method is mediated by these rules
>> dbus bus=session subj { path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver } peer { label=/usr/bin/gnome-settings-daemon } receive,
>> dbus bus=session subj { path=/org/gnome/ScreenSaver interface=org.gnome.ScreenSaver } peer { name=com.canonical.indicator.session } receive,
>>
>> # Sends messages on the session bus
>> dbus bus=session peer { name=org.gnome.SessionManager path=/org/gnome/SessionManager/Presence interface=org.freedesktop.DBus.Properties } send,
>> dbus bus=session peer { path=/org/gtk/vfs/mounttracker interface=org.gtk.vfs.MountTracker } send,
>> dbus bus=session peer { name=org.gnome.Shell path=/org/gnome/Shell interface=org.freedesktop.DBus.Properties } send,
>> }
>>
>> Personally, I still prefer Proposal #3 with the potential to
>> go multi-line in the future.
>
> I very very slightly prefer this over Proposal #3. They both contain a
> lot of duplication though.
>
Maybe I've profiled too many file rules, but I don't mind the
duplication. It is explicit, clear and easy to audit. I've always touted
a strength of AppArmor is how policy is relatively easy to audit. Plus,
with Proposal #4, I feel like I should learn a new way to write rules as
opposed to dbus/IPC rules feeling like an extension of the language.
IMO, the dbus/IPC rules should be about clarity, not compactness, and
Proposal #4 sacrifices too much of the former to achieve a bit of the
latter.
I strongly prefer Proposal #3 over #1, #2 and #4. My personal preference
is for 'peer=()' and 'subject=()' instead of 'peer {}' and 'subject {}',
but I could live with '{}'. I think I somewhat prefer the access at the
front (right after dbus), but am fine at the end as well.
--
Jamie Strandboge http://www.ubuntu.com/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130612/eaac8746/attachment-0001.pgp>
More information about the AppArmor
mailing list