Re: [libvirt] [patch v2 1/1] virt-aa-helper: Add support for smartcard host-certificates

On 12/5/19 12:11 PM, Arnaud Patard wrote: > When emulating smartcard with host certificates, qemu needs to > be able to read the certificates files. Add necessary code to > add the smartcard certificates file path to the apparmor profile. > > Passthrough support has been tested with spicevmc and remote-viewer. > > v2: > - Fix CodingStyle > - Add support for 'host' case. > - Add a comment to mention that the passthrough case doesn't need > some configuration > - Use one rule with '{,*}' instead of two rules. > > Signed-off-by: Arnaud Patard <apatard(a)hupstream.com&gt; > Index: libvirt/src/security/virt-aa-helper.c > =================================================================== > --- libvirt.orig/src/security/virt-aa-helper.c > +++ libvirt/src/security/virt-aa-helper.c > @@ -1271,6 +1271,39 @@ 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) { > + /* > + * Note: At time of writing, to get this working, qemu seccomp sandbox has > + * to be disabled or the host must be running QEMU with commit > + * 9a1565a03b79d80b236bc7cc2dbce52a2ef3a1b8. > + * It's possibly due to libcacard:vcard_emul_new_event_thread(), which calls > + * PR_CreateThread(), which calls {g,s}etpriority(). And resourcecontrol seccomp > + * filter forbids it (cf src/qemu/qemu_command.c which seems to always use > + * resourcecontrol=deny). > + */ This doesn't seem like the type of thing to track in a permanent code comment, nor a commit message, but as part of the email discussion. Otherwise, for the code because I don't have a test setup: Reviewed-by: Cole Robinson <crobinso(a)redhat.com&gt; If apparmor maintainers agree they can strip out of the comment so doesn't require a repost either way IMO