security: Don't ignore errors when parsing DAC security labels

The DAC security driver silently ignored errors when parsing the DAC
label and used default values instead.

With a domain containing the following label definition:

<seclabel type='static' model='dac' relabel='yes'>
  <label>sdfklsdjlfjklsdjkl</label>
</seclabel>

the domain would start normaly but the disk images would be still owned
by root and no error was displayed.

This patch changes the behavior if the parsing of the label fails (note
that a not present label is not a failure and in this case the default
label should be used) the error isn't masked but is raised that causes
the domain start to fail with a descriptive error message:

virsh #  start tr
error: Failed to start domain tr
error: internal error invalid argument: failed to parse DAC seclabel
'sdfklsdjlfjklsdjkl' for domain 'tr'

I also changed the error code to "invalid argument" from "internal
error" and tweaked the various error messages to contain correct and
useful information.
This commit is contained in:
Peter Krempa 2012-09-18 12:20:41 +02:00
parent 740be0061a
commit ede89aab64

View File

@ -90,6 +90,7 @@ int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr)
return 0; return 0;
} }
/* returns 1 if label isn't found, 0 on success, -1 on error */
static static
int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
{ {
@ -98,20 +99,18 @@ int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
virSecurityLabelDefPtr seclabel; virSecurityLabelDefPtr seclabel;
if (def == NULL) if (def == NULL)
return -1; return 1;
seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (seclabel == NULL || seclabel->label == NULL) { if (seclabel == NULL || seclabel->label == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, VIR_DEBUG("DAC seclabel for domain '%s' wasn't found", def->name);
_("security label for DAC not found in domain %s"), return 1;
def->name);
return -1;
} }
if (seclabel->label && parseIds(seclabel->label, &uid, &gid)) { if (seclabel->label && parseIds(seclabel->label, &uid, &gid)) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INVALID_ARG,
_("failed to parse uid and gid for DAC " _("failed to parse DAC seclabel '%s' for domain '%s'"),
"security driver: %s"), seclabel->label); seclabel->label, def->name);
return -1; return -1;
} }
@ -127,19 +126,35 @@ static
int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr) uid_t *uidPtr, gid_t *gidPtr)
{ {
if (virSecurityDACParseIds(def, uidPtr, gidPtr) == 0) int ret;
return 0;
if (!def && !priv) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to determine default DAC seclabel "
"for an unknown object"));
return -1;
}
if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
return ret;
if (!priv) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("DAC seclabel couldn't be determined "
"for domain '%s'"), def->name);
return -1;
}
if (priv) {
if (uidPtr) if (uidPtr)
*uidPtr = priv->user; *uidPtr = priv->user;
if (gidPtr) if (gidPtr)
*gidPtr = priv->group; *gidPtr = priv->group;
return 0; return 0;
}
return -1;
} }
/* returns 1 if label isn't found, 0 on success, -1 on error */
static static
int virSecurityDACParseImageIds(virDomainDefPtr def, int virSecurityDACParseImageIds(virDomainDefPtr def,
uid_t *uidPtr, gid_t *gidPtr) uid_t *uidPtr, gid_t *gidPtr)
@ -149,21 +164,19 @@ int virSecurityDACParseImageIds(virDomainDefPtr def,
virSecurityLabelDefPtr seclabel; virSecurityLabelDefPtr seclabel;
if (def == NULL) if (def == NULL)
return -1; return 1;
seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (seclabel == NULL || seclabel->imagelabel == NULL) { if (seclabel == NULL || seclabel->imagelabel == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, VIR_DEBUG("DAC imagelabel for domain '%s' wasn't found", def->name);
_("security label for DAC not found in domain %s"), return 1;
def->name);
return -1;
} }
if (seclabel->imagelabel if (seclabel->imagelabel
&& parseIds(seclabel->imagelabel, &uid, &gid)) { && parseIds(seclabel->imagelabel, &uid, &gid)) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INVALID_ARG,
_("failed to parse uid and gid for DAC " _("failed to parse DAC imagelabel '%s' for domain '%s'"),
"security driver: %s"), seclabel->label); seclabel->imagelabel, def->name);
return -1; return -1;
} }
@ -179,17 +192,31 @@ static
int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr) uid_t *uidPtr, gid_t *gidPtr)
{ {
if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0) int ret;
return 0;
if (!def && !priv) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to determine default DAC imagelabel "
"for an unknown object"));
return -1;
}
if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0)
return ret;
if (!priv) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("DAC imagelabel couldn't be determined "
"for domain '%s'"), def->name);
return -1;
}
if (priv) {
if (uidPtr) if (uidPtr)
*uidPtr = priv->user; *uidPtr = priv->user;
if (gidPtr) if (gidPtr)
*gidPtr = priv->group; *gidPtr = priv->group;
return 0; return 0;
}
return -1;
} }