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

AppArmor: allow more read-only bind remounts #12698

Closed
wants to merge 1 commit into from
Closed

Conversation

enr0n
Copy link

@enr0n enr0n commented Jan 4, 2024

There is already a set of rules to allow mounts with

options=(ro,remount,bind,nosuid,noexec,nodev).

Expand on this slightly by also allowing mounts with

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

Without this change, systemd hits an AppArmor denial when attempting to setup credentials for a service[1].

[1] https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/2046486

Note: The version of apparmor_parser in the core22 snap does not understand the nosymfollow mount option. E.g., if I try to expand the policy manually:

$ lxc config set systemd-lxc raw.apparmor "mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/shm,"
Error: Parse AppArmor profile: Failed to run: apparmor_parser -QWL /var/snap/lxd/common/lxd/security/apparmor/cache /var/snap/lxd/common/lxd/security/apparmor/profiles/lxd-systemd-lxc: exit status 1 (unsupported mount options)

So I guess this couldn't actually be included until the lxd snap is based on core24?

There is already a set of rules to allow mounts with

  options=(ro,remount,bind,nosuid,noexec,nodev).

Expand on this slightly by also allowing mounts with

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

Without this change, systemd hits an AppArmor denial when attempting to
setup credentials for a service[1].

[1] https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/2046486
@enr0n enr0n requested a review from tomponline as a code owner January 4, 2024 19:46
@tomponline
Copy link
Member

@enr0n I am working on updating the LXD snap to use core24 now actually, although we wont be able to merge this until core24 is out of edge.

@tomponline
Copy link
Member

@enr0n @mihalicyn this also looks like its going to play havoc with our tests which are based on ubuntu 22.04.
I wonder if there is a way to use a newer apparmor parser in 22.04.

@tomponline
Copy link
Member

@enr0n is there any value in landing an earlier version of this without the nosymfollow option before we switch to core24?

@enr0n
Copy link
Author

enr0n commented Jan 17, 2024

@enr0n is there any value in landing an earlier version of this without the nosymfollow option before we switch to core24?

No, there is already a rule for this set of options minus nosymfollow.

@tomponline tomponline changed the title lxd/apparmor: allow more read-only bind remounts AppArmor: allow more read-only bind remounts Feb 8, 2024
@enr0n
Copy link
Author

enr0n commented May 10, 2024

Gentle ping. There are more services in systemd utilizing credentials (namely systemd-networkd and systemd-journald) in v256, so this issue will become more apparent.

@tomponline
Copy link
Member

Indeed. We need to switch to core24 for this to work, we have this on our items for this cycle.

@mihalicyn
Copy link
Member

It is interesting. If apparmor_parser doesn't know anything about nosymfollow why kernel still applies policies on this? It would make more sense to allow everything which is not supported by the apparmor_parser by default. It makes things not to break for older user space version.

So, if user updates apparmor_parser version to that which supports nosymfollow then we can apply this policy to profiles generated by this version of apparmor_parser. But if user uses old versions of apparmor_parser with a recent kernel (which supports nosymfollow) there is no reason to forbid it.

Kernel updates must be fully compatible with old user space versions. Always.

@mihalicyn
Copy link
Member

This canonical/lxd-pkg-snap#477 unblocks this PR. But we need to properly check AppArmor version from the LXD side.

@mihalicyn
Copy link
Member

Slightly reworked version
#13681

mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jun 28, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jun 28, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jun 28, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jun 28, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@tomponline
Copy link
Member

@enr0n hi please can you review #13681 and let us know a use case for testing the fix works. Thanks

@tomponline tomponline closed this Jul 1, 2024
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jul 1, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@tomponline
Copy link
Member

See #13681 (comment) for reproducer steps

tomponline pushed a commit to tomponline/lxd that referenced this pull request Jul 22, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@tomponline tomponline reopened this Jul 22, 2024
@tomponline tomponline closed this Jul 22, 2024
hamistao pushed a commit to hamistao/lxd that referenced this pull request Jul 23, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jul 25, 2024
mihalicyn added a commit to mihalicyn/lxd that referenced this pull request Jul 25, 2024
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]>
tomponline added a commit that referenced this pull request Jul 25, 2024
It turns out, that a ruleset:
```
{{- if .feature_mount_nosymfollow }}
  # see #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
tomponline pushed a commit to tomponline/lxd that referenced this pull request Aug 2, 2024
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]>
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
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]>
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
(cherry picked from commit b5a4a65)
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
(cherry picked from commit b5a4a65)
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
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]>
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
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]>
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
(cherry picked from commit b5a4a65)
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
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]>
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
(cherry picked from commit b5a4a65)
tomponline pushed a commit to tomponline/lxd that referenced this pull request Sep 13, 2024
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]>
(cherry picked from commit 86c3d51)
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.

3 participants