[apparmor] DBus rule syntax for subject and peer components

[John Johansen]

On 06/11/2013 06:46 PM, Seth Arnold wrote:
> On Mon, Jun 10, 2013 at 06:44:13PM -0700, Tyler Hicks wrote:
>> To start us off, here's the profile using the current DBus syntax. It is
>> complex, but it uses all of the DBus accesses (send, receive, and
>> acquire) and it is representative of what a real profile may look like.
> 
> Thanks for this. These are all less readable than I'd expected, and as
> you pointed out, when we used com.example.foo it was too easy to overlook
> the complexity we've got here.
> 
> So, here's a first shot at Proposal #4:
> 
> I'm having trouble coming up with a BNF-style description of my newest
> proposal, which doesn't bode well for implementing it, but I hope
> factoring out common pieces of policy that "float up" makes some sense.
> 
> First, move the permission next to the 'dbus' keyword; this makes it
> easier to spot, groups like-permissions, and allows bus= handling to be
> very similar to the other keywords. Next, more brackets! If a portion of
> a rule can be 'factored' out of several rules, pull it 'up', and push the
> specifics 'down'.
> 
> Here's the example gnome-screensaver; I've converted the original 16
> lines into 50 lines, so it isn't exactly space-saving. There's no reason
> for the newlines 'within' a block except to keep it from going longer
> than 80 chars, so it could be much shorter, but I think this is how I
> would want to write the rules.
> 
> /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 think the language I've proposed could be used in either compressed or
> expanded modes. The last block could also be written 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,
> 
> But, the fact that describing this thing in a BNF is difficult might
> make implementing it too expensive.
> 

so I would say I am not opposed to this and it mirrors some blocking
extensions that where proposed in the past. That is

  audit {
    /foo rw,
  }


  audit {

     deny {
         /etc/* w,
	 /bar   w,
     }

     /foo rw,
     /blah r,
  }

it has a certain logical sense to it, and I can write BNF for it, though
it is more difficult. The real issue is that this will be hard to do
with the parser in its current state.

I have plans to finish refactoring the parser making the block context
available to rules but I haven't found time to do it yet.

However I am not terribly found of the embedding of the peer block in
the subject block as you have done in your last example.


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

Yes density is a bit of a problem here, but linewraps do help. Its also
possible to take a hybrid approach of just using block on subj and
peer right now (basically replace subj=() with subj { }) and then in
the future allow for finer grained blocks.

> 
> /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,
> }
> 
> 
> So, does anyone else like my factoring approach? are there cleaner /
> nicer ways to perform the factoring?
> 

My only issue with the factoring/blocking approach is that I am not
fond of the peer part of the rule inside the subject block, but I
do understand why you do it.

The use of { } was proposed in the dbus/pair address rule encoding thread.
though only to the first level, that is for subj { } , and peer { }

I will admit to saying I wasn't a fan of just having the keyword next to
the { } block, and preferred subj={ } but thinking about it more, if
we allow multi level factoring/blocking and if we want to allow
audit { ... } and other block types then it makes sense.

Also I am not too fond of subj=(name=a.b.c), it feels funny. And if
we have another form of ipc that allows for a multi-valued set then
  subj=(foo=(a b) bar=c)

is do able but even uglier.