[libvirt] [patch v2 1/1] virt-aa-helper: Add support for smartcard host-certificates
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@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"); + break; + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + virBufferAsprintf(&buf, " \"%s/{,*}\" rk,\n", sc_db); + break; + /* + * 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];
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@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). + */
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@redhat.com> If apparmor maintainers agree they can strip out of the comment so doesn't require a repost either way IMO - Cole
Cole Robinson <crobinso@redhat.com> writes: Hi,
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@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). + */
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@redhat.com>
If apparmor maintainers agree they can strip out of the comment so doesn't require a repost either way IMO
This patch doesn't seem to have been merged. Did it get lost or is it waiting for me to resubmit it without the comment ? Thanks, Arnaud
On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard <apatard@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@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@fedora~]# rpm -qf /etc/pki/nssdb/ nss-3.47.0-2.fc29.x86_64 [root@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@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
Christian Ehrhardt <christian.ehrhardt@canonical.com> writes:
On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard <apatard@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@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@fedora~]# rpm -qf /etc/pki/nssdb/ nss-3.47.0-2.fc29.x86_64 [root@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@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.
I don't remember seeing this /var/lib/nssdb path on my Debian. I don't even find the libnss3-nssdb package. Is it Ubuntu-specific and documented somewhere ? I can add this path, I've no problem with that. Nevertheless, I'm wondering that if it's really distribution specific (like done only on Ubuntu), wouldn't it be better to have the distribution patch libvirt to add this path ?
+ 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.
I remember testing the case with certificates inside /etc/pki/nssdb and not specifying the <database/> element at the early version of the patch.
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);
Not sure to understand. The certificates are inside a SQLite database, for instance: $ ls /etc/pki/nssdb cert9.db key4.db pkcs11.txt How your proposed change would be valid, since there's no specific file for the certificate (for instance /etc/pki/nssdb/cert1). Is apparmor able to filter the access of the content of a SQLite database ? Arnaud
On Fri, Jan 24, 2020 at 2:15 PM Arnaud Patard <apatard@hupstream.com> wrote:
Christian Ehrhardt <christian.ehrhardt@canonical.com> writes:
On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard <apatard@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@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@fedora~]# rpm -qf /etc/pki/nssdb/ nss-3.47.0-2.fc29.x86_64 [root@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@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.
I don't remember seeing this /var/lib/nssdb path on my Debian. I don't even find the libnss3-nssdb package. Is it Ubuntu-specific and documented somewhere ?
Hmm, I was just checking paths on a multitude of containers and which package it belongs. After you wondered I checked more in detail - the package and the /var... path are only in older releases. So please feel free to ignore that part of my question.
I can add this path, I've no problem with that. Nevertheless, I'm wondering that if it's really distribution specific (like done only on Ubuntu), wouldn't it be better to have the distribution patch libvirt to add this path ?
+ 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.
I remember testing the case with certificates inside /etc/pki/nssdb and not specifying the <database/> element at the early version of the patch.
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);
Not sure to understand. The certificates are inside a SQLite database, for instance: $ ls /etc/pki/nssdb cert9.db key4.db pkcs11.txt
How your proposed change would be valid, since there's no specific file
for the certificate (for instance /etc/pki/nssdb/cert1). Is apparmor able to filter the access of the content of a SQLite database ?
Hmm - yeah I thought that would be files, but you are right "...provide three NSS certificate names residing in a database..." The reason I was looking for an alternative is that I'm just generally a bit averse to most rules that end with *. Once we went as far as generating them (instead of adding them to the general template) you'd hope that we can make only explicit paths. But it seems if people need separation they have to do so via the paths and that is it. Thanks Arnaud for the clarifications that helped me to get the nss details I was missing. Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Arnaud
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
On Mon, Feb 10, 2020 at 7:36 AM Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
On Fri, Jan 24, 2020 at 2:15 PM Arnaud Patard <apatard@hupstream.com> wrote:
Christian Ehrhardt <christian.ehrhardt@canonical.com> writes:
On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard <apatard@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@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@fedora~]# rpm -qf /etc/pki/nssdb/ nss-3.47.0-2.fc29.x86_64 [root@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@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.
I don't remember seeing this /var/lib/nssdb path on my Debian. I don't even find the libnss3-nssdb package. Is it Ubuntu-specific and documented somewhere ?
Hmm, I was just checking paths on a multitude of containers and which package it belongs. After you wondered I checked more in detail - the package and the /var... path are only in older releases. So please feel free to ignore that part of my question.
I can add this path, I've no problem with that. Nevertheless, I'm wondering that if it's really distribution specific (like done only on Ubuntu), wouldn't it be better to have the distribution patch libvirt to add this path ?
+ 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.
I remember testing the case with certificates inside /etc/pki/nssdb and not specifying the <database/> element at the early version of the patch.
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);
Not sure to understand. The certificates are inside a SQLite database, for instance: $ ls /etc/pki/nssdb cert9.db key4.db pkcs11.txt
How your proposed change would be valid, since there's no specific file
for the certificate (for instance /etc/pki/nssdb/cert1). Is apparmor able to filter the access of the content of a SQLite database ?
Hmm - yeah I thought that would be files, but you are right "...provide three NSS certificate names residing in a database..."
The reason I was looking for an alternative is that I'm just generally a bit averse to most rules that end with *. Once we went as far as generating them (instead of adding them to the general template) you'd hope that we can make only explicit paths. But it seems if people need separation they have to do so via the paths and that is it.
Thanks Arnaud for the clarifications that helped me to get the nss details I was missing. Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Arnaud
We have a review and an ack, no negative feedback left and a test build + make check was fine. No 6.1.0 freeze active yet, so I'm pushing this to master with the Review/Ack statements added.
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
participants (3)
-
Arnaud Patard -
Christian Ehrhardt -
Cole Robinson