[libvirt] [PATCH] security: AppArmor profile fixes for swtpm

The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file. --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 326cfaf52a..3d7cc32459 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,10 +1238,10 @@ get_files(vahControl * ctl) * directory, log, and PID files. */ virBufferAsprintf(&buf, - " \"%s/lib/libvirt/swtpm/%s/%s/**\" rw,\n", + " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", LOCALSTATEDIR, uuidstr, tpmpath); virBufferAsprintf(&buf, - " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" a,\n", + " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", -- 2.20.1

On Mon, 16 Sep 2019, Chris Coulson wrote:
The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file. --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 326cfaf52a..3d7cc32459 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,10 +1238,10 @@ get_files(vahControl * ctl) * directory, log, and PID files. */ virBufferAsprintf(&buf, - " \"%s/lib/libvirt/swtpm/%s/%s/**\" rw,\n", + " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", LOCALSTATEDIR, uuidstr, tpmpath); virBufferAsprintf(&buf, - " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" a,\n", + " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
LGTM. +1 to apply -- Jamie Strandboge | http://www.canonical.com

On Mon, Sep 16, 2019 at 03:27:25PM +0200, Chris Coulson wrote:
The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file.
Thanks for submitting this patch. As written on our governance page [1], particularly under contributors [2], this project requires providing a Signed-off-by as an agreement with the Developer Certificate of Origin [3]. If you can resend the patches with that change (easily with git commit --amend, very easily with a silly command-line [4]) I will merge this patch. Have a nice day, Martin [1] https://libvirt.org/governance.html [2] https://libvirt.org/governance.html#contributors [3] https://developercertificate.org/ [4] EDITOR='git interpret-trailers --in-place --trailer "Signed-off-by=..."' git commit --amend

On 9/16/19 9:27 AM, Chris Coulson wrote:
The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file.
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> I suppose you are not using the AppArmor policy that is part of the libvirt repo but Ubuntu has its own, right? I couldn't get it to work with the one in libvirt back then, so the AppArmor support was 'best effort'. Thanks a lot for fixing this! Please repost it with your Signed-off-by! Regards, Stefan
--- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 326cfaf52a..3d7cc32459 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,10 +1238,10 @@ get_files(vahControl * ctl) * directory, log, and PID files. */ virBufferAsprintf(&buf, - " \"%s/lib/libvirt/swtpm/%s/%s/**\" rw,\n", + " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", LOCALSTATEDIR, uuidstr, tpmpath); virBufferAsprintf(&buf, - " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" a,\n", + " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",

The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file. Signed-off-by: Chris Coulson <chris.coulson@canonical.com> --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 326cfaf52a..3d7cc32459 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,10 +1238,10 @@ get_files(vahControl * ctl) * directory, log, and PID files. */ virBufferAsprintf(&buf, - " \"%s/lib/libvirt/swtpm/%s/%s/**\" rw,\n", + " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", LOCALSTATEDIR, uuidstr, tpmpath); virBufferAsprintf(&buf, - " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" a,\n", + " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", -- 2.20.1

On 9/24/19 9:25 PM, Chris Coulson wrote:
The AppArmor profile generated by virt-aa-helper is too strict for swtpm. This change contains 2 small fixes: - Relax append access to swtpm's log file to permit write access instead. Append access is insufficient because the log is opened with O_CREAT. - Permit swtpm to acquire a lock on its lock file.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com> --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Congratulations on your first libvirt contribution. Michal
participants (5)
-
Chris Coulson
-
Jamie Strandboge
-
Martin Kletzander
-
Michal Privoznik
-
Stefan Berger