[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] This patch adds the label to lxc-enter-namespace



On Fri, Jan 25, 2013 at 02:39:25PM -0500, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> (2nd pass)
> 
> 
> lxc-enter-namespace  allows a process from outside a container to start a
> process inside a container.  One problem with the current code is the process
> running within the container would run with the label of the process that
> created it.
> 
> For example if the admin process is running as unconfined_t and executes the
> following command
> 
> 
> # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
> LABEL                             PID TTY          TIME CMD
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient
> staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps
> 
> Note the ps command is running as unconfined_t,  After this patch,
> 
> 
> virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ
> LABEL                             PID TTY          TIME CMD
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps
> 
> I also add a --nolabel command to virsh, which can go back to the original
> behaviour.
> 
> virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
> LABEL                             PID TTY          TIME CMD
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
> system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
> staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps


> diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h
> index f2c87fb..257637b 100644
> --- a/include/libvirt/libvirt-lxc.h
> +++ b/include/libvirt/libvirt-lxc.h
> @@ -43,6 +43,9 @@ int virDomainLxcEnterNamespace(virDomainPtr domain,
>                                 int **oldfdlist,
>                                 unsigned int flags);
>  
> +int virDomainLxcGetSecurityLabel(virDomainPtr domain,
> +                                 virSecurityLabelPtr seclabel);
> +

I'm not sure why you're adding this method - it is identical
to the existing virDomainGetSecurityLabel() method. Just
remove this.

>  # ifdef __cplusplus
>  }
>  # endif
> diff --git a/python/generator.py b/python/generator.py
> index f853d77..f98818e 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -551,6 +551,7 @@ skip_function = (
>  
>  lxc_skip_function = (
>    "virDomainLxcEnterNamespace",
> +  "virDomainLxcGetSecurityLabel",

And remove this.

> diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
> index f580c3c..a4aff59 100644
> --- a/src/libvirt-lxc.c
> +++ b/src/libvirt-lxc.c
> @@ -41,6 +41,57 @@
>                           __LINE__, info)
>  
>  /**
> + * virDomainLxcGetSecurityLabel:
> + * @domain: a domain object
> + * @seclabel: pointer to a virSecurityLabel structure
> + *
> + * This API is LXC specific, so it will only work with hypervisor
> + * connections to the LXC driver.
> + *
> + * Get the security label associated with the container @domain.
> + *
> + * Returns 0 on success, or -1 on error
> + */
> +int
> +virDomainLxcGetSecurityLabel(virDomainPtr domain,
> +				 virSecurityLabelPtr seclabel)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DEBUG("domain=%p", domain);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> +	virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> +	virDispatchError(NULL);
> +	return -1;
> +    }
> +
> +    conn = domain->conn;
> +
> +    if (conn->flags & VIR_CONNECT_RO) {
> +	virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +	goto error;
> +    }
> +
> +    if (conn->driver->domainGetSecurityLabel) {
> +
> +	if (conn->driver->domainGetSecurityLabel(domain,
> +						 seclabel) < 0)
> +	    goto error;
> +
> +	return 0;
> +    }
> +
> +    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(conn);
> +    return -1;
> +}

And remove this.

> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 1fe8039..fd60712 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -1125,6 +1125,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla
>  {
>      virLXCDriverPtr driver = dom->conn->privateData;
>      virDomainObjPtr vm;
> +    virLXCDomainObjPrivatePtr priv;
>      int ret = -1;
>  
>      lxcDriverLock(driver);
> @@ -1162,8 +1163,14 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla
>       *   LXC monitor hasn't seen SIGHUP/ERR on poll().
>       */
>      if (virDomainObjIsActive(vm)) {
> +	priv = vm->privateData;
> +	if (!priv->initpid) {
> +	    virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +			   _("Init pid is not yet available"));
> +	    goto cleanup;
> +	}

Identation has gone a bit screwy here.

>          if (virSecurityManagerGetProcessLabel(driver->securityManager,
> -                                              vm->def, vm->pid, seclabel) < 0) {
> +                                              vm->def, priv->initpid, seclabel) < 0) {

Yep, this makes sense

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 026dac1..c53f817 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -56,6 +56,10 @@
>  #include "virtypedparam.h"
>  #include "virxml.h"
>  
> +#ifdef WITH_SELINUX
> +#include <selinux/selinux.h>
> +#endif

You need to use '# include' rather than "#include" in order
to get 'make syntax-check' pass

> @@ -7678,6 +7682,7 @@ static const vshCmdInfo info_lxc_enter_namespace[] = {
>  
>  static const vshCmdOptDef opts_lxc_enter_namespace[] = {
>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> +    {"nolabel", VSH_OT_BOOL, 0, N_("Do Not Change Process Label ")},
>      {"cmd", VSH_OT_ARGV, VSH_OFLAG_REQ, N_("namespace")},
>      {NULL, 0, 0, NULL}
>  };
> @@ -7686,6 +7691,7 @@ static bool
>  cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
>  {
>      virDomainPtr dom = NULL;
> +    virSecurityLabelPtr seclabel;

If you declare this as 'virSecurityLabel seclabel'...

>      bool ret = false;
>      const vshCmdOpt *opt = NULL;
>      char **cmdargv = NULL;
> @@ -7694,6 +7700,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
>      int nfdlist;
>      int *fdlist;
>      size_t i;
> +    int label = false;
>  
>      dom = vshCommandOptDomain(ctl, cmd, NULL);
>      if (dom == NULL)
> @@ -7715,12 +7722,30 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
>      if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist, 0)) < 0)
>          goto cleanup;
>  
> +    if (! vshCommandOptBool(cmd, "nolabel")) {
> +        if (VIR_ALLOC(seclabel) < 0)
> +            goto cleanup;

...this allocation can be removed.

> +        label = true;
> +        if (virDomainLxcGetSecurityLabel(dom, seclabel) < 0)

Just replace this call with virDomainGetSecurityLabel()

>  cleanup:
> +    if (label)
> +        VIR_FREE(seclabel);

And then this can go too


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]