Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container: Allow apparmor nosymfollow mount flag in more cases #13820

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

mihalicyn
Copy link
Member

@mihalicyn mihalicyn commented Jul 25, 2024

It turns out, that a ruleset:

{{- if .feature_mount_nosymfollow }}
  # see https://github.com/canonical/lxd/pull/12698
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /[^spd]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /d[^e]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /de[^v]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.[^l]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.l[^x]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lx[^c]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lxc?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/[^.]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /p[^r]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pr[^o]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pro[^c]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /proc?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /s[^y]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sy[^s]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sys?*{,/**},
{{- end }}

is not enough to allow nosymfollow. We still getting AppArmor denials like this:

[110841.647871] audit: type=1400 audit(1721910063.197:1611): apparmor="DENIED" operation="mount" class="mount" info="failed flags match" error=-13 profile="lxd-secure-oriole_</var/snap/lxd/common/lxd>" name="/dev/shm/" pid=712867 comm="(sd-mkdcreds)" flags="ro, nosuid, nodev, noexec, remount, bind"

First of all, there is no "nosymfollow" in the kernel log. Which is a bug and should be fixed by: https://lore.kernel.org/all/[email protected]/

Secondly, it looks like these rules in the form of mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /some/path,

just does not work at all. At least in AppArmor 4.0+ (have not yet tested with older ones).

During my local experiments, I found that working variant of it might be: mount

options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) -> /some/path,

or wider:

mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow),

Let's just add a wider variant of the rule in addition to what we already have for unprivileged containers. But keep in mind that something is wrong with these rules in their more restrictive form (with path specifier). This is a matter of a futher investigation, because it's important for privileged containers case.

See also:
#12698

Closes #12698
May close #13810

It turns out, that a ruleset:
{{- if .feature_mount_nosymfollow }}
  # see canonical#12698
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /[^spd]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /d[^e]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /de[^v]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.[^l]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.l[^x]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lx[^c]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lxc?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/[^.]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /p[^r]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pr[^o]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pro[^c]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /proc?*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /s[^y]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sy[^s]*{,/**},
  mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sys?*{,/**},
{{- end }}

is not enough to allow nosymfollow. We still getting AppArmor denials like this:
[110841.647871] audit: type=1400 audit(1721910063.197:1611): apparmor="DENIED" operation="mount"
class="mount" info="failed flags match" error=-13 profile="lxd-secure-oriole_</var/snap/lxd/common/lxd>" name="/dev/shm/"
pid=712867 comm="(sd-mkdcreds)" flags="ro, nosuid, nodev, noexec, remount, bind"

First of all, there is no "nosymfollow" in the kernel log. Which is a bug and should be fixed by:
https://lore.kernel.org/all/[email protected]/

Secondly, it looks like these rules in the form of
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /some/path,

just does not work at all. At least in AppArmor 4.0+ (have not yet tested with older ones).

During my local experiments, I found that working variant of it might be:
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) -> /some/path,

or wider:
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow),

Let's just add a wider variant of the rule in addition to what we already
have for unprivileged containers. But keep in mind that something is wrong with
these rules in their more restrictive form (with path specifier). This is a matter
of a futher investigation, because it's important for privileged containers case.

See also:
canonical#12698

Closes canonical#12698
May close canonical#13810

Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn mihalicyn requested a review from tomponline July 25, 2024 12:33
@mihalicyn
Copy link
Member Author

mihalicyn commented Jul 25, 2024

cc @jrjohansen @gegarcia

@tomponline
Copy link
Member

But keep in mind that something is wrong with these rules in their more restrictive form (with path specifier). This is a matter of a futher investigation, because it's important for privileged containers case.

Does this PR relax the rules for privileged containers also?

@tomponline
Copy link
Member

@mihalicyn does this fix Oracular containers?

@mihalicyn
Copy link
Member Author

Does this PR relax the rules for privileged containers also?

No-no, only for unprivileged case for now, because we already have analogical rules.

For privileged case it's a matter of further investigation.

@mihalicyn
Copy link
Member Author

mihalicyn commented Jul 25, 2024

@mihalicyn does this fix Oracular containers?

yes. It fixes Oracular unprivileged containers.

@tomponline
Copy link
Member

Awesome thanks!

@tomponline tomponline changed the title lxd/apparmor/instance_lxc: allow nosymfollow mount flag in more cases Container: Allow apparmor nosymfollow mount flag in more cases Jul 25, 2024
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@tomponline tomponline merged commit e32e1a4 into canonical:main Jul 25, 2024
28 of 29 checks passed
tomponline added a commit to tomponline/lxd-pkg-snap that referenced this pull request Aug 1, 2024
tomponline added a commit to tomponline/lxd-pkg-snap that referenced this pull request Aug 20, 2024
  - Fix for Oracular unprivileged containers from canonical/lxd#13820
  - Fix snapshot importing from canonical/lxd#13899
  - Fix Dell Powerflex migrations from canonical/lxd#13934

Signed-off-by: Thomas Parrott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Container issues with Ubuntu Oracular systemd version 256-1ubuntu1
2 participants