[PATCH] security: Don't ignore errors when parsing DAC security labels

[Peter Krempa <pkrempa@xxxxxxxxxx>]

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: invalid argument: failed to parse uid and gid for DAC security
driver: sdfklsdjlfjklsdjkl

I also changed the error code to "invalid argument" from "internal
error".
---
 src/security/security_dac.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 211fb37..c669496 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -140,6 +140,8 @@ int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
     return -1;
 }

+
+/* returns 1 if label isn't found, 0 on success, -1 on error */
 static
 int virSecurityDACParseImageIds(virDomainDefPtr def,
                                 uid_t *uidPtr, gid_t *gidPtr)
@@ -149,19 +151,19 @@ int virSecurityDACParseImageIds(virDomainDefPtr def,
     virSecurityLabelDefPtr seclabel;

     if (def == NULL)
-        return -1;
+        return 1;

     seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
     if (seclabel == NULL || seclabel->imagelabel == NULL) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("security label for DAC not found in domain %s"),
                        def->name);
-        return -1;
+        return 1;
     }

     if (seclabel->imagelabel
         && parseIds(seclabel->imagelabel, &uid, &gid)) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
+        virReportError(VIR_ERR_INVALID_ARG,
                        _("failed to parse uid and gid for DAC "
                          "security driver: %s"), seclabel->label);
         return -1;
@@ -179,8 +181,10 @@ static
 int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
                          uid_t *uidPtr, gid_t *gidPtr)
 {
-    if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0)
-        return 0;
+    int ret;
+
+    if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0)
+        return ret;

     if (priv) {
         if (uidPtr)
-- 
1.7.12

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list