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

[Cedric Bosdonnat]

On Fri, 2016-03-11 at 20:07 +0000, Serge Hallyn wrote:
> [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.]

Well, that was so heavily changed with Jamie's comments I can't really
claim paternity of anything on that patch ;)
--
Cedric

> From: Cèdric Bosdonnat <cbosdonnat at suse.com>
> 
> 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;
>          }
>      }