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&gt; 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) {

+ 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; + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: + break; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i]; -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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&gt; > > 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&gt; > > 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.