8:57 p.m.
I finally got around to investigating some apparmor-related test failures.
The first patch is apparmor specific. The second patch fixes a bug that
should affect all security drivers.
Jim Fehlig (2):
apparmor: Allow lxc processes to receive signals from libvirt
security: Avoid calling virSecurityManagerCheckModel with NULL model
src/security/apparmor/libvirt-lxc | 4 ++++
src/security/security_manager.c | 3 +++
2 files changed, 7 insertions(+)
--
2.29.2
8:57 p.m.
New subject: [PATCH 1/2] apparmor: Allow lxc processes to receive signals from libvirt
LXC processes confined by apparmor are not permitted to receive signals
from libvirtd. Attempting to destroy such a process fails
virsh --connect lxc:/// destroy distro_apparmor
error: Failed to destroy domain distro_apparmor
error: Failed to kill process 29491: Permission denied
And from /var/log/audit/audit.log
type=AVC msg=audit(1606949706.142:6345): apparmor="DENIED"
operation="signal"
profile="libvirt-314b7109-fdce-48dc-ad28-7c47958a27c1"
pid=29390 comm="libvirtd" requested_mask="receive"
denied_mask="receive"
signal=term peer="libvirtd"
Similar to the libvirt-qemu abstraction, add a rule to the libvirt-lxc
abstraction allowing reception of signals from libvirtd.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/security/apparmor/libvirt-lxc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/security/apparmor/libvirt-lxc b/src/security/apparmor/libvirt-lxc
index e556f2a7bd..0c8b812743 100644
--- a/src/security/apparmor/libvirt-lxc
+++ b/src/security/apparmor/libvirt-lxc
@@ -1,5 +1,9 @@
#include <abstractions/base>
+ # Allow receiving signals from libvirtd
+ signal (receive) peer=libvirtd,
+ signal (receive) peer=/usr/sbin/libvirtd,
+
umount,
# ignore DENIED message on / remount
--
2.29.2
3:46 a.m.
New subject: [PATCH 1/2] apparmor: Allow lxc processes to receive signals from libvirt
On Thu, Dec 3, 2020 at 3:58 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
LXC processes confined by apparmor are not permitted to receive signals
from libvirtd. Attempting to destroy such a process fails
virsh --connect lxc:/// destroy distro_apparmor
error: Failed to destroy domain distro_apparmor
error: Failed to kill process 29491: Permission denied
And from /var/log/audit/audit.log
type=AVC msg=audit(1606949706.142:6345): apparmor="DENIED"
operation="signal"
profile="libvirt-314b7109-fdce-48dc-ad28-7c47958a27c1"
pid=29390 comm="libvirtd" requested_mask="receive"
denied_mask="receive"
signal=term peer="libvirtd"
Similar to the libvirt-qemu abstraction, add a rule to the libvirt-lxc
abstraction allowing reception of signals from libvirtd.
Agreed that it is the same rule as in libvirt-qemu and therefore
should be rather safe.
TBH I did not see the denial when testing 6.9.0 [1], but the pattern
is known and therefore I think adding the rule is fine.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
[1]:
https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929...
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/security/apparmor/libvirt-lxc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/security/apparmor/libvirt-lxc b/src/security/apparmor/libvirt-lxc
index e556f2a7bd..0c8b812743 100644
--- a/src/security/apparmor/libvirt-lxc
+++ b/src/security/apparmor/libvirt-lxc
@@ -1,5 +1,9 @@
#include <abstractions/base>
+ # Allow receiving signals from libvirtd
+ signal (receive) peer=libvirtd,
+ signal (receive) peer=/usr/sbin/libvirtd,
+
umount,
# ignore DENIED message on / remount
--
2.29.2
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
8:57 p.m.
New subject: [PATCH 2/2] security: Avoid calling virSecurityManagerCheckModel with NULL model
Attempting to create a domain with <seclabel type='none'/> results in
virsh --connect lxc:/// create distro_nosec.xml
error: Failed to create domain from distro_nosec.xml
error: unsupported configuration: Security driver model '(null)' is not available
With <seclabel type='none'/>, the model field of virSecurityLabelDef will
be NULL, causing virSecurityManagerCheckModel() to fail with the above
error. Avoid calling virSecurityManagerCheckModel() when they seclabel
type is VIR_DOMAIN_SECLABEL_NONE.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
This could also be fixed by checking for a NULL secmodel in
virSecurityManagerCheckModel, but it seems more appropriate to check for
a valid seclabel type before checking the model.
src/security/security_manager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index be81ee5e44..789e24d273 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -781,6 +781,9 @@ virSecurityManagerCheckDomainLabel(virSecurityManagerPtr mgr,
size_t i;
for (i = 0; i < def->nseclabels; i++) {
+ if (def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_NONE)
+ continue;
+
if (virSecurityManagerCheckModel(mgr, def->seclabels[i]->model) < 0)
return -1;
}
--
2.29.2
8:01 a.m.
New subject: [PATCH 2/2] security: Avoid calling virSecurityManagerCheckModel with NULL model
On 12/3/20 3:57 AM, Jim Fehlig wrote:
Attempting to create a domain with <seclabel
type='none'/> results in
virsh --connect lxc:/// create distro_nosec.xml
error: Failed to create domain from distro_nosec.xml
error: unsupported configuration: Security driver model '(null)' is not
available
With <seclabel type='none'/>, the model field of virSecurityLabelDef will
be NULL, causing virSecurityManagerCheckModel() to fail with the above
error. Avoid calling virSecurityManagerCheckModel() when they seclabel
type is VIR_DOMAIN_SECLABEL_NONE.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
This could also be fixed by checking for a NULL secmodel in
virSecurityManagerCheckModel, but it seems more appropriate to check for
a valid seclabel type before checking the model.
src/security/security_manager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index be81ee5e44..789e24d273 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -781,6 +781,9 @@ virSecurityManagerCheckDomainLabel(virSecurityManagerPtr mgr,
size_t i;
for (i = 0; i < def->nseclabels; i++) {
+ if (def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_NONE)
+ continue;
+
if (virSecurityManagerCheckModel(mgr, def->seclabels[i]->model) < 0)
return -1;
}
This doesn't look right. If type='none' then ->model should contain
"none" string which in turn means virSecurityManagerCheckModel() is a NOP.
Looking into the code I've found the following in src/conf/domain_conf.c
line 9239:
/* Copy model from host. */
VIR_DEBUG("Found seclabel without a model, using '%s'",
xmlopt->config.defSecModel);
def->seclabels[0]->model = g_strdup(xmlopt->config.defSecModel);
and looking around and on git blame I found the following commit:
638ffa222847acc74dd2d84d2088590ecbf8eb70 (let's not get into who
reviewed it O:-)) which made ->model initialization happen only if
drvier initialized xmlopt .defSecModel which is happening only in qemu
driver and not LXC. Therefore I think we need something like this:
diff --git i/src/lxc/lxc_conf.c w/src/lxc/lxc_conf.c
index 13da6c4586..fc26a0a38b 100644
--- i/src/lxc/lxc_conf.c
+++ w/src/lxc/lxc_conf.c
@@ -212,6 +212,7 @@ virDomainXMLOptionPtr
lxcDomainXMLConfInit(virLXCDriverPtr driver)
{
virLXCDriverDomainDefParserConfig.priv = driver;
+ virLXCDriverDomainDefParserConfig.defSecModel = "none";
return virDomainXMLOptionNew(&virLXCDriverDomainDefParserConfig,
&virLXCDriverPrivateDataCallbacks,
&virLXCDriverDomainXMLNamespace,
But that makes lxcxml2xmltest fail, because the output XMLs don't
contain the model. I'm wondering if that's because we did not/do not
pass caps with host->nsecModels > 0 and thus this autofill wasn't run
but with the change I'm suggesting it is now.
Michal
5:36 p.m.
New subject: [PATCH 2/2] security: Avoid calling virSecurityManagerCheckModel with NULL model
On 12/3/20 7:01 AM, Michal Privoznik wrote:
On 12/3/20 3:57 AM, Jim Fehlig wrote:
> Attempting to create a domain with <seclabel type='none'/> results in
>
> virsh --connect lxc:/// create distro_nosec.xml
> error: Failed to create domain from distro_nosec.xml
> error: unsupported configuration: Security driver model '(null)' is not
available
>
> With <seclabel type='none'/>, the model field of virSecurityLabelDef
will
> be NULL, causing virSecurityManagerCheckModel() to fail with the above
> error. Avoid calling virSecurityManagerCheckModel() when they seclabel
> type is VIR_DOMAIN_SECLABEL_NONE.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
>
> This could also be fixed by checking for a NULL secmodel in
> virSecurityManagerCheckModel, but it seems more appropriate to check for
> a valid seclabel type before checking the model.
>
> src/security/security_manager.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index be81ee5e44..789e24d273 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -781,6 +781,9 @@ virSecurityManagerCheckDomainLabel(virSecurityManagerPtr mgr,
> size_t i;
> for (i = 0; i < def->nseclabels; i++) {
> + if (def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_NONE)
> + continue;
> +
> if (virSecurityManagerCheckModel(mgr, def->seclabels[i]->model) <
0)
> return -1;
> }
>
This doesn't look right. If type='none' then ->model should contain
"none"
string which in turn means virSecurityManagerCheckModel() is a NOP.
Looking into the code I've found the following in src/conf/domain_conf.c line 9239:
/* Copy model from host. */
VIR_DEBUG("Found seclabel without a model, using '%s'",
xmlopt->config.defSecModel);
def->seclabels[0]->model = g_strdup(xmlopt->config.defSecModel);
and looking around and on git blame I found the following commit:
638ffa222847acc74dd2d84d2088590ecbf8eb70 (let's not get into who reviewed it
O:-)) which made ->model initialization happen only if drvier initialized xmlopt
.defSecModel which is happening only in qemu driver and not LXC.
Ah, thanks for a pointer to the commit. That's a nice hint!
Therefore I think we need something like this:
diff --git i/src/lxc/lxc_conf.c w/src/lxc/lxc_conf.c
index 13da6c4586..fc26a0a38b 100644
--- i/src/lxc/lxc_conf.c
+++ w/src/lxc/lxc_conf.c
@@ -212,6 +212,7 @@ virDomainXMLOptionPtr
lxcDomainXMLConfInit(virLXCDriverPtr driver)
{
virLXCDriverDomainDefParserConfig.priv = driver;
+ virLXCDriverDomainDefParserConfig.defSecModel = "none";
return virDomainXMLOptionNew(&virLXCDriverDomainDefParserConfig,
&virLXCDriverPrivateDataCallbacks,
&virLXCDriverDomainXMLNamespace,
But that makes lxcxml2xmltest fail, because the output XMLs don't contain the
model. I'm wondering if that's because we did not/do not pass caps with
host->nsecModels > 0 and thus this autofill wasn't run but with the change
I'm
suggesting it is now.
I took a similar approach as the qemu driver and initialized the defSecModel
from the driver's securityManager (ignoring stacked managers since afaict the
lxc driver doesn't support it). With this approach lxcxml2xmltest passes and my
issue is resolved :-).
https://www.redhat.com/archives/libvir-list/2020-December/msg00291.html
Regards,
Jim
1453
days inactive
1453
days old
5 comments
3 participants
participants (3)
-
Christian Ehrhardt -
Jim Fehlig -
Michal Privoznik