[PATCH 2/2] security: Avoid calling virSecurityManagerCheckModel with NULL model

[Michal Privoznik]

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 at 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