[apparmor] DBus rule syntax for subject and peer components

[Seth Arnold]

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.

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.

> > 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. :)

> 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.

Thanks Jamie
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130612/f0a031a4/attachment-0001.pgp>