Merge ~lvoytek/ubuntu/+source/swtpm:aa-allow-libvirt-pid-access into ubuntu/+source/swtpm:ubuntu/devel

Proposed by Lena Voytek
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merged at revision: aca173688bf8445b1713ed75486dbbb170234fb9
Proposed branch: ~lvoytek/ubuntu/+source/swtpm:aa-allow-libvirt-pid-access
Merge into: ubuntu/+source/swtpm:ubuntu/devel
Diff against target: 28 lines (+8/-1)
2 files modified
debian/changelog (+7/-0)
debian/usr.bin.swtpm (+1/-1)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Andreas Hasenack Approve
Canonical Server Reporter Pending
Review via email: mp+432149@code.launchpad.net

Description of the change

PPA: https://launchpad.net/~lvoytek/+archive/ubuntu/swtpm-fix-apparmor-libvirt

Testing

# sudo apt update && sudo apt dist-upgrade -y
# sudo apt install virt-manager swtpm

Add PPA here to fix -> sudo add-apt-repository ppa:lvoytek/swtpm-fix-apparmor-libvirt && sudo apt update && sudo apt upgrade

Create a vm in virt-manager and on the last page

> Select "Customize configuration before install"
> Click Finish

> Click Add Hardware
> Select TPM with Model "TIS" and version 2.0

> Click "Begin Installation"

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote (last edit ):

We don't need a windows vm to reproduce the problem, any linux vm with tpm 2.0 added will work. Without the apparmor change, the same DENIED will appear in the logs and virt-manager will fail to start the VM, so I think the test case can be simplified to something like "launch ubuntu vm, add tpm 2.0 device".

While checking this, I noticed that it looks like swtpm (or libvirt) is leaking pid files in /run/libvirt/qemu/swtpm:
# ls -la /run/libvirt/qemu/swtpm/
total 24
drwxrwx--- 2 libvirt-qemu swtpm 160 Oct 24 19:20 .
drwxr-xr-x 5 root root 180 Oct 24 19:28 ..
-rw-r--r-- 1 root root 7 Oct 24 18:45 2-win11_22H2-swtpm.pid
-rw-r--r-- 1 root root 7 Oct 24 18:56 3-win11_22H2-swtpm.pid
-rw-r--r-- 1 root root 7 Oct 24 18:59 4-win11_22H2-swtpm.pid
-rw-r--r-- 1 root root 7 Oct 24 19:01 5-win11_22H2-swtpm.pid
-rw-r--r-- 1 root root 7 Oct 24 19:18 6-win11_22H2-swtpm.pid
-rw-r--r-- 1 root root 7 Oct 24 19:20 7-win11_22H2-swtpm.pid

Everytime I stop and start a VM with a tpm 2.0 device, I new pid file gets created, and when that vm is stopped, the pid file is not removed. I'm ready to file a separate bug for this, but maybe we could take a quick look to see why this is happening. I wanted to try some strace/opensnoop to see if someone is even trying to remove those pid files (and perhaps failing due to "reasons").

Note that the directory permissions libvirt-qemu:swtpm 775 would allow the swtpm user to remove the pid file, even though it's owned by root.

Revision history for this message
Lena Voytek (lvoytek) wrote :

I figured out the duplicating pid file issue, mp here: https://code.launchpad.net/~lvoytek/ubuntu/+source/libvirt/+git/libvirt/+merge/433390

Still having trouble figuring out why the pid files belong to root though. It may be best to go ahead with this apparmor change since its affecting a decent amount of people

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

Approvers: ahasenack, lvoytek
Uploaders: ahasenack
MP auto-approved

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I'll change the ubuntu release to lunar before upload

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Uploaded to lunar:
Uploading swtpm_0.6.3-0ubuntu5.dsc
Uploading swtpm_0.6.3-0ubuntu5.debian.tar.xz
Uploading swtpm_0.6.3-0ubuntu5_source.buildinfo
Uploading swtpm_0.6.3-0ubuntu5_source.changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 8091a44..67b3d4f 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+swtpm (0.6.3-0ubuntu5) kinetic; urgency=medium
7+
8+ * d/usr.bin.swtpm: Allow swtpm to also access /run/libvirt/qemu/swtpm/*.pid
9+ files that it does not own (LP: #1989100)
10+
11+ -- Lena Voytek <lena.voytek@canonical.com> Mon, 24 Oct 2022 10:52:06 -0700
12+
13 swtpm (0.6.3-0ubuntu4) kinetic; urgency=medium
14
15 * d/usr.bin.swtpm: Update apparmor profile to match swtpm upstream
16diff --git a/debian/usr.bin.swtpm b/debian/usr.bin.swtpm
17index 56702ad..bb29217 100644
18--- a/debian/usr.bin.swtpm
19+++ b/debian/usr.bin.swtpm
20@@ -32,7 +32,7 @@ profile swtpm /usr/bin/swtpm {
21 owner /var/lib/libvirt/swtpm/** rwk,
22 /run/libvirt/qemu/swtpm/*.sock rwk,
23 owner /var/log/swtpm/libvirt/qemu/*.log rwk,
24- owner /run/libvirt/qemu/swtpm/*.pid rwk,
25+ /run/libvirt/qemu/swtpm/*.pid rwk,
26 owner /dev/vtpmx rw,
27 owner /etc/nsswitch.conf r,
28 owner /var/lib/swtpm/** rwk,

Subscribers

People subscribed via source and target branches

to all changes: