Provide a way to Update AppArmor rules for click tests only once

[Omer Akram]

We use autopkgtest in ubuntu-system-tests project to run those tests on the touch device. While developing the tests, it becomes a bit of a challenge to run the tests as it takes a few moment to update AppArmor rules for click.

As part of profiling for ubuntu-system-tests, we found it takes more than a minute each time to update AppArmor rules. So we probably need some kind of parameter to adt-run to not delete the /var/cache/apparmor/click-ap.rules file after running tests.

[Omer Akram (om26er)]

Attached diff should be able to achieve just that, still need to test it though.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "autopkg_no_restore_apparmor.diff" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Omer Akram (om26er)]

Updated patch to optionally not use --force else apparmor click update takes a long time

[Martin Pitt (pitti)]

Summary from IRC discussion:

This isn't sufficient yet as it will still need to regenerate the profiles at the beginning, due to the changing /tmp/adt-run.XXXX paths. It's also a bit ugly as this is supposed to be an internal implementation detail which I wouldn't like to expose for eternity in a command line option.

Alternative proposal:

 (1) apparmor_click() should become a no-op if /var/cache/apparmor/click-ap.rules already exists (http://paste.ubuntu.com/15335183/)
 (2) apparmor_restore_click() is already a no-op if apparmor_click() didn't do anything (no change needed)
 (3) add a setup script which sets a blanket /tmp/adt-run.*/** in the AppArmor policy, so that it works for any run

This would keep the current "correct, but slow" behavior, avoids exposing the internals as CLI args, but if you choose to use the setup script once (either manually after you (re)install the phone, or via adt-run --setup-commands) then adt-run will not touch the apparmor profiles at all, and things should be fast.

The script should look like this:

---------- 8< -----------------
cat <<EOF > /var/cache/apparmor/click-ap.rules
dbus (receive, send) bus=session path=/com/canonical/Autopilot/**,
/tmp/adt-run.** r,
---------- 8< -----------------

Omer, could you test the above patch and that setup script and make sure it DTRT?

[Omer Akram (om26er)]

I have attached a script based on your proposal.

I am not sure though if we can consider this the solution. The problem is that we want to run this script after the debs are installed either to the temp location or system location.

Need some more brainstorming ;)

[Martin Pitt (pitti)]

I'm confused, what does this have to do with deb installation? The apparmor rule hackery never affected "real" system installation of debs, and the blanket /tmp/adt-run.** pattern should apply to all subsequent temp location installs.

These apparmor rules should apply to click packages (only), not to debs. So do you mean that with this approach /var/cache/apparmor/click-ap.rules does not apply to clicks which are installed *after* creating that file? That indeed would sound like a bug in Apparmor itself as surely a newly installed click package should cause its own profile to be generated on installation?

Can you please be more specific and detailled what the remaining problem is after calling apparmor-rules-update-lite? (Btw, this can be dramatically simplified, but that can happen later).

[Omer Akram (om26er)]

So I tried my patch and made relevant changes to our test suite (lp:ubuntu-system-tests) and it does seem to be working fine.

My previous comment came as I had inspecting that aa-clickhook was called after temporary debs were installed in /tmp/adt-run.XXXX directory. and this code[1], which added each directory path to the click-ap.rules file. Since the above patch is working, we can blame my understand of aa-clickhook here to be the issue.

Do you have comments about the diff ?

[1] http://anonscm.debian.org/cgit/autopkgtest/autopkgtest.git/tree/lib/adt_testbed.py#n786

[Martin Pitt (pitti)]

> My previous comment came as I had inspecting that aa-clickhook was called after temporary debs were installed in /tmp/adt-run.XXXX directory.

Right, that's necessary as otherwise we don't know the precise file paths, and the on-demand added AA profile tries minimize extra privileges. But the blanket "/tmp/adt-run.**" rule allows all of them. Thus it's less safe/robust, but can be applied once right after installing the phone, and then never again.

I committed the adt_testbed.py logic update, which is certainly the more urgent part:
http://anonscm.debian.org/cgit/autopkgtest/autopkgtest.git/commit/?id=3a596b638

The bit I don't understand about your apparmor-rules-update-lite script is why you'd want to call aa-clickhook at all if the file already exists -- that should be a no-op then (without --force)?

My initial idea was to have this literal script:

---------- 8< -----------------
cat <<EOF > /var/cache/apparmor/click-ap.rules
dbus (receive, send) bus=session path=/com/canonical/Autopilot/**,
/tmp/adt-run.** r,
EOF
aa-clickhook --force --include=/var/cache/apparmor/click-ap.rules
---------- 8< -----------------

You could run this script after you flash the phone, or the first time you run a test on that device, and then never again. If you always want to specify it in --setup-commands, it could also exit right away if /var/cache/apparmor/click-ap.rules already exists.

I might still not understand something here, of course.

[Launchpad Janitor (janitor)]

This bug was fixed in the package autopkgtest - 3.20.1

---------------
autopkgtest (3.20.1) unstable; urgency=medium

  * When testing click packages, don't regenerate all AppArmor profiles if
    /var/cache/apparmor/click-ap.rules already exists. That way the profiles
    can be pre-adjusted once in a testbed instead of once for each test run,
    which greatly speeds up iterations. (LP: #1553797)
  * Move SshRunner* tests from lxc to lxd.
  * NullRunner.test_tree_output_dir test: Only check for cpu_{model,flags} on
    x86 and ARM, as these need parsing adjustments on other architectures.
  * Disable lxd autopkgtest for now, this still needs some way to set a proxy.
  * lib/VirtSubproc.py, cmd_reboot(): Add workaround for dhclient hanging on
    reboot (see LP #1556175).
  * Fix regular expression for removing profile guarded dependencies on hosts
    that don't support build profiles.
  * adt-virt-lxc: Suppress lxc-copy's stdout in the "no btrfs" fallback case.
    (Closes: #818185)

 -- Martin Pitt <email address hidden> Mon, 14 Mar 2016 22:35:54 +0100