[PATCH] apparmor: let image label setting loop over backing files
Peter Krempa
pkrempa at redhat.com
Tue Jan 19 10:43:40 UTC 2021
On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> When adding a rule for an image file and that image file has a chain
> of backing files then we need to add a rule for each of those files.
>
> To get that iterate over the backing file chain the same way as
> dac/selinux already do and add a label for each.
>
> Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> ---
> src/security/security_apparmor.c | 39 ++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index 29f0956d22..1f309c0c9f 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
>
> /* Called when hotplugging */
> static int
> -AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> - virDomainDefPtr def,
> - virStorageSourcePtr src,
> - virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED)
> +AppArmorSetSecurityImageLabelInternal(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virStorageSourcePtr src)
> {
> - virSecurityLabelDefPtr secdef;
> g_autofree char *vfioGroupDev = NULL;
> const char *path;
>
> - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> - if (!secdef || !secdef->relabel)
> - return 0;
> -
> - if (!secdef->imagelabel)
> - return 0;
> -
> if (src->type == VIR_STORAGE_TYPE_NVME) {
> const virStorageSourceNVMeDef *nvme = src->nvme;
>
> @@ -797,6 +788,30 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> return reload_profile(mgr, def, path, true);
> }
>
> +static int
> +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virStorageSourcePtr src,
> + virSecurityDomainImageLabelFlags flags G_GNUC_UNUSED)
> +{
> + virSecurityLabelDefPtr secdef;
> + virStorageSourcePtr n;
> +
> + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> + if (!secdef || !secdef->relabel)
> + return 0;
> +
> + if (!secdef->imagelabel)
> + return 0;
So apparmor doesn't support per-image security labels?
> +
> + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
> + if (AppArmorSetSecurityImageLabelInternal(mgr, def, n) < 0)
It feels a bit suboptimal to fork+exec the aahelper for every single
image. The selinux/dac drivers collect the list of things to do before
forking when we are in the transaction mode (or do just chown/selinux
labelling, which is trivial)
Given that this is usually on an expensive code path, it's probably okay
for now though.
Reviewed-by: Peter Krempa <pkrempa at redhat.com>
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static int
> AppArmorSecurityVerify(virSecurityManagerPtr mgr G_GNUC_UNUSED,
> virDomainDefPtr def)
> --
> 2.30.0
>
More information about the libvir-list
mailing list