AppArmor always denies pivot_root when mediation rules contain put_old or new_root

[Tyler Hicks]

After stgraber reported unexpected AppArmor denials when lxc was trying to pivot_root(), I wrote some tests for AppArmor's regression test suite.

The pivot_root syntax looks like this:

  [audit] [deny] pivot_root [oldroot=put_old] [new_root] [-> new_profile],

If [oldroot=put_old] or [new_root] are specified, AppArmor always denies the pivot_root(). I've verified this to be the case in Trusty and 12.04 LTS.

[Tyler Hicks (tyhicks)]

Here's the patch to the AppArmor regression tests that add fairly comprehensive tests for pivot_root mediation. What is missing from the patch is testing audit and deny modifiers on pivot_root rules. I'll add those before I upstream the patch.

[Tyler Hicks (tyhicks)]

Here are the results of the pivot_root.sh test on amd64 Trusty:

$ cat /proc/version_signature
Ubuntu 3.13.0-23.45-generic 3.13.8
$ make USE_SYSTEM=1 pivot_root && sudo VERBOSE=1 bash pivot_root.sh
ok: PIVOT_ROOT (unconfined)
ok: PIVOT_ROOT (unconfined, bad context)
ok: PIVOT_ROOT (no perms)
ok: PIVOT_ROOT (cap only)
ok: PIVOT_ROOT (bare rule, no cap)
ok: PIVOT_ROOT (bare rule)
Error: pivot_root failed. Test 'PIVOT_ROOT (new_root)' was expected to 'pass'. Reason for failure 'FAIL - pivot_root: Permission denied'
ok: PIVOT_ROOT (bad new_root)
Error: pivot_root failed. Test 'PIVOT_ROOT (put_old)' was expected to 'pass'. Reason for failure 'FAIL - pivot_root: Permission denied'
ok: PIVOT_ROOT (bad put_old)
Error: pivot_root failed. Test 'PIVOT_ROOT (put_old, new_root)' was expected to 'pass'. Reason for failure 'FAIL - pivot_root: Permission denied'
ok: PIVOT_ROOT (bad put_old, new_root)
ok: PIVOT_ROOT (put_old, bad new_root)
ok: PIVOT_ROOT (transition)
ok: PIVOT_ROOT (transition, no perms)
ok: PIVOT_ROOT (bad transition)
ok: PIVOT_ROOT (bad transition comparison)
Error: pivot_root failed. Test 'PIVOT_ROOT (new_root, transition)' was expected to 'pass'. Reason for failure 'FAIL - pivot_root: Permission denied'
ok: PIVOT_ROOT (new_root, bad transition)
Error: pivot_root failed. Test 'PIVOT_ROOT (put_old, new_root, transition)' was expected to 'pass'. Reason for failure 'FAIL - pivot_root: Permission denied'
ok: PIVOT_ROOT (put_old, new_root, bad transition)

[John Johansen (jjohansen)]

So having gone through this, pivot_root is working as expected and the test suite does pass with some minor alterations to fix a few problems with the profiles it was generating.

There is a bug in how the parser is handling the rules, and I am working on a patch for that. Until that patch lands please be very careful with '/' and variable expansion and remember the roots being pivoted in are directories and must end with '/'

eg.
   pivot_root old_root=/foo /bar,

will fail because neither /foo nor /bar are directories, the correct rule is
  pivot_root old_root/foo/ /bar/,

Check variable expansions as they can result in invalid paths that are not properly handled atm.
  @{foo} = /foo/
  pivot_root @{foo}/bar/,

this is resulting in the post variable expansion rule that looks like
  pivot_root /foo//bar/,

which will always fail to match

[Tyler Hicks (tyhicks)]

John, what do you think about the parser erroring out on pivot_root rules with non-glob'ed paths that are missing the trailing slash? I don't know how easy that is to implement in the parser but it would provide immediate feedback to policy writers when they make the mistake of writing such a rule.

[John Johansen (jjohansen)]

I think its a good idea, it shouldn't be too hard to do (worst case would be a trailing alternation), and would help with mount rules too. I think as a first we could just allow trailing alternations, which should keep the patch to a few lines of code.

[Jamie Strandboge (jdstrand)]

In either case, I think it would be worth updating the man page for the potential gotcha. pivot_root is only barely mentioned in the man page and other examples with mount leave off the trailing slash.

[Jamie Strandboge (jdstrand)]

Ok, I see '[PATCH] parser: Document pivot_root in the apparmor.d(5) man page' has the documentation patch, but it doesn't say anything special about ending with a slash or variable expansion. It does mention directories, but doesn't reiterate that directories must end with a '/'.

[Tyler Hicks (tyhicks)]

I've sent another patch out against the apparmor.d man page to mention that trailing slashes are required.

I'm going to mark this bug as invalid, as pivot_root works as expected. I'm also going to create a new bug suggesting that the parser reject pivot_root rules with non-directory arguments.

There's also the bug that John discovered about variable expansion. This seems like something more general that affects all rule types. John, could you open a bug on that?