On Wed, Nov 18, 2020 at 10:38 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Tue, Nov 17, 2020 at 09:11:48PM -0500, Neal Gompa wrote:
On Tue, Nov 17, 2020 at 11:49 AM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote:
On Mon, Nov 16, 2020 at 3:28 PM Michal Privoznik <mprivozn@redhat.com> wrote:
On 11/16/20 1:26 PM, Christian Ehrhardt wrote:
'kvm-spice' is a binary name used to call 'kvm' which actually is a wrapper around qemu-system-x86_64 enabling kvm acceleration. This isn't in use for quite a while anymore, but required to work for compatibility e.g. when migrating in old guests.
For years this was a symlink kvm-spice->kvm and therefore covered apparmor-wise by the existing entry: /usr/bin/kvm rmix, But due to a recent change [1] in qemu packaging this now is no symlink, but a wrapper on its own and therefore needs an own entry that allows it to be executed.
[1]: https://salsa.debian.org/qemu-team/qemu/-/commit/9944836d3
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thank you Michal, it also passed fine through my tests (as backport to 6.8 and 6.9). We are not in any freeze, review has happened, tests LGTM - pushed to git.
Hold up, why was this merged? Did anyone validate whether this would break the other AppArmor user (SUSE)?
Unlike SELinux, AppArmor functionality is quite fragmented between Ubuntu and SUSE distributions (the two major users of AppArmor), and there did not seem to be any indication that this AppArmor patch was validated with openSUSE before merging. My personal experience with AppArmor across the two distribution families is that it's really easy to make profiles that work for Ubuntu but fail on SUSE because of the disparity of functionality. I also don't see Jim Fehlig stepping in to indicate that this worked for him.
I haven't had a chance to test this myself, but I am immediately suspicious of a change that references a commit based on Debian packaging of QEMU.
Historically the AppArmor policy in libvirt has been exclusively maintained and tested by the Debian and Ubuntu maintainers. We have never considered SUSE in any changes made to it.
Ack to what Daniel wrote. In addition neither are other - be it Suse or 3rd party - changes gated on Debian/Ubuntu testing them. If I fail to catch the changes on the ML-discussion as part of staring at my inbox, then the testing for us happens whenever we merge a new upstream version. The general rule of thumb that we not-strictly followed in recent times aret: - logical changes e.g. to virt-aa-helper will have a build time self-test associated - labelling changes (related to hot add for example) are usually up for discussion a bit longer and tested by the author in the context that the issues were found - rule allow-additions (like the one here) are discussed and added if there are no security concerns I don't remember we've made anything more restrictive recently, that would probably be somewhere between the latter two points above. The duration also depends on the complexity - in this particular case as Michal already stated there isn't a high chance of breaking something with it. OTOH I'm fine to work out something more official/established if you want to define something - but keep in mind that many of us do this as a fraction of a part of their duties. Due to that sometimes human/machine time isn't available for short round trip times which are needed for a formal gating process.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd