Comment 4 for bug 1371574

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This is a long reply, but stick with me until the end-- it should be worth it. :)

This would complicate things a bit in the following ways (which may reinforce some of your points but listing them for the alternate perspective if nothing else). This bit I am most concerned about is that we have multiple profiles for a particular app version since each profile will have a different CLICK_DIR. Eg, from comment #2, cjwatson may end up with version 1.1 in /opt/click.ubuntu.com and everyone else 1.1 in /usr/share/click/preinstalled. This means:

1. we have extra profiles to compile which will take more time when compiles are required. On the one hand, we can expect this number to be relatively low, but I've heard custom tarballs having as many as 30 or more clicks. Compiling profiles takes time, and while there are some additional gains to be made, we are likely going to focus on precompiling profiles in the short-medium term (see below)

2. these extra profiles need to be loaded into the kernel. With the numbers we are talking about, we probably won't be over 3M for these extra profiles unless the device has an extreme number of preinstalled/custom clicks, but I thought the point worth making (profiles are typically (far) under 50K each). It also takes time to load profiles into the kernel, but most of this pain should be addressed by recent changes to profile loading (we can load ~40 cached profiles per second on mako, for example). With the numbers we are talking about, this shouldn't be more than 2 seconds.

3. precompiling profiles is affected, but not appreciably more complicated. Right now we precompile profiles for preinstalled apps and custom apps. Preinstalled apps have the cache files in the rootfs and custom in the custom tarball. Scripts and boot jobs would have to be updated for both, but there is a 1 to 1 mapping (ie, custom always deals with custom and preinstalled with rootfs) so this change should be easy enough

4. app launch can be complicated since it would have to figure out which profile to use. This could probably be done without startup impact

5. developers are familiar with current filenames and the APP_ID concept. Adding complexity based on db path could confuse existing users and increase the learning curve for new users.

6. Most importantly, the APP_ID concept becomes considerably more complicated. Right now, the APP_ID flows though click, apparmor, lifecycle, and trusted helpers. Click currently works with click-apparmor to generate profile names that are the APP_ID. AppArmor doesn't care about the filenames of the profiles, and uses the APP_ID as the profile name. App launch (UAL and aa-exec-click, the latter not used in unity8) uses the APP_ID to change profile (application lifecycle also keeps track of the APP_ID for tracking processes of course, though I don't think it looks at the label of the process to do this). Trusted helpers will look at the AppArmor profile name to make decisions (eg trust-store lookups, online accounts access, etc). Right now, the APP_ID is a thread that all of these disparate parts can trust. The current proposal would necessitate the profile name to change to differentiate between profiles with different CLICK_DIRs. For example, two profiles with the same profile name might have policy like this:
   /var/lib/apparmor/profiles/click_opt_click_foo:
     @{CLICK_DIR}="/opt/click.ubuntu.com"
     profile foo { ... }

   /var/lib/apparmor/profiles/click_usr_share_click_preinstalled_foo:
     @{CLICK_DIR}="/usr/share/click/preinstalled"
     profile foo { ... }
then the last one loaded wins. We would have to adjust the profile name to also include the click dir. Eg, policy is:
   /var/lib/apparmor/profiles/click_opt_click_foo
     @{CLICK_DIR}="/opt/click.ubuntu.com"

This solves AppArmor, but breaks the APP_ID concept because now application launch has to consider this, trusted helpers need to be aware (especially if the user upgrades from custom to store profile and the APP_ID changes)

There are probably some others I am forgetting about....

While I appreciate the desire to push some of this outside of click proper, I can't help but feeling that if we push beyond click hooks, we are getting things wrong. Ie, making click-apparmor do something different is one thing, but changing the concept of APP_ID or making things like application launch or trusted helpers aware of the differences in click databases is not what we want to do.

I had an alternate idea that I think will work well for your needs-- adjust CLICK_DIR to not specify a single directory, but instead an alternation of click directories. Ie, right now we have:

= profile for com.ubuntu.developer.jdstrand.foo_appname_0.1 =
  @{CLICK_DIR}="/opt/click.ubuntu.com"
  profile "com.ubuntu.developer.jdstrand.foo_appname_0.1" {
    ...
    @{CLICK_DIR}/@{APP_PKGNAME}/@{APP_VERSION}/** mrklix,
    ...
  }

= profile for com.ubuntu.developer.jdstrand.bar_appname_0.2 =
  @{CLICK_DIR}="/usr/share/click/preinstalled"
  profile "com.ubuntu.developer.jdstrand.bar_appname_0.2" {
    ...
    @{CLICK_DIR}/@{APP_PKGNAME}/@{APP_VERSION}/** mrklix,
    ...
  }

but we could instead do:
= profile for com.ubuntu.developer.jdstrand.foo_appname_0.1 =
  @{CLICK_DIR}="{/opt/click.ubuntu.com,/usr/share/click/preinstalled}"
  profile "com.ubuntu.developer.jdstrand.foo_appname_0.1" {
    ...
    @{CLICK_DIR}/@{APP_PKGNAME}/@{APP_VERSION}/** mrklix,
    ...
  }

= profile for com.ubuntu.developer.jdstrand.bar_appname_0.2 =
  @{CLICK_DIR}="{/opt/click.ubuntu.com,/usr/share/click/preinstalled}"
  profile "com.ubuntu.developer.jdstrand.bar_appname_0.2" {
    ...
    @{CLICK_DIR}/@{APP_PKGNAME}/@{APP_VERSION}/** mrklix,
    ...
  }

In this manner, we can keep the APP_ID concept and the only bits that need to change are click and click-apparmor. click-apparmor would be adjusted to not care about what the symlinks are pointing to, but instead make an API call (or perhaps hardcode in the short term) to get a list of click directories to shove into CLICK_DIR. To reduce changes in click-apparmor, we would want the symlinks in /var/lib/apparmor/clicks to only be one per profile (ie, the status quo). IIUC, making this one change to click-apparmor would actually address this bug, but it is of course click-apparmor specific and I'm not sure if you would want to still make all the changes to click you are proposing (we can discuss). I think it would allow us to kick the can for now.