Hi,
On Mon, Jan 18, 2016 at 01:45:08PM +0100, Cédric Bosdonnat wrote:
> Better fix replacing c726af2d: introducing an 'R' permission to
> add read rule, but no explicit deny write rule.
> ---
> src/security/virt-aa-helper.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index a2d7226..d0c5246 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -785,12 +785,18 @@ 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)
> {
> char *tmp = NULL;
> int rc = -1;
> bool readonly = true;
> + bool explicit_deny_rule = true;
> + char *sub = NULL;
>
> if (path == NULL)
> return rc;
> @@ -815,8 +821,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 it with 'r' */
> + sub[0] = 'r';
> + explicit_deny_rule = false;
> + }
>
> rc = valid_path(tmp, readonly);
> if (rc != 0) {
> @@ -831,7 +845,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 +1144,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;
> }
> }
this looks good to me but it would be great to have Jamie's input on
that.
Ceers,
-- Guido
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list