Merge ~paelzer/ubuntu/+source/libvirt:fix-lp-1879325-metadata-GROOVY into ubuntu/+source/libvirt:ubuntu/groovy-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4068/+packages

WIP until we know from the reporter if this is enough.

[Christian Ehrhardt  (paelzer)]

Well, let me open up the MPs for pre-review - even though there might be more coming later ...

[Rafael David Tinoco (rafaeldtinoco)]

+1 as you said this is a pre-review. I'll do the full review whenever you need.

[Christian Ehrhardt  (paelzer)]

FYI updated along my upstream contribution ...
PPA updated as well

[Christian Ehrhardt  (paelzer)]

I Pushed an update to the upstream accepted version of the patch (no-op for Ubuntu as the change was for selinux).

Also Updated the PPA with the newest build.

Ready for final-review

[Christian Ehrhardt  (paelzer)]

Non case specific regression testing is a few thousand tests in and still working.
Things should be good, let me know if you spot any mistake in the MP, otherwise I'll sponsor later on today.

[Rafael David Tinoco (rafaeldtinoco)]

# CHECKLIST
----------------------------
 [.] changelog entry correct:
 [.] targeted to correct codename
 [.] version number is correct
 [.] update-maintainer has been run before
 ----
 [.] changes forwarded upstream/debian (if appropriate)
 [.] patches match what was proposed upstream
 ----
 [.] patches correctly included in debian/patches/series?
 [.] patches have correct DEP3 metadata
 ----
 [.] relying on PPA only for build check ?
 [.] if relying on PPA, did it install correctly ?
 ----
 [-] building it locally ?
 [-] if building locally, was source build good ?
 [-] if building locally, was binary build good ?
 ----
 [-] was autopkgtest tested ?
 ----
 [-] is this a SRU ?
 [-] if a SRU, does the public bug have a template ?
 [-] is this a bundle of fixes ?
 [.] is this a single fix ?
 ----
 [.] if single fix, was testcase provided ?
 [-] if single fix, and testcase provided, could I reproduce it ?
 [-] if single fix, and testcase provided, did it work ?
 ----
 [-] is this a MERGE ?
 [-] if MERGE, is there a public bug referred ?
 [-] if MERGE, does it add/remove existing packages ?
 [-] if MERGE, does it bump library SONAME ?
----------------------------
 [.] = ok | [x] = not ok | [?] = question | [!] = note | [-] = n/a
----------------------------
# comments:

all good here, +1.
----

review: Approve

[Christian Ehrhardt  (paelzer)]

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/libvirt
 * [new tag] upload/6.0.0-0ubuntu9 -> upload/6.0.0-0ubuntu9

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading libvirt_6.0.0-0ubuntu9.dsc: done.
  Uploading libvirt_6.0.0-0ubuntu9.debian.tar.xz: done.
  Uploading libvirt_6.0.0-0ubuntu9_source.buildinfo: done.
  Uploading libvirt_6.0.0-0ubuntu9_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index a551db0..78a41da 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+libvirt (6.0.0-0ubuntu9) groovy; urgency=medium
