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

Re: [libvirt] [PATCH v2 3/5] Add two new security label types



On 01/25/2012 07:12 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> Curently security labels can be of type 'dynamic' or 'static'.

s/Curently/Currently/

> If no security label is given, then 'dynamic' is assumed. The
> current code takes advantage of this default, and avoids even
> saving <seclabel> elements with type='dynamic' to disk. This
> means if you temporarily change security driver, the guests
> can all still start.
> 
> With the introduction of sVirt to LXC though, there needs to be
> a new default of 'none' to allow unconfined LXC containers.
> 
> This patch introduces two new security label types
> 
>  - default:  the host configuration decides whether to run the
>              guest with type 'none' or 'dynamic' at guest start
>  - none:     the guest will run unconfined by security policy
> 
> The 'none' label type will obviously be undesirable for some
> deployments, so a new qemu.conf option allows a host admin to
> mandate confined guests. It is also possible to turn off default
> confinement
> 
>   security_default_confined = 1|0  (default == 1)
>   security_require_confined = 1|0  (default == 0)
> 
> * src/conf/domain_conf.c, src/conf/domain_conf.h: Add new
>   seclabel types
> * src/security/security_manager.c, src/security/security_manager.h:
>   Set default sec label types
> * src/security/security_selinux.c: Handle 'none' seclabel type
> * src/qemu/qemu.conf, src/qemu/qemu_conf.c, src/qemu/qemu_conf.h,
>   src/qemu/libvirtd_qemu.aug: New security config options
> * src/qemu/qemu_driver.c: Tell security driver about default
>   config
> ---
>  docs/formatdomain.html.in       |   24 +++++++++----
>  docs/schemas/domaincommon.rng   |    5 +++
>  po/POTFILES.in                  |    1 +
>  src/conf/domain_conf.c          |   70 ++++++++++++++++++++++++--------------
>  src/conf/domain_conf.h          |    2 +
>  src/qemu/libvirtd_qemu.aug      |    2 +
>  src/qemu/qemu.conf              |    8 ++++
>  src/qemu/qemu_conf.c            |   11 ++++++
>  src/qemu/qemu_conf.h            |    2 +
>  src/qemu/qemu_driver.c          |    7 +++-
>  src/security/security_manager.c |   51 +++++++++++++++++++++++++---
>  src/security/security_manager.h |    8 ++++-
>  src/security/security_selinux.c |   32 ++++++++++++++----
>  tests/seclabeltest.c            |    2 +-
>  14 files changed, 177 insertions(+), 48 deletions(-)

Just glancing at this diffstat, it looks like you hit my major concerns
from v1
(https://www.redhat.com/archives/libvir-list/2012-January/msg00940.html)

> @@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null
>  
>      <p>
>        The <code>seclabel</code> element allows control over the
> -      operation of the security drivers. There are two basic
> -      modes of operation, dynamic where libvirt automatically
> -      generates a unique security label, or static where the
> -      application/administrator chooses the labels. With dynamic
> +      operation of the security drivers. There are three basic
> +      modes of operation, 'dynamic' where libvirt automatically
> +      generates a unique security label, 'static' where the
> +      application/administrator chooses the labels, or 'none'
> +      where confinement is disabled. With dynamic
>        label generation, libvirt will always automatically
>        relabel any resources associated with the virtual machine.
>        With static label assignment, by default, the administrator

Probably want to also document with a <span class="since"> that 'none'
was introduced in 0.9.10.

> @@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null
>    &lt;seclabel type='static' model='selinux' relabel='yes'&gt;
>      &lt;label&gt;system_u:system_r:svirt_t:s0:c392,c662&lt;/label&gt;
>    &lt;/seclabel&gt;
> +
> +  &lt;seclabel type='none'/&gt;
>      </pre>
>  
>      <p>
> +      If no 'type' attribute is provided in the input XML, then
> +      the security driver default setting will be used, which
> +      may be either 'none' or 'static'.

Actually, it is either 'none' or 'dynamic'; the only way to get 'static'
is with explicit type attribute.

> @@ -2591,12 +2602,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
>          def->imagelabel = p;
>      }
>  
> -    /* Only parse baselabel, for dynamic label */
> -    if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> +    /* Only parse baselabel, for dynamic or none label types */
> +    if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC ||
> +        def->type == VIR_DOMAIN_SECLABEL_NONE) {
>          p = virXPathStringLimit("string(./seclabel/baselabel[1])",
>                                  VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
>          if (p != NULL)
>              def->baselabel = p;
> +        /* Forces none type to dynamic for back compat */
> +        def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;

Missing braces.  This should be:

if (p != NULL) {
    def->baselabel = p;
    /* Force none to dynamic for back compat */
    def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
}

ACK with those items fixed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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