On Thu, 14 Nov 2019, Cole Robinson wrote:
On 10/24/19 4:57 AM, Arnaud Patard wrote:
> When emulating smartcard with host certificates, qemu needs to
> be able to read the certificates files, which is denied by apparmor.
> Add necessary code to add the smartcard certificates related directory
> to the apparmor profile.
>
> This code supports only this case smartcard 'host' and
'passthrough'
> settings are not supported, as I can't test them.
>
> Signed-off-by: Arnaud Patard <apatard(a)hupstream.com>
> Index: libvirt-5.0.0/src/security/virt-aa-helper.c
> ===================================================================
> --- libvirt-5.0.0.orig/src/security/virt-aa-helper.c
> +++ libvirt-5.0.0/src/security/virt-aa-helper.c
> @@ -1251,6 +1251,26 @@ get_files(vahControl * ctl)
> }
> }
>
> + for (i = 0; i < ctl->def->nsmartcards; i++) {
> + virDomainSmartcardDefPtr sc = ctl->def->smartcards[i];
> + virDomainSmartcardType sc_type = sc->type;
> + char *sc_db = (char *)VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
> + if (sc->data.cert.database)
> + sc_db = sc->data.cert.database;
> + switch(sc_type) {
Add a space after 'switch'. 'make syntax-check' will catch this. libvirt
style is typically to not indent the 'case' keyword either, but this
file is inconsistent on that front. With those fixed:
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
This matches what is done for the selinux driver AFAICT
CCing apparmor maintainers, I'll defer to them to commit
- Cole
> + case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> + break;
> + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> + virBufferAsprintf(&buf, " \"%s/\" rk,\n",
sc_db);
> + virBufferAsprintf(&buf, " \"%s/*\" rk,\n",
sc_db);
> + break;
I'll let other comment on the code changes, but these rules look ok. I
might suggest using this one line instead:
virBufferAsprintf(&buf, " \"%s/{,*}" rk,\n", sc_db);
Is it possible that the certificates might be in a lower directory? Ie,
is '**' warranted?
> + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> + break;
> + case VIR_DOMAIN_SMARTCARD_TYPE_LAST:
> + break;
> + }
> + }
It probably makes sense to add a code comment on why
VIR_DOMAIN_SMARTCARD_TYPE_HOST, VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH
and VIR_DOMAIN_SMARTCARD_TYPE_LAST aren't supported and ignored.
--
Jamie Strandboge |
http://www.canonical.com