[PATCH 2/3] Introduce readonly without explicit deny-write

[Serge Hallyn <serge.hallyn@xxxxxxxxxx>]

[Sorry, the Ubuntu package suggests this came from Cèdric, although
I can't quite find this patch on the mailing list.  Those patches
which I did see from Cèdric did not have a Signed-off-by, so I didn't
add one for him.]

From: Cèdric Bosdonnat <cbosdonnat@xxxxxxxx>

Upstream changed get_files to unconditionally be "rw" to allow audit
files to be written. Still under discussion but the proposed approach
is to have a way of saying readonly but do not implicitely create a
write deny rule.

---
 Changelog: do not overwrite const memory, that leads to segv (serge)
---
 src/security/virt-aa-helper.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index b466626..34d08c8 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -785,12 +785,19 @@ get_definition(vahControl * ctl, const char *xmlStr)
     return rc;
 }
 
+/*
+ * The permissions allowed are apparmor valid permissions and 'R'. 'R' stands
+ * for read with no explicit deny rule.
+ */
 static int
-vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursive)
+vah_add_path(virBufferPtr buf, const char *path, const char *inperms, bool recursive)
 {
     char *tmp = NULL;
     int rc = -1;
     bool readonly = true;
+    bool explicit_deny_rule = true;
+    char *perms = strdupa(inperms);
+    char *sub = NULL;
 
     if (path == NULL)
         return rc;
@@ -815,8 +822,16 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
         return rc;
     }
 
-    if (strchr(perms, 'w') != NULL)
+    if (strchr(perms, 'w') != NULL) {
         readonly = false;
+        explicit_deny_rule = false;
+    }
+
+    if ((sub = strchr(perms, 'R')) != NULL) {
+        /* Don't write the invalid 'R' permission, replace with 'r' */
+        sub[0] = 'r';
+        explicit_deny_rule = false;
+    }
 
     rc = valid_path(tmp, readonly);
     if (rc != 0) {
@@ -831,7 +846,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
         tmp[strlen(tmp) - 1] = '\0';
 
     virBufferAsprintf(buf, "  \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms);
-    if (readonly) {
+    if (explicit_deny_rule) {
         virBufferAddLit(buf, "  # don't audit writes to readonly files\n");
         virBufferAsprintf(buf, "  deny \"%s%s\" w,\n", tmp, recursive ? "/**" : "");
     }
@@ -1130,7 +1145,7 @@ get_files(vahControl * ctl)
             /* We don't need to add deny rw rules for readonly mounts,
              * this can only lead to troubles when mounting / readonly.
              */
-            if (vah_add_path(&buf, fs->src, "rw", true) != 0)
+            if (vah_add_path(&buf, fs->src, fs->readonly ? "R" : "rw", true) != 0)
                 goto cleanup;
         }
     }
-- 
2.7.0

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