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

[Christian Ehrhardt]

On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard <apatard at hupstream.com> 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 at hupstream.com>
> 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).
> +             */
> +            case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> +                virBufferAddLit(&buf, "  \"/etc/pki/nssdb/{,*}\" rk,\n");
>

That path matches the examples in libvirt/qemu and also the fedora nss
package
[root at fedora~]# rpm -qf /etc/pki/nssdb/
nss-3.47.0-2.fc29.x86_64
[root at fedora ~]# ll /etc/pki/nssdb/
total 8
-rw-r--r-- 1 root root 65536 Jan  6  2017 cert8.db
-rw-r--r-- 1 root root  9216 Jan  6  2017 cert9.db
-rw-r--r-- 1 root root 16384 Jan  6  2017 key3.db
-rw-r--r-- 1 root root 11264 Jan  6  2017 key4.db
-rw-r--r-- 1 root root   451 Oct 23 11:23 pkcs11.txt
-rw-r--r-- 1 root root 16384 Jan  6  2017 secmod.db

But on Debian/Ubuntu the paths are slightly different.
root at x:~# dpkg -L libnss3-nssdb
[...]
/var/lib/nssdb/key4.db
/var/lib/nssdb/cert9.db
/var/lib/nssdb/pkcs11.txt
/var/lib/nssdb/secmod.db

Therefore I'd ask you to add that path as well here.


+                break;
> +            case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> +                virBufferAsprintf(&buf, "  \"%s/{,*}\" rk,\n", sc_db);
> +                break;
>

>From https://libvirt.org/formatdomain.html#elementsSmartcard
"An additional sub-element <database> can specify the absolute path to an
alternate directory ... if not present, it defaults to /etc/pki/nssdb."
That is in "VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE".
Have you tested if sc_db is actually set to
"VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE"
in that case?
If it is e.g. undefined we need to check for that and add
"VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE"
instead.

Furthermore actually this lets us define:
  <smartcard mode='host-certificates'>
    <certificate>cert1</certificate>
    <certificate>cert2</certificate>
    <certificate>cert3</certificate>
    <database>/etc/pki/nssdb/</database>
  </smartcard>

There could be two guests using rather different cert[1-3] and there is no
need letting them cross read right?
So instead of
  virBufferAsprintf(&buf, "  \"%s/{,*}\" rk,\n", sc_db);
maybe something like this is safer:
iterate on certs-that-are-defined => cert
    virBufferAsprintf(&buf, "  \"%s/%s\" rk,\n", sc_db, cert);



> +            /*
> +             * Nothing to do for passthrough, as the smartcard
> +             * access is done through TCP or Spice
> +             */
> +            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];
>
>
>

-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200116/970f2bad/attachment-0001.htm>