On 01/11/2012 09:33 AM, Daniel P. Berrange wrote: This line is not quite accurate, to this diffstat (drop lxc_container). Otherwise, looks like a useful API. Should we be using virFileOpenAs() for the sake of touching a file with different permissions on a root-squash NFS server? In particular, I wonder if any of qemu_driver.c could reuse this function, if it were to use virFileOpenAs() - for example, creating a qemu snapshot requires that a file already exist and be empty before making the monitor call. ACK - any change to use virFileOpenAs can come as a later patch. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/11/2012 09:33 AM, Daniel P. Berrange wrote: Complex enough that I agree with your decision to separate the revert from the new implementation, even if it means a potential for a failed 'git bisect'. Missing a check here that we aren't trying to override when the top-level seclabel forbids it; something to replace this hunk removed in 3/7: - /* Can't use overrides if top-level doesn't allow relabeling. */ - if (default_seclabel && default_seclabel->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", - _("label overrides require relabeling to be " - "enabled at the domain level")); - goto cleanup; except that it would now be looking at 'vmDef->norelabel' instead of 'default_seclabel && default_seclabel->norelabel'. The code in 3/7 was defaulting (*def)->norelabel to false, regardless of the top-level setting (that is, the only way to get a device label to set norelabel is with an explicit attribute, so that a device label of: <seclabel> <label>...</label> </seclabel> is then valid shorthand for: <seclabel relabel='yes'> <label>...</label> </seclabel> Do we want to be saving this label if norelabel is true (that is, if the user passes: <seclabel relabel='no'> <label>...</label> </seclabel> do we reject that as invalid, or silently ignore the useless <label>, instead of your behavior of accepting it)? With norelabel, this would generate the odd-looking: <seclabel relabel='no'> </seclabel> How about instead doing: virBufferAsprintf(buf, "<seclabel relabel='%s'", def->norelabel ? "no" : "yes"); if (def->label) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); } Close, but I'd feel a bit more comfortable seeing a v2. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Curently security labels can be of type 'dynamic' or 'static'. 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 introduce 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 --- src/conf/domain_conf.c | 81 +++++++++++++++++++++++---------------- 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 | 28 ++++++++++---- 10 files changed, 152 insertions(+), 48 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4ae417e..a2b7430 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -551,6 +551,8 @@ VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, "unknown") VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST, + "default", + "none", "dynamic", "static") @@ -2547,13 +2549,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, "%s", _("missing security type")); goto error; } + def->type = virDomainSeclabelTypeFromString(p); VIR_FREE(p); - if (def->type < 0) { + if (def->type <= 0) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("invalid security type")); goto error; } + p = virXPathStringLimit("string(./seclabel/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { @@ -2574,13 +2578,23 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, "%s", _("dynamic label type must use resource relabeling")); goto error; } + if (def->type == VIR_DOMAIN_SECLABEL_NONE && + !def->norelabel) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("dynamic label type must use not resource relabeling")); + goto error; + } } else { - if (def->type == VIR_DOMAIN_SECLABEL_STATIC) + if (def->type == VIR_DOMAIN_SECLABEL_STATIC || + def->type == VIR_DOMAIN_SECLABEL_NONE) def->norelabel = true; else def->norelabel = false; } + if (def->type == VIR_DOMAIN_SECLABEL_NONE) + return 0; + /* Only parse label, if using static labels, or * if the 'live' VM XML is requested */ @@ -9810,37 +9824,41 @@ virDomainLifecycleDefFormat(virBufferPtr buf, } -static int -virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def, - unsigned int flags) +static void +virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def) { const char *sectype = virDomainSeclabelTypeToString(def->type); - int ret = -1; if (!sectype) - goto cleanup; + return; - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - !def->baselabel && - (flags & VIR_DOMAIN_XML_INACTIVE)) { - /* This is the default for inactive xml, so nothing to output. */ - } else { - virBufferAsprintf(buf, "<seclabel type='%s' model='%s' relabel='%s'>\n", - sectype, def->model, - def->norelabel ? "no" : "yes"); - virBufferEscapeString(buf, " <label>%s</label>\n", - def->label); - if (!def->norelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", - def->imagelabel); - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", - def->baselabel); - virBufferAddLit(buf, "</seclabel>\n"); + if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) + return; + + virBufferAsprintf(buf, "<seclabel type='%s'", + sectype); + if (def->model) + virBufferEscapeString(buf, " model='%s' ", def->model); + + virBufferAsprintf(buf, " relabel='%s'", + def->norelabel ? "no" : "yes"); + + if (def->type == VIR_DOMAIN_SECLABEL_NONE) { + virBufferAddLit(buf, "/>\n"); + return; } - ret = 0; -cleanup: - return ret; + + virBufferAddLit(buf, ">\n"); + + virBufferEscapeString(buf, " <label>%s</label>\n", + def->label); + if (!def->norelabel) + virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", + def->imagelabel); + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", + def->baselabel); + virBufferAddLit(buf, "</seclabel>\n"); } @@ -11811,12 +11829,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </devices>\n"); - if (def->seclabel.model) { - virBufferAdjustIndent(buf, 2); - if (virSecurityLabelDefFormat(buf, &def->seclabel, flags) < 0) - goto cleanup; - virBufferAdjustIndent(buf, -2); - } + virBufferAdjustIndent(buf, 2); + virSecurityLabelDefFormat(buf, &def->seclabel); + virBufferAdjustIndent(buf, -2); if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 25aae62..d3a1175 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -162,6 +162,8 @@ struct _virDomainDeviceInfo { }; enum virDomainSeclabelType { + VIR_DOMAIN_SECLABEL_DEFAULT, + VIR_DOMAIN_SECLABEL_NONE, VIR_DOMAIN_SECLABEL_DYNAMIC, VIR_DOMAIN_SECLABEL_STATIC, diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 078e9c4..f6cec1f 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -33,6 +33,8 @@ module Libvirtd_qemu = | bool_entry "vnc_sasl" | str_entry "vnc_sasl_dir" | str_entry "security_driver" + | bool_entry "security_default_confined" + | bool_entry "security_require_confined" | str_entry "user" | str_entry "group" | bool_entry "dynamic_ownership" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 4ec5e6c..d32a4c3 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -138,6 +138,14 @@ # # security_driver = "selinux" +# If set to non-zero, then the default security labelling +# will make guests confined. If set to zero, then guests +# will be unconfined by default. Defaults to 1 +# security_default_confined = 1 + +# If set to non-zero, then attempts to create unconfined +# guests will be blocked. Defaults to zero. +# security_require_confined = 1 # The user ID for QEMU processes run by the system instance. #user = "root" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index bc0a646..e95c7a5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -75,6 +75,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, int i; /* Setup critical defaults */ + driver->securityDefaultConfined = true; + driver->securityRequireConfined = false; driver->dynamicOwnership = 1; driver->clearEmulatorCapabilities = 1; @@ -195,6 +197,15 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "security_default_confined"); + CHECK_TYPE ("security_default_confined", VIR_CONF_LONG); + if (p) driver->securityDefaultConfined = p->l; + + p = virConfGetValue (conf, "security_require_confined"); + CHECK_TYPE ("security_require_confined", VIR_CONF_LONG); + if (p) driver->securityRequireConfined = p->l; + + p = virConfGetValue (conf, "vnc_sasl"); CHECK_TYPE ("vnc_sasl", VIR_CONF_LONG); if (p) driver->vncSASL = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d8d7915..fb98ef2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -115,6 +115,8 @@ struct qemud_driver { virDomainEventStatePtr domainEventState; char *securityDriverName; + bool securityDefaultConfined; + bool securityRequireConfined; virSecurityManagerPtr securityManager; char *saveImageFormat; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 712f1fc..b697f43 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -208,7 +208,10 @@ static int qemuSecurityInit(struct qemud_driver *driver) { virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName, - driver->allowDiskFormatProbing); + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined); + if (!mgr) goto error; @@ -216,6 +219,8 @@ qemuSecurityInit(struct qemud_driver *driver) virSecurityManagerPtr dac = virSecurityManagerNewDAC(driver->user, driver->group, driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined, driver->dynamicOwnership); if (!dac) goto error; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 2e4956a..d0bafae 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -36,10 +36,14 @@ struct _virSecurityManager { virSecurityDriverPtr drv; bool allowDiskFormatProbing; + bool defaultConfined; + bool requireConfined; }; static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, - bool allowDiskFormatProbing) + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityManagerPtr mgr; @@ -50,6 +54,8 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr mgr->drv = drv; mgr->allowDiskFormatProbing = allowDiskFormatProbing; + mgr->defaultConfined = defaultConfined; + mgr->requireConfined = requireConfined; if (drv->open(mgr) < 0) { virSecurityManagerFree(mgr); @@ -64,7 +70,9 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, - virSecurityManagerGetAllowDiskFormatProbing(primary)); + virSecurityManagerGetAllowDiskFormatProbing(primary), + virSecurityManagerGetDefaultConfined(primary), + virSecurityManagerGetRequireConfined(primary)); if (!mgr) return NULL; @@ -78,11 +86,15 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, gid_t group, bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined, bool dynamicOwnership) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, - allowDiskFormatProbing); + allowDiskFormatProbing, + defaultConfined, + requireConfined); if (!mgr) return NULL; @@ -95,13 +107,18 @@ virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, } virSecurityManagerPtr virSecurityManagerNew(const char *name, - bool allowDiskFormatProbing) + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityDriverPtr drv = virSecurityDriverLookup(name); if (!drv) return NULL; - return virSecurityManagerNewDriver(drv, allowDiskFormatProbing); + return virSecurityManagerNewDriver(drv, + allowDiskFormatProbing, + defaultConfined, + requireConfined); } @@ -149,6 +166,16 @@ bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) return mgr->allowDiskFormatProbing; } +bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) +{ + return mgr->defaultConfined; +} + +bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) +{ + return mgr->requireConfined; +} + int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainDiskDefPtr disk) @@ -248,6 +275,20 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { + if (vm->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) { + if (mgr->defaultConfined) + vm->seclabel.type = VIR_DOMAIN_SECLABEL_DYNAMIC; + else + vm->seclabel.type = VIR_DOMAIN_SECLABEL_NONE; + } + + if ((vm->seclabel.type == VIR_DOMAIN_SECLABEL_NONE) && + mgr->requireConfined) { + virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unconfined guests are not allowed on this host")); + return -1; + } + if (mgr->drv->domainGenSecurityLabel) return mgr->drv->domainGenSecurityLabel(mgr, vm); diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 6731d59..32c8c3b 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -32,7 +32,9 @@ typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; virSecurityManagerPtr virSecurityManagerNew(const char *name, - bool allowDiskFormatProbing); + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined); virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr secondary); @@ -40,6 +42,8 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, gid_t group, bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined, bool dynamicOwnership); void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); @@ -49,6 +53,8 @@ void virSecurityManagerFree(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr); const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr); bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); +bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); +bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index c2dceca..ade36f9 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -170,6 +170,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, int c1 = 0; int c2 = 0; context_t ctx = NULL; + const char *range; if ((def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) && !def->seclabel.baselabel && @@ -200,7 +201,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return rc; } - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) { + switch (def->seclabel.type) { + case VIR_DOMAIN_SECLABEL_STATIC: if (!(ctx = context_new(def->seclabel.label)) ) { virReportSystemError(errno, _("unable to allocate socket security context '%s'"), @@ -208,13 +210,15 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return rc; } - const char *range = context_range_get(ctx); + range = context_range_get(ctx); if (!range || !(mcs = strdup(range))) { virReportOOMError(); goto cleanup; } - } else { + break; + + case VIR_DOMAIN_SECLABEL_DYNAMIC: do { c1 = virRandom(1024); c2 = virRandom(1024); @@ -246,12 +250,20 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, _("cannot generate selinux context for %s"), mcs); goto cleanup; } + break; + + case VIR_DOMAIN_SECLABEL_NONE: + /* no op */ + break; } - def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs); - if (!def->seclabel.imagelabel) { - virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate selinux context for %s"), mcs); - goto cleanup; + + if (!def->seclabel.norelabel) { + def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs); + if (!def->seclabel.imagelabel) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot generate selinux context for %s"), mcs); + goto cleanup; + } } if (!def->seclabel.model && -- 1.7.7.5

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; For the sake of backwards compat, LXC guests are *not* confined by default. This is because it is not practical to dynamically relabel containers using large filesystem trees. Applications can create confined containers though, by giving suitable XML configs * src/Makefile.am: Link libvirt_lxc to security drivers * src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h, src/lxc/lxc_conf.c, src/lxc/lxc.conf, src/lxc/test_libvirtd_lxc.aug: Config file handling for security driver * src/lxc/lxc_driver.c: Wire up security driver functions * src/lxc/lxc_controller.c: Add a '--security' flag to specify which security driver to activate * src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set the process label just before exec'ing init. --- src/Makefile.am | 13 +++ src/lxc/libvirtd_lxc.aug | 15 +++- src/lxc/lxc.conf | 18 ++++ src/lxc/lxc_conf.c | 62 ++++++++++++-- src/lxc/lxc_conf.h | 8 ++- src/lxc/lxc_container.c | 9 ++- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 28 +++++- src/lxc/lxc_driver.c | 198 ++++++++++++++++++++++++++++++++++++++++- src/lxc/test_libvirtd_lxc.aug | 2 + 10 files changed, 340 insertions(+), 15 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 0a1221a..a4688a1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1501,7 +1501,14 @@ libvirt_lxc_SOURCES = \ $(DOMAIN_CONF_SOURCES) \ $(SECRET_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ + $(SECURITY_DRIVER_SOURCES) \ $(NWFILTER_PARAM_CONF_SOURCES) +if WITH_SECDRIVER_SELINUX +libvirt_lxc_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES) +endif +if WITH_SECDRIVER_APPARMOR +libvirt_lxc_SOURCES += $(SECURITY_DRIVER_APPARMOR_SOURCES) +endif libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS) libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ $(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \ @@ -1511,6 +1518,9 @@ libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ if WITH_DTRACE libvirt_lxc_LDADD += probes.o endif +if WITH_SECDRIVER_SELINUX +libvirt_lxc_LDADD += $(SELINUX_LIBS) +endif libvirt_lxc_CFLAGS = \ $(LIBPARTED_CFLAGS) \ $(NUMACTL_CFLAGS) \ @@ -1523,6 +1533,9 @@ if HAVE_LIBBLKID libvirt_lxc_CFLAGS += $(BLKID_CFLAGS) libvirt_lxc_LDADD += $(BLKID_LIBS) endif +if WITH_SECDRIVER_SELINUX +libvirt_lxc_CFLAGS += $(SELINUX_CFLAGS) +endif endif endif EXTRA_DIST += $(LXC_CONTROLLER_SOURCES) diff --git a/src/lxc/libvirtd_lxc.aug b/src/lxc/libvirtd_lxc.aug index 10f25e4..be6402c 100644 --- a/src/lxc/libvirtd_lxc.aug +++ b/src/lxc/libvirtd_lxc.aug @@ -7,13 +7,26 @@ module Libvirtd_lxc = let value_sep = del /[ \t]*=[ \t]*/ " = " let indent = del /[ \t]*/ "" + let array_sep = del /,[ \t\n]*/ ", " + let array_start = del /\[[ \t\n]*/ "[ " + let array_end = del /\]/ "]" + + let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\"" let bool_val = store /0|1/ + let int_val = store /[0-9]+/ + let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" + let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] - + let int_entry (kw:string) = [ key kw . value_sep . int_val ] + let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ] (* Config entry grouped by function - same order as example config *) let log_entry = bool_entry "log_with_libvirtd" + | str_entry "security_driver" + | bool_entry "security_default_confined" + | bool_entry "security_require_confined" (* Each enty in the config is one of the following three ... *) let entry = log_entry diff --git a/src/lxc/lxc.conf b/src/lxc/lxc.conf index 7a5066f..ca3f4c6 100644 --- a/src/lxc/lxc.conf +++ b/src/lxc/lxc.conf @@ -11,3 +11,21 @@ # This is disabled by default, uncomment below to enable it. # # log_with_libvirtd = 1 + + +# The default security driver is SELinux. If SELinux is disabled +# on the host, then the security driver will automatically disable +# itself. If you wish to disable QEMU SELinux security driver while +# leaving SELinux enabled for the host in general, then set this +# to 'none' instead. +# +# security_driver = "selinux" + +# If set to non-zero, then the default security labelling +# will make guests confined. If set to zero, then guests +# will be unconfined by default. Defaults to zero +# security_default_confined = 1 + +# If set to non-zero, then attempts to create unconfined +# guests will be blocked. Defaults to zero. +# security_require_confined = 1 \ No newline at end of file diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index e842736..72547c4 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -49,7 +49,7 @@ static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) /* Functions */ -virCapsPtr lxcCapsInit(void) +virCapsPtr lxcCapsInit(lxc_driver_t *driver) { struct utsname utsname; virCapsPtr caps; @@ -127,8 +127,30 @@ virCapsPtr lxcCapsInit(void) /* LXC Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); + if (driver) { + /* Security driver data */ + const char *doi, *model; + + doi = virSecurityManagerGetDOI(driver->securityManager); + model = virSecurityManagerGetModel(driver->securityManager); + if (STRNEQ(model, "none")) { + if (!(caps->host.secModel.model = strdup(model))) + goto no_memory; + if (!(caps->host.secModel.doi = strdup(doi))) + goto no_memory; + } + + VIR_DEBUG("Initialized caps for security driver \"%s\" with " + "DOI \"%s\"", model, doi); + } else { + VIR_INFO("No driver, not initializing security driver"); + } + return caps; +no_memory: + virReportOOMError(); + error: virCapabilitiesFree(caps); return NULL; @@ -140,6 +162,9 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) virConfPtr conf; virConfValuePtr p; + driver->securityDefaultConfined = false; + driver->securityRequireConfined = false; + /* Set the container configuration directory */ if ((driver->configDir = strdup(LXC_CONFIG_DIR)) == NULL) goto no_memory; @@ -161,14 +186,39 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) if (!conf) goto done; +#define CHECK_TYPE(name,typ) if (p && p->type != (typ)) { \ + lxcError(VIR_ERR_INTERNAL_ERROR, \ + "%s: %s: expected type " #typ, \ + filename, (name)); \ + virConfFree(conf); \ + return -1; \ + } + p = virConfGetValue(conf, "log_with_libvirtd"); - if (p) { - if (p->type != VIR_CONF_LONG) - VIR_WARN("lxcLoadDriverConfig: invalid setting: log_with_libvirtd"); - else - driver->log_libvirtd = p->l; + CHECK_TYPE ("log_with_libvirtd", VIR_CONF_LONG); + if (p) driver->log_libvirtd = p->l; + + p = virConfGetValue (conf, "security_driver"); + CHECK_TYPE ("security_driver", VIR_CONF_STRING); + if (p && p->str) { + if (!(driver->securityDriverName = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } } + p = virConfGetValue (conf, "security_default_confined"); + CHECK_TYPE ("security_default_confined", VIR_CONF_LONG); + if (p) driver->securityDefaultConfined = p->l; + + p = virConfGetValue (conf, "security_require_confined"); + CHECK_TYPE ("security_require_confined", VIR_CONF_LONG); + if (p) driver->securityRequireConfined = p->l; + + +#undef CHECK_TYPE + virConfFree(conf); done: diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index b124330..ebdc173 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -33,6 +33,7 @@ # include "capabilities.h" # include "threads.h" # include "cgroup.h" +# include "security/security_manager.h" # include "configmake.h" # define LXC_CONFIG_DIR SYSCONFDIR "/libvirt/lxc" @@ -57,6 +58,11 @@ struct __lxc_driver { virDomainEventStatePtr domainEventState; + char *securityDriverName; + bool securityDefaultConfined; + bool securityRequireConfined; + virSecurityManagerPtr securityManager; + /* Mapping of 'char *uuidstr' -> virConnectPtr * of guests which will be automatically killed * when the virConnectPtr is closed*/ @@ -64,7 +70,7 @@ struct __lxc_driver { }; int lxcLoadDriverConfig(lxc_driver_t *driver); -virCapsPtr lxcCapsInit(void); +virCapsPtr lxcCapsInit(lxc_driver_t *driver); # define lxcError(code, ...) \ virReportErrorHelper(VIR_FROM_LXC, code, __FILE__, \ diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 3fb7d06..4df0b55 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -91,6 +91,7 @@ typedef char lxc_message_t; typedef struct __lxc_child_argv lxc_child_argv_t; struct __lxc_child_argv { virDomainDefPtr config; + virSecurityManagerPtr securityDriver; unsigned int nveths; char **veths; int monitor; @@ -1273,6 +1274,10 @@ static int lxcContainerChild( void *data ) goto cleanup; } + VIR_DEBUG("Setting up security labeling"); + if (virSecurityManagerSetProcessLabel(argv->securityDriver, vmDef) < 0) + goto cleanup; + ret = 0; cleanup: VIR_FREE(ttyPath); @@ -1335,6 +1340,7 @@ const char *lxcContainerGetAlt32bitArch(const char *arch) * Returns PID of container on success or -1 in case of error */ int lxcContainerStart(virDomainDefPtr def, + virSecurityManagerPtr securityDriver, unsigned int nveths, char **veths, int control, @@ -1346,7 +1352,8 @@ int lxcContainerStart(virDomainDefPtr def, int cflags; int stacksize = getpagesize() * 4; char *stack, *stacktop; - lxc_child_argv_t args = { def, nveths, veths, control, + lxc_child_argv_t args = { def, securityDriver, + nveths, veths, control, ttyPaths, nttyPaths, handshakefd}; /* allocate a stack for the container */ diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index ffeda5e..77fb9b2 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -25,6 +25,7 @@ # define LXC_CONTAINER_H # include "lxc_conf.h" +# include "security/security_manager.h" enum { LXC_CONTAINER_FEATURE_NET = (1 << 0), @@ -49,6 +50,7 @@ int lxcContainerSendContinue(int control); int lxcContainerWaitForContinue(int control); int lxcContainerStart(virDomainDefPtr def, + virSecurityManagerPtr securityDriver, unsigned int nveths, char **veths, int control, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index bb936ee..6bc54b7 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1267,6 +1267,7 @@ cleanup: static int lxcControllerRun(virDomainDefPtr def, + virSecurityManagerPtr securityDriver, unsigned int nveths, char **veths, int monitor, @@ -1421,6 +1422,7 @@ lxcControllerRun(virDomainDefPtr def, goto cleanup; if ((container = lxcContainerStart(def, + securityDriver, nveths, veths, control[1], @@ -1529,11 +1531,13 @@ int main(int argc, char *argv[]) { "veth", 1, NULL, 'v' }, { "console", 1, NULL, 'c' }, { "handshakefd", 1, NULL, 's' }, + { "security", 1, NULL, 'S' }, { "help", 0, NULL, 'h' }, { 0, 0, 0, 0 }, }; int *ttyFDs = NULL; size_t nttyFDs = 0; + virSecurityManagerPtr securityDriver = NULL; if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || @@ -1545,7 +1549,7 @@ int main(int argc, char *argv[]) while (1) { int c; - c = getopt_long(argc, argv, "dn:v:m:c:s:h", + c = getopt_long(argc, argv, "dn:v:m:c:s:h:S:", options, NULL); if (c == -1) @@ -1593,6 +1597,14 @@ int main(int argc, char *argv[]) } break; + case 'S': + if (!(securityDriver = virSecurityManagerNew(optarg, false, false, false))) { + fprintf(stderr, "Cannot create security manager '%s'", + optarg); + goto cleanup; + } + break; + case 'h': case '?': fprintf(stderr, "\n"); @@ -1605,12 +1617,20 @@ int main(int argc, char *argv[]) fprintf(stderr, " -c FD, --console FD\n"); fprintf(stderr, " -v VETH, --veth VETH\n"); fprintf(stderr, " -s FD, --handshakefd FD\n"); + fprintf(stderr, " -S NAME, --security NAME\n"); fprintf(stderr, " -h, --help\n"); fprintf(stderr, "\n"); goto cleanup; } } + if (securityDriver == NULL) { + if (!(securityDriver = virSecurityManagerNew("none", false, false, false))) { + fprintf(stderr, "%s: cannot initialize nop security manager", argv[0]); + goto cleanup; + } + } + if (name == NULL) { fprintf(stderr, "%s: missing --name argument for configuration\n", argv[0]); @@ -1630,7 +1650,7 @@ int main(int argc, char *argv[]) virEventRegisterDefaultImpl(); - if ((caps = lxcCapsInit()) == NULL) + if ((caps = lxcCapsInit(NULL)) == NULL) goto cleanup; if ((configFile = virDomainConfigFile(LXC_STATE_DIR, @@ -1696,10 +1716,10 @@ int main(int argc, char *argv[]) goto cleanup; } - rc = lxcControllerRun(def, nveths, veths, monitor, client, + rc = lxcControllerRun(def, securityDriver, + nveths, veths, monitor, client, ttyFDs, nttyFDs, handshakefd); - cleanup: if (def) virPidFileDelete(LXC_STATE_DIR, def->name); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3baff19..0cd1ffd 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -440,6 +440,9 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml) VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (virSecurityManagerVerify(driver->securityManager, def) < 0) + goto cleanup; + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) goto cleanup; @@ -1425,7 +1428,21 @@ static int lxcMonitorClient(lxc_driver_t * driver, return -1; } - if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0) { + VIR_ERROR(_("Failed to set security context for monitor for %s"), + vm->def->name); + goto error; + } + + fd = socket(PF_UNIX, SOCK_STREAM, 0); + + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0) { + VIR_ERROR(_("Failed to clear security context for monitor for %s"), + vm->def->name); + goto error; + } + + if (fd < 0) { virReportSystemError(errno, "%s", _("Failed to create client socket")); goto error; @@ -1468,6 +1485,16 @@ static int lxcVmTerminate(lxc_driver_t *driver, return -1; } + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, false); + virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + /* Clear out dynamically assigned labels */ + if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + VIR_FREE(vm->def->seclabel.model); + VIR_FREE(vm->def->seclabel.label); + VIR_FREE(vm->def->seclabel.imagelabel); + } + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) == 0) { rc = virCgroupKillPainfully(group); if (rc < 0) { @@ -1598,6 +1625,12 @@ lxcBuildControllerCmd(lxc_driver_t *driver, virCommandAddArgFormat(cmd, "%d", ttyFDs[i]); virCommandPreserveFD(cmd, ttyFDs[i]); } + + if (driver->securityDriverName) { + virCommandAddArg(cmd, "--security"); + virCommandAddArg(cmd, driver->securityDriverName); + } + virCommandAddArg(cmd, "--handshake"); virCommandAddArgFormat(cmd, "%d", handshakefd); virCommandAddArg(cmd, "--background"); @@ -1792,6 +1825,24 @@ static int lxcVmStart(virConnectPtr conn, virReportOOMError(); goto cleanup; } + + /* If you are using a SecurityDriver with dynamic labelling, + then generate a security label for isolation */ + VIR_DEBUG("Generating domain security label (if required)"); + if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) + vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_NONE; + + if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) { + virDomainAuditSecurityLabel(vm, false); + goto cleanup; + } + virDomainAuditSecurityLabel(vm, true); + + VIR_DEBUG("Setting domain security labels"); + if (virSecurityManagerSetAllLabel(driver->securityManager, + vm->def, NULL) < 0) + goto cleanup; + for (i = 0 ; i < vm->def->nconsoles ; i++) ttyFDs[i] = -1; @@ -1947,6 +1998,16 @@ cleanup: if (rc != 0) { VIR_FORCE_CLOSE(priv->monitor); virDomainConfVMNWFilterTeardown(vm); + + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, false); + virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + /* Clear out dynamically assigned labels */ + if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + VIR_FREE(vm->def->seclabel.model); + VIR_FREE(vm->def->seclabel.label); + VIR_FREE(vm->def->seclabel.imagelabel); + } } for (i = 0 ; i < nttyFDs ; i++) VIR_FORCE_CLOSE(ttyFDs[i]); @@ -2071,6 +2132,9 @@ lxcDomainCreateAndStart(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (virSecurityManagerVerify(driver->securityManager, def) < 0) + goto cleanup; + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; @@ -2115,6 +2179,105 @@ cleanup: } +static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + memset(seclabel, 0, sizeof(*seclabel)); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainVirtTypeToString(vm->def->virtType)) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("unknown virt type in domain definition '%d'"), + vm->def->virtType); + goto cleanup; + } + + /* + * Theoretically, the pid can be replaced during this operation and + * return the label of a different process. If atomicity is needed, + * further validation will be required. + * + * Comment from Dan Berrange: + * + * Well the PID as stored in the virDomainObjPtr can't be changed + * because you've got a locked object. The OS level PID could have + * exited, though and in extreme circumstances have cycled through all + * PIDs back to ours. We could sanity check that our PID still exists + * after reading the label, by checking that our FD connecting to the + * LXC monitor hasn't seen SIGHUP/ERR on poll(). + */ + if (virDomainObjIsActive(vm)) { + if (virSecurityManagerGetProcessLabel(driver->securityManager, + vm->def, vm->pid, seclabel) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to get security label")); + goto cleanup; + } + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + lxcDriverUnlock(driver); + return ret; +} + +static int lxcNodeGetSecurityModel(virConnectPtr conn, + virSecurityModelPtr secmodel) +{ + lxc_driver_t *driver = conn->privateData; + char *p; + int ret = 0; + + lxcDriverLock(driver); + memset(secmodel, 0, sizeof(*secmodel)); + + /* NULL indicates no driver, which we treat as + * success, but simply return no data in *secmodel */ + if (driver->caps->host.secModel.model == NULL) + goto cleanup; + + p = driver->caps->host.secModel.model; + if (strlen(p) >= VIR_SECURITY_MODEL_BUFLEN-1) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("security model string exceeds max %d bytes"), + VIR_SECURITY_MODEL_BUFLEN-1); + ret = -1; + goto cleanup; + } + strcpy(secmodel->model, p); + + p = driver->caps->host.secModel.doi; + if (strlen(p) >= VIR_SECURITY_DOI_BUFLEN-1) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("security DOI string exceeds max %d bytes"), + VIR_SECURITY_DOI_BUFLEN-1); + ret = -1; + goto cleanup; + } + strcpy(secmodel->doi, p); + +cleanup: + lxcDriverUnlock(driver); + return ret; +} + + static int lxcDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback callback, @@ -2363,6 +2526,10 @@ lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) lxcMonitorEvent, vm, NULL)) < 0) goto error; + + if (virSecurityManagerReserveLabel(driver->securityManager, + vm->def, vm->pid) < 0) + goto error; } else { vm->def->id = -1; VIR_FORCE_CLOSE(priv->monitor); @@ -2379,6 +2546,27 @@ error: } +static int +lxcSecurityInit(lxc_driver_t *driver) +{ + virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName, + false, + driver->securityDefaultConfined, + driver->securityRequireConfined); + if (!mgr) + goto error; + + driver->securityManager = mgr; + + return 0; + +error: + VIR_ERROR(_("Failed to initialize security drivers")); + virSecurityManagerFree(mgr); + return -1; +} + + static int lxcStartup(int privileged) { char *ld; @@ -2439,7 +2627,10 @@ static int lxcStartup(int privileged) if (lxcLoadDriverConfig(lxc_driver) < 0) goto cleanup; - if ((lxc_driver->caps = lxcCapsInit()) == NULL) + if (lxcSecurityInit(lxc_driver) < 0) + goto cleanup; + + if ((lxc_driver->caps = lxcCapsInit(lxc_driver)) == NULL) goto cleanup; lxc_driver->caps->privateDataAllocFunc = lxcDomainObjPrivateAlloc; @@ -2531,6 +2722,7 @@ static int lxcShutdown(void) lxcProcessAutoDestroyShutdown(lxc_driver); virCapabilitiesFree(lxc_driver->caps); + virSecurityManagerFree(lxc_driver->securityManager); VIR_FREE(lxc_driver->configDir); VIR_FREE(lxc_driver->autostartDir); VIR_FREE(lxc_driver->stateDir); @@ -3859,6 +4051,8 @@ static virDriver lxcDriver = { .domainGetBlkioParameters = lxcDomainGetBlkioParameters, /* 0.9.8 */ .domainGetInfo = lxcDomainGetInfo, /* 0.4.2 */ .domainGetState = lxcDomainGetState, /* 0.9.2 */ + .domainGetSecurityLabel = lxcDomainGetSecurityLabel, /* 0.9.4 */ + .nodeGetSecurityModel = lxcNodeGetSecurityModel, /* 0.9.4 */ .domainGetXMLDesc = lxcDomainGetXMLDesc, /* 0.4.2 */ .listDefinedDomains = lxcListDefinedDomains, /* 0.4.2 */ .numOfDefinedDomains = lxcNumDefinedDomains, /* 0.4.2 */ diff --git a/src/lxc/test_libvirtd_lxc.aug b/src/lxc/test_libvirtd_lxc.aug index e757b82..f8544a0 100644 --- a/src/lxc/test_libvirtd_lxc.aug +++ b/src/lxc/test_libvirtd_lxc.aug @@ -13,6 +13,7 @@ module Test_libvirtd_lxc = # This is disabled by default, uncomment below to enable it. # log_with_libvirtd = 1 +security_driver = \"selinux\" " test Libvirtd_lxc.lns get conf = @@ -29,3 +30,4 @@ log_with_libvirtd = 1 { "#comment" = "This is disabled by default, uncomment below to enable it." } { "#comment" = "" } { "log_with_libvirtd" = "1" } +{ "security_driver" = "selinux" } -- 1.7.7.5

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; To allow the container to access /dev and /dev/pts when under sVirt, set an explicit mount option. Also set a max size on the /dev mount to prevent DOS on memory usage * src/lxc/lxc_container.c: Set /dev mount context * src/lxc/lxc_controller.c: Set /dev/pts mount context --- src/lxc/lxc_container.c | 75 +++++++++++++++++++++++++++++++++++---------- src/lxc/lxc_controller.c | 43 +++++++++++++++++++++++--- 2 files changed, 96 insertions(+), 22 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4df0b55..3c8e0e3 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -36,6 +36,10 @@ #include <unistd.h> #include <mntent.h> +#if HAVE_SELINUX +# include <selinux/selinux.h> +#endif + /* Yes, we want linux private one, for _syscall2() macro */ #include <linux/unistd.h> @@ -419,7 +423,6 @@ err: static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) { const struct { - bool onlyPivotRoot; bool needPrefix; const char *src; const char *dst; @@ -433,16 +436,19 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) * mount point in the main OS becomes readonly too which is not what * we want. Hence some things have two entries here. */ - { true, false, "devfs", "/dev", "tmpfs", "mode=755", MS_NOSUID }, - { false, false, "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, - { false, false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND }, - { false, false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, - { false, true, "/sys", "/sys", NULL, NULL, MS_BIND }, - { false, true, "/sys", "/sys", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, - { false, true, "/selinux", "/selinux", NULL, NULL, MS_BIND }, - { false, true, "/selinux", "/selinux", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, + { false, "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV }, + { false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND }, + { false, "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, + { true, "/sys", "/sys", NULL, NULL, MS_BIND }, + { true, "/sys", "/sys", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, + { true, "/selinux", "/selinux", NULL, NULL, MS_BIND }, + { true, "/selinux", "/selinux", NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY }, }; int i, rc = -1; + char *opts = NULL; +#if HAVE_SELINUX + security_context_t con; +#endif VIR_DEBUG("Mounting basic filesystems %s pivotRoot=%d", NULLSTR(srcprefix), pivotRoot); @@ -450,10 +456,8 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) char *src = NULL; const char *srcpath = NULL; - VIR_DEBUG("Consider %s onlyPivotRoot=%d", - mnts[i].src, mnts[i].onlyPivotRoot); - if (mnts[i].onlyPivotRoot && !pivotRoot) - continue; + VIR_DEBUG("Process %s -> %s", + mnts[i].src, mnts[i].dst); if (virFileMakePath(mnts[i].dst) < 0) { virReportSystemError(errno, @@ -474,8 +478,10 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) /* Skip if mount doesn't exist in source */ if ((srcpath[0] == '/') && - (access(srcpath, R_OK) < 0)) + (access(srcpath, R_OK) < 0)) { + VIR_FREE(src); continue; + } VIR_DEBUG("Mount %s on %s type=%s flags=%x, opts=%s", srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags, mnts[i].opts); @@ -489,15 +495,50 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) VIR_FREE(src); } + if (pivotRoot) { +#if HAVE_SELINUX + if (getfilecon("/", &con) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, "%s", + _("Failed to query file context on /")); + goto cleanup; + } else { +#endif + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ + if (virAsprintf(&opts, "mode=755,size=65536%%%s%s%s", + con ? ",context=\"" : "", + con ? (const char *)con : "", + con ? "\"" : "") < 0) { + virReportOOMError(); + goto cleanup; + } +#if HAVE_SELINUX + } +#endif + + VIR_DEBUG("Mount devfs on /dev type=tmpfs flags=%x, opts=%s", + MS_NOSUID, opts); + if (mount("devfs", "/dev", "tmpfs", MS_NOSUID, opts) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on %s type %s"), + "devfs", "/dev", "tmpfs"); + goto cleanup; + } + } + rc = 0; cleanup: VIR_DEBUG("rc=%d", rc); + VIR_FREE(opts); return rc; } -static int lxcContainerMountDevFS(virDomainFSDefPtr root) +static int lxcContainerMountFSDevPTS(virDomainFSDefPtr root) { char *devpts = NULL; int rc = -1; @@ -1069,8 +1110,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerMountBasicFS("/.oldroot", true) < 0) return -1; - /* Mounts /dev and /dev/pts */ - if (lxcContainerMountDevFS(root) < 0) + /* Mounts /dev/pts */ + if (lxcContainerMountFSDevPTS(root) < 0) return -1; /* Populates device nodes in /dev/ */ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 6bc54b7..edd9dca 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -52,6 +52,9 @@ # define NUMA_VERSION1_COMPATIBILITY 1 # include <numa.h> #endif +#if HAVE_SELINUX +# include <selinux/selinux.h> +#endif #include "virterror_internal.h" #include "logging.h" @@ -1339,6 +1342,10 @@ lxcControllerRun(virDomainDefPtr def, * marked as shared */ if (root) { +#if HAVE_SELINUX + security_context_t con; +#endif + char *opts; VIR_DEBUG("Setting up private /dev/pts"); if (!virFileExists(root->src)) { @@ -1373,16 +1380,42 @@ lxcControllerRun(virDomainDefPtr def, goto cleanup; } - /* XXX should we support gid=X for X!=5 for distros which use - * a different gid for tty? */ - VIR_DEBUG("Mounting 'devpts' on %s", devpts); - if (mount("devpts", devpts, "devpts", 0, - "newinstance,ptmxmode=0666,mode=0620,gid=5") < 0) { +#if HAVE_SELINUX + if (getfilecon(root->src, &con) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, + _("Failed to query file context on %s"), + root->src); + goto cleanup; + } else { +#endif + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ + /* XXX should we support gid=X for X!=5 for distros which use + * a different gid for tty? */ + if (virAsprintf(&opts, "newinstance,ptmxmode=0666,mode=0620,gid=5%s%s%s", + con ? ",context=\"" : "", + con ? (const char *)con : "", + con ? "\"" : "") < 0) { + virReportOOMError(); + goto cleanup; + } +#if HAVE_SELINUX + } +#endif + + VIR_DEBUG("Mount devpts on %s type=tmpfs flags=%x, opts=%s", + devpts, MS_NOSUID, opts); + if (mount("devpts", devpts, "devpts", MS_NOSUID, opts) < 0) { + VIR_FREE(opts); virReportSystemError(errno, _("Failed to mount devpts on %s"), devpts); goto cleanup; } + VIR_FREE(opts); if (access(devptmx, R_OK) < 0) { VIR_WARN("Kernel does not support private devpts, using shared devpts"); -- 1.7.7.5