+
+  * d/p/ubuntu/lp-1879325-*: avoid issues with apparmor metadata labeling
+    (LP: #1879325)
+
+ -- Christian Ehrhardt <[e-mail]>  Wed, 20 May 2020 06:59:57 +0200
+
 libvirt (6.0.0-0ubuntu8) focal; urgency=medium
 
   * d/control, d/rules: Disable rbd and zfs on riscv64 where they are
diff --git a/debian/patches/series b/debian/patches/series
index 3f54079..c5989b2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -80,3 +80,5 @@ ubuntu/lp-1853200-cpu_map-Don-t-use-new-noTSX-models-for-host-model-CP.patch
 ubuntu/lp-1868528-util-virhostcpu-Fail-when-fetching-CPU-Stats-for-inv.patch
 ubuntu-aa/lp-1871354-apparmor-avoid-denials-on-libpmem-initialization.patch
 ubuntu/CVE-CVE-2020-10701-api-disallow-virDomainAgentSetResponseTimeout-on-rea.patch
+ubuntu/lp-1879325-Don-t-require-secdrivers-to-implement-.domainMoveIma.patch
+ubuntu/lp-1879325-security-don-t-fail-if-built-without-attr-support.patch
diff --git a/debian/patches/ubuntu/lp-1879325-Don-t-require-secdrivers-to-implement-.domainMoveIma.patch b/debian/patches/ubuntu/lp-1879325-Don-t-require-secdrivers-to-implement-.domainMoveIma.patch
new file mode 100644
index 0000000..a92307a
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1879325-Don-t-require-secdrivers-to-implement-.domainMoveIma.patch
@@ -0,0 +1,44 @@
+From cc8c297e473afd55e5d8e35e18345d8df176059d Mon Sep 17 00:00:00 2001
+From: Michal Privoznik <[e-mail]>
+Date: Mon, 18 May 2020 10:07:30 +0200
+Subject: [PATCH] Don't require secdrivers to implement
+ .domainMoveImageMetadata
+
+The AppArmor secdriver does not use labels to grant access to
+resources. Therefore, it doesn't use XATTRs and hence it lacks
+implementation of .domainMoveImageMetadata callback. This leads
+to a harmless but needless error message appearing in the logs:
+
+  virSecurityManagerMoveImageMetadata:476 : this function is not
+  supported by the connection driver: virSecurityManagerMoveImageMetadata
+
+Closes: https://gitlab.com/libvirt/libvirt/-/issues/25
+
+Signed-off-by: Michal Privoznik <[e-mail]>
+Reviewed-by: Erik Skultety <[e-mail]>
+
+Origin: upstream, https://libvirt.org/git/?p=libvirt.git;a=commit;h=cc8c297e473afd55e5d8e35e18345d
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1879325
+Last-Update: 2020-05-20
+
+---
+ src/security/security_manager.c | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+diff --git a/src/security/security_manager.c b/src/security/security_manager.c
+index 2dea294784..b1237d63b6 100644
+--- a/src/security/security_manager.c
++++ b/src/security/security_manager.c
+@@ -473,8 +473,7 @@ virSecurityManagerMoveImageMetadata(virSecurityManagerPtr mgr,
+         return ret;
+     }
+ 
+-    virReportUnsupportedError();
+-    return -1;
++    return 0;
+ }
+ 
+ 
+-- 
+2.26.0
+
diff --git a/debian/patches/ubuntu/lp-1879325-security-don-t-fail-if-built-without-attr-support.patch b/debian/patches/ubuntu/lp-1879325-security-don-t-fail-if-built-without-attr-support.patch
new file mode 100644
index 0000000..4f3b857
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1879325-security-don-t-fail-if-built-without-attr-support.patch
@@ -0,0 +1,77 @@
+From 55029d93150e33d70b02b6de2b899c05054c5d3a Mon Sep 17 00:00:00 2001
+From: Christian Ehrhardt <[e-mail]>
+Date: Tue, 26 May 2020 09:33:38 +0200
+Subject: [PATCH] security: don't fail if built without attr support
+
+If built without attr support removing any image will trigger
+ qemuBlockRemoveImageMetadata (the one that emits the warning)
+   -> qemuSecurityMoveImageMetadata
+     -> virSecurityManagerMoveImageMetadata
+       -> virSecurityDACMoveImageMetadata
+         -> virSecurityDACMoveImageMetadataHelper
+           -> virProcessRunInFork (spawns subprocess)
+             -> virSecurityMoveRememberedLabel
+
+In there due to !HAVE_LIBATTR virFileGetXAttrQuiet will return
+ENOSYS and from there the chain will error out.
+
+That is wrong and looks like:
+  libvirtd[6320]: internal error: child reported (status=125):
+  libvirtd[6320]: Unable to remove disk metadata on vm testguest from
+  /var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda)
+
+This change makes virSecurityDACMoveImageMetadataHelper and
+virSecuritySELinuxMoveImageMetadataHelper accept that
+error code gracefully and in that sense it is an extension of:
+5214b2f1a3f "security: Don't skip label restore on file systems lacking XATTRs"
+which does the same for other call chains into the virFile*XAttr functions.
+
+Signed-off-by: Christian Ehrhardt <[e-mail]>
+Reviewed-by: Michal Privoznik <[e-mail]>
+
+Origin: upstream, https://libvirt.org/git/?p=libvirt.git;a=commit;h=55029d93150e33d70b02b6de2b899c05054c5d3a
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1879325
+Last-Update: 2020-05-27
+
+---
+ src/security/security_dac.c     | 6 ++++++
+ src/security/security_selinux.c | 6 ++++++
+ 2 files changed, 12 insertions(+)
+
+diff --git a/src/security/security_dac.c b/src/security/security_dac.c
+index bdc2d7edf3..7b95a6f86d 100644
+--- a/src/security/security_dac.c
++++ b/src/security/security_dac.c
+@@ -1117,6 +1117,12 @@ virSecurityDACMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
+ 
+     ret = virSecurityMoveRememberedLabel(SECURITY_DAC_NAME, data->src, data->dst);
+     virSecurityManagerMetadataUnlock(data->mgr, &state);
++
++    if (ret == -2) {
++        /* Libvirt built without XATTRS */
++        ret = 0;
++    }
++
+     return ret;
+ }
+ 
+diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
+index 9a929debe1..7bb7c2b7b1 100644
+--- a/src/security/security_selinux.c
++++ b/src/security/security_selinux.c
+@@ -1975,6 +1975,12 @@ virSecuritySELinuxMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
+ 
+     ret = virSecurityMoveRememberedLabel(SECURITY_SELINUX_NAME, data->src, data->dst);
+     virSecurityManagerMetadataUnlock(data->mgr, &state);
++
++    if (ret == -2) {
++        /* Libvirt built without XATTRS */
++        ret = 0;
++    }
++
+     return ret;
+ }
+ 
+-- 
+2.26.0
+