[libvirt] [PATCH] security: chown/label bidrectional and unidirectional fifos
Laine Stump
laine at laine.org
Tue Sep 27 18:43:28 UTC 2011
Sigh. I just looked at my bugzilla mail backlog and saw that this
morning Dan Berrange voted for the simpler method of fixing this, but I
didn't notice it before implementing this patch.
I'll be back shortly with a v2 that does it the other way.
On 09/27/2011 02:36 PM, Laine Stump wrote:
> This patch fixes the regression with using named pipes for qemu serial
> devices noted in:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=740478
>
> The problem was that, while new code in libvirt looks for a single
> bidirectional fifo of the name given in the config, then relabels that
> and continues without looking for / relabelling the two unidirectional
> fifos named ${name}.in and ${name}.out, qemu looks in the opposite
> order. So if the user had naively created all three fifos, libvirt
> would relabel the bidirectional fifo to allow qemu access, but qemu
> would attempt to use the two unidirectional fifos and fail (because it
> didn't have proper permissions/rights).
>
> The solution implemented here is to always look for and chown/relabel
> both types of fifos, then let qemu decide which one it wants to use
> (in the unusual case that both are present). If one of the types is
> present, libvirt will silently ignore when the other type is missing
> (since that will be the most common case), but if neither type is
> present, there will be an error logged about failing to relabel/chown
> one of the bidirectional pipes. (Likewise, if one direction of the
> unidirectional pipes is present but the other is missing, this will
> also result in an error log).
>
> (Note commit d37c6a3a (which first appeared in libvirt-0.9.2) added
> the code that checked for a bidirectional fifo. Prior to that commit,
> bidirectional fifos fdor serial devices didn't work unless the fifo
> was pre-owned/labelled such that qemu could access it.)
> ---
> src/security/security_dac.c | 39 ++++++++++++++++++++++++++++++---------
> src/security/security_selinux.c | 39 ++++++++++++++++++++++++++++++---------
> 2 files changed, 60 insertions(+), 18 deletions(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index af02236..f97d2d6 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -397,6 +397,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
> {
> virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> char *in = NULL, *out = NULL;
> + bool found_bipipe = false;
> int ret = -1;
>
> switch (dev->type) {
> @@ -406,19 +407,28 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
> break;
>
> case VIR_DOMAIN_CHR_TYPE_PIPE:
> + /* look for / chown both bidirectional pipe and dual uni-directional
> + * pipes if found; let the hypervisor decide which to use.
> + */
> if (virFileExists(dev->data.file.path)) {
> + found_bipipe = true;
> if (virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group)< 0)
> goto done;
> - } else {
> - if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
> - (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
> - virReportOOMError();
> - goto done;
> - }
> - if ((virSecurityDACSetOwnership(in, priv->user, priv->group)< 0) ||
> - (virSecurityDACSetOwnership(out, priv->user, priv->group)< 0))
> - goto done;
> }
> + if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
> + (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
> + virReportOOMError();
> + goto done;
> + }
> + if (found_bipipe&&
> + !(virFileExists(in) || virFileExists(out))) {
> + ret = 0;
> + goto done;
> + }
> + if ((virSecurityDACSetOwnership(in, priv->user, priv->group)< 0) ||
> + (virSecurityDACSetOwnership(out, priv->user, priv->group)< 0))
> + goto done;
> +
> ret = 0;
> break;
>
> @@ -438,6 +448,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> virDomainChrSourceDefPtr dev)
> {
> char *in = NULL, *out = NULL;
> + bool found_bipipe = false;
> int ret = -1;
>
> switch (dev->type) {
> @@ -447,11 +458,21 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> break;
>
> case VIR_DOMAIN_CHR_TYPE_PIPE:
> + if (virFileExists(dev->data.file.path)) {
> + found_bipipe = true;
> + if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path)< 0)
> + goto done;
> + }
> if ((virAsprintf(&out, "%s.out", dev->data.file.path)< 0) ||
> (virAsprintf(&in, "%s.in", dev->data.file.path)< 0)) {
> virReportOOMError();
> goto done;
> }
> + if (found_bipipe&&
> + !(virFileExists(in) || virFileExists(out))) {
> + ret = 0;
> + goto done;
> + }
> if ((virSecurityDACRestoreSecurityFileLabel(out)< 0) ||
> (virSecurityDACRestoreSecurityFileLabel(in)< 0))
> goto done;
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 0807a34..e7a18a6 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -794,6 +794,7 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm,
> {
> const virSecurityLabelDefPtr secdef =&vm->def->seclabel;
> char *in = NULL, *out = NULL;
> + bool found_bipipe = false;
> int ret = -1;
>
> if (secdef->norelabel)
> @@ -806,19 +807,28 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm,
> break;
>
> case VIR_DOMAIN_CHR_TYPE_PIPE:
> + /* look for / chown both bidirectional pipe and dual uni-directional
> + * pipes if found; let the hypervisor decide which to use.
> + */
> if (virFileExists(dev->data.file.path)) {
> + found_bipipe = true;
> if (SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel)< 0)
> goto done;
> - } else {
> - if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
> - (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
> - virReportOOMError();
> - goto done;
> - }
> - if ((SELinuxSetFilecon(in, secdef->imagelabel)< 0) ||
> - (SELinuxSetFilecon(out, secdef->imagelabel)< 0))
> - goto done;
> }
> + if ((virAsprintf(&in, "%s.in", dev->data.file.path)< 0) ||
> + (virAsprintf(&out, "%s.out", dev->data.file.path)< 0)) {
> + virReportOOMError();
> + goto done;
> + }
> + if (found_bipipe&&
> + !(virFileExists(in) || virFileExists(out))) {
> + ret = 0;
> + goto done;
> + }
> + if ((SELinuxSetFilecon(in, secdef->imagelabel)< 0) ||
> + (SELinuxSetFilecon(out, secdef->imagelabel)< 0))
> + goto done;
> +
> ret = 0;
> break;
>
> @@ -840,6 +850,7 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm,
> {
> const virSecurityLabelDefPtr secdef =&vm->def->seclabel;
> char *in = NULL, *out = NULL;
> + bool found_bipipe = false;
> int ret = -1;
>
> if (secdef->norelabel)
> @@ -853,11 +864,21 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm,
> ret = 0;
> break;
> case VIR_DOMAIN_CHR_TYPE_PIPE:
> + if (virFileExists(dev->data.file.path)) {
> + found_bipipe = true;
> + if (SELinuxRestoreSecurityFileLabel(dev->data.file.path)< 0)
> + goto done;
> + }
> if ((virAsprintf(&out, "%s.out", dev->data.file.path)< 0) ||
> (virAsprintf(&in, "%s.in", dev->data.file.path)< 0)) {
> virReportOOMError();
> goto done;
> }
> + if (found_bipipe&&
> + !(virFileExists(in) || virFileExists(out))) {
> + ret = 0;
> + goto done;
> + }
> if ((SELinuxRestoreSecurityFileLabel(out)< 0) ||
> (SELinuxRestoreSecurityFileLabel(in)< 0))
> goto done;
More information about the libvir-list
mailing list