-
Notifications
You must be signed in to change notification settings - Fork 929
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
Instance: Relax apparmor QEMU proc rules a bit to workaround bug #13673
Conversation
…in AppArmor For some reason (bug in AppArmor?): owner @{PROC}/@{pid}/cpuset r, owner @{PROC}/@{pid}/task/@{tid}/comm rw, rules don't work properly and forbid perfectly legal access of Qemu to proc: [13830.493684] audit: type=1400 audit(1719481742.274:388): apparmor="DENIED" operation="open" class="file" profile="lxd-v1_</var/snap/lxd/common/lxd>" name="/proc/94213/task/94293/comm" pid=94213 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=999 ouid=0 I've noticed, that removing the "owner" keyword makes it work. Let's do this, despite it relaxes profile and makes things less secure, I can't really see any serious security impact from that change. This only reproducible when core24 is used (new AppArmor) and ceph volume is attached to the VM. Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
@@ -41,8 +41,8 @@ profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) { | |||
/{,usr/}bin/qemu-system-* mrix, | |||
/usr/share/qemu/** kr, | |||
/usr/share/seabios/** kr, | |||
owner @{PROC}/@{pid}/cpuset r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does removing the owner
part mean (in theory)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to the AppArmor man:
owner
Specifies that the task must have the same euid/fsuid as the object being referenced
by the permission check.
So, this rule was applied only to the files which are owned by the user who runs Qemu, but after removing it we allow Qemu to access any other files too. To be more precise, AppArmor won't disallow access to other files, at the same time generic Linux permission checks are still applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is @{PROC}/@{pid}/cpuset
normally owned by the same user as qemu is run as? But for some reason apparmor is deny access?
Is PROC in this case the /proc
?
So we are only allowing it access to files inside its own pid anyway?
This seems fine, there are a couple of other uses of @{PROC}/@{pid}/cpuset
without owner
in LXD already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is @{PROC}/@{pid}/cpuset normally owned by the same user as qemu is run as? But for some reason apparmor is deny access?
Absolutely.
Is PROC in this case the /proc?
Yes.
So we are only allowing it access to files inside its own pid anyway?
with owner
- yes, without owner
we allow access to the files for other pids too. But, of course, standard Linux permission checks are still applied.
That's not a bug in Apparmor. The |
So its a bug fix :) |
@simondeziel you are right, owner of That's not AppArmor bug, likely, bug was fixed in AppArmor which leads to this behavior change. But this PR is still valid. |
For some reason (bug in AppArmor?):
owner @{PROC}/@{pid}/cpuset r,
owner @{PROC}/@{pid}/task/@{tid}/comm rw,
rules don't work properly and forbid perfectly legal access of Qemu to proc:
[13830.493684] audit: type=1400 audit(1719481742.274:388): apparmor="DENIED" operation="open" class="file" profile="lxd-v1_</var/snap/lxd/common/lxd>" name="/proc/94213/task/94293/comm" pid=94213 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=999 ouid=0
I've noticed, that removing the "owner" keyword makes it work. Let's do this, despite it relaxes profile and makes things less secure, I can't really see any serious security impact from that change.
This only reproducible when core24 is used (new AppArmor) and ceph volume is attached to the VM.