Comment 21 for bug 1719579

------- Comment From <email address hidden> 2018-02-27 08:24 EDT-------
(In reply to comment #61)
> Thanks for the full dmesg.
> It seems to me that:
> "unable to set AppArmor profile
> 'libvirt-81b387d9-1dfc-4f55-8b98-0318f1f94442'"
> means there is an issue in loading the profile after your change.
>
> That matches:
> audit: type=1400 audit(1519028363.683:12417): apparmor="DENIED"
> operation="change_profile" info="label not found" error=-2
> profile="/usr/sbin/libvirtd"
> name="libvirt-81b387d9-1dfc-4f55-8b98-0318f1f94442" pid=12949 comm="libvirtd"
>
> It is not getting to the actual restore, it is failing when spawning the
> guest to to the changes in the apparmor profile.
>
> I tried to check what you hit:
> $ virsh save bionic-test --file /var/tmp/bionic-test.save --verbose
> Guest is shut-off and I have
> -rw------- 1 root root 527808329 Feb 19 12:34 /var/tmp/bionic-test.save
> The restore hits the (silent) denial we discussed.
> #deny /tmp/{,**} r,
> #deny /var/tmp/{,**} r,
> Changed the two lines above to a comment.
> Then restored again, just worked:
> $ virsh restore /var/tmp/bionic-test.save
> Domain restored from /var/tmp/bionic-test.save
>
> To quote jdstrand from bug 1403648:
> "We should not allow access to /tmp and /var/tmp as that breaks application
> isolation."
>
> That said we are in the following situation:
> 1. /tmp and /var/tmp are not allowed to be read (apparmor default for app
> isolation)
> 2. read denies there are silenced via explicit denies in
> /etc/apparmor.d/abstractions/libvirt-qemu
> 3. I see your point:
> 3.1 on save libvirt writes to that place (libvirt is allowed to do so, while
> qemu is not)
> 3.2 on restore qemu wants to read it and is denied.
>
> And you wonder about the asymetric behavior of 3.1 and 3.2.
> I agree that it is somewhat unexpected, but wonder what would be better
> 1. We could also deny /var /tmp for the lbivirt daemon (which intentionally
> has a rather lenient apparmor profile). Then already on the save people
> would be denied, maybe for a new release - but not as an SRU to not break
> people relying on that access working.

Okay, Agreed.

> 2. And on the new release we already have the --bypass-cache fixes you
> referred to to get the restore working there as a workaround - so the
> benefit of preventing libvirt to access there isn't too big either. So
> forbidding the access on "save" for libvirt there would make that useless.

Anyway, when restore is denied in turn it would make save as useless is my point here.
let's document it in man page about --bypass-cache would help

>
> I'm unsure how to continue. To better brain-storm with you on how to proceed
> do you have a clear preferred solution (other than the already included
> bypass-cache fixes) or is it just "not nice in general" that the denial
> should be consistent for save/restore?

if it is possible to error out or warn in Libvirt when performing save in denial paths that this
would fail on restore by apparmor, then it would be a proper way.
>
> Separate to the discussion above:
> To find how your modified apparmor profile breaks your guest start you could
> share it - as I mentioned it worked for me right away (no need to restart
> libvirt after changing btw, the one we change it loaded on guest load).

Thank you for detailed explanation :+1: