If swtpm binary fails to start after successful exec() (e.g. it
fails to initialize itself), the seclabels set in
qemuSecurityStartTPMEmulator() are not restored. This is due to
lacking qemuSecurityRestoreTPMLabels() call in the error path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The qemuSecurityCleanupTPMEmulator() function calls
virSecurityManagerRestoreTPMLabels() and thus the proper name is
qemuSecurityRestoreTPMLabels(). Rename it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, qemuSecurityCleanupTPMEmulator() returns nothing which
means a caller (well, there's only one - qemuExtTPMStop()) can't
produce a warning when restoring seclabels on TPM state failed.
True, qemuSecurityCleanupTPMEmulator() does report a warning
itself, but only in one specific error path.
Make the function return an integer, just like the rest of
qemuSecurity*Restore() functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Introduce a new backend type 'external' for connecting to a swtpm daemon
not managed by libvirtd.
Mostly in one commit, thanks to -Wswitch and the way we generate
capabilities.
https://bugzilla.redhat.com/show_bug.cgi?id=2063723
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Recently, the QEMU driver gained support for migration with TPM
state on a shared volume (e.g. NFS). As a part of that, the
destination side avoids setting seclabels on it to avoid cutting
off the source while it is still using it. Makes sense, except
for a wee bit: the secdriver API does a bit more - it also sets
label on the swtpm log file. And this one definitely needs to be
labeled (it lives under /var/log/swtpm/libvirt/qemu/..., i.e. not
on a shared volume).
Previously, qemuSecurityStartTPMEmulator() took care of that. But
during rework to shared volume migration, the code was changed so
now plain qemuSecurityCommandRun() would be run (i.e. no
relabelling).
But after previous commits, we can now chose whether the TPM
state should be relabelled or just the log file.
Fixes: 2e669ec789231d39e0d5f5f6a201d2a661b8070c
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2130192#c7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is basically just a continuation of the previous commit.
Now that the security driver APIs have a boolean flag that
controls setting/restoring seclabel of either both TPM state and
log files, or just the log file, propagate this boolean into
those APIs that start/stop swtpm emulator. For now, just pass
true. The juicy bits are soon to come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCommandSetSendBuffer() function consumes passed @buffer,
but takes it only as plain pointer. Switch to a double pointer to
make this obvious. This allows us then to drop all
g_steal_pointer() in callers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Inside of qemuTPMEmulatorBuildCommand() there are two calls to
qemuTPMSetupEncryption() which simply ignore returned error. This
is suboptimal because then we rely on swtpm binary reporting a
generic error (something among invalid command line arguments)
while an error reported by qemuTPMSetupEncryption() is more
specific.
However, since virCommandSetSendBuffer() only sets an error
inside of virCommand structure (the error is then reported in
virCommandRun()), we need to exempt its retval from error
checking. Thus, the signature of qemuTPMSetupEncryption() is
changed a bit so that -1/0 can be returned to indicate error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In recent commits migration of TPM on shared storage was
introduced. However, I've only complied it with gcc and thus did
not notice that clang build fails due to missing break; at the
end of some (empty) cases in switch() statements.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Never remove the TPM state on outgoing migration if the storage setup
has shared storage for the TPM state files. Also, do not do the security
cleanup on outgoing migration if shared storage is detected.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When using shared storage there is no need to apply security labels on the
storage since the files have to have been labeled already on the source
side and we must assume that the source and destination side have been
setup to use the same uid and gid for running swtpm as well as share the
same security labels. Whether the security labels can be used at all
depends on the shared storage and whether and how it supports them.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pass the --migration option to swtpm if swptm supports it (starting
with v0.8) and if the TPM's state is written on shared storage. If this
is the case apply the 'release-lock-outgoing' parameter with this
option and apply the 'incoming' parameter for incoming migration so that
swtpm releases the file lock on the source side when the state is migrated
and locks the file on the destination side when the state is received.
If a started swtpm instance is running with the necessary options of
migrating with share storage then remember this with a flag in the
virDomainTPMPrivateDef.
Report an error if swtpm does not support the --migration option and an
incoming migration across shared storage is requested.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Do not create storage if the TPM state files are on shared storage and
there's an incoming migration since in this case the storage directory
must already exist. Also do not run swtpm_setup in this case.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
New qemuTPMHasSharedStorage() function is introduced which
returns whether the swtpm state directory is on a shared
filesystem (e.g. NFS).
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags to qemuDomainUndefineFlags()
API and --tpm and --keep-tpm to 'virsh undefine'. Pass the
virDomainUndefineFlagsValues via qemuDomainRemoveInactive()
from qemuDomainUndefineFlags() all the way down to
qemuTPMEmulatorCleanupHost() and delete TPM storage there considering that
the UNDEFINE_TPM flag has priority over the persistent_state attribute
from the domain XML. Pass 0 in all other API call sites to
qemuDomainRemoveInactive() for now.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Just like the socket, remove the pidfile when TPM emulator is being stopped. In
order to make this a bit cleaner, try to remove it even if swtpm_ioctl does not
exist.
https://bugzilla.redhat.com/show_bug.cgi?id=2111301
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The virCommand module is specifically designed so that no caller
has to check for retval of individual virCommand*() APIs except
for virCommandRun() where the actual error is reported. Moreover,
virCommandNew*() use g_new0() to allocate memory and thus it's
not really possible for those APIs to return NULL. Which is why
they are even marked as ATTRIBUTE_NONNULL. But there are few
places where we do check the retval which is a dead code
effectively. Drop those checks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Historically, the tpm->data.emulator.activePcrBanks member was an
unsigned int but since it was used as a bitmap it was converted
to virBitmap type instead. Now, the virBitmap is allocated inside
of virDomainTPMDefParseXML() but only if <activePcrBanks/> was
found with at last one child element. Otherwise it stays NULL.
Fast forward to starting a domain with TPM 2.0 and no
<activePcrBanks/> configured. Eventually,
qemuTPMEmulatorBuildCommand() is called, which subsequently calls
qemuTPMEmulatorReconfigure() and finally
qemuTPMPcrBankBitmapToStr() passing the NULL value. Before
rewrite to virBitmap this function would return NULL for empty
activePcrBanks but now, well, now it crashes.
Fixes: 52c7c31c8038aa31d502f59a40e4fb4ba9f61113
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The _virDomainTPMDef structure has 'version' member, which is a
bit misplaced. It's only emulator type of TPM that can have a
version, even our documentation says so:
``version``
The ``version`` attribute indicates the version of the TPM. This attribute
only works with the ``emulator`` backend. The following versions are
supported:
Therefore, move the member into that part of union that's
covering emulated TPM devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In qemu_extdevice.c lives code that handles helper daemons that
are required for some types of devices (e.g. virtiofsd,
vhost-user-gpu, swtpm, etc.). These devices have their own
handling code in separate files, with only a very basic functions
exposed (e.g. for starting/stopping helper process, placing it
into given CGroup, etc.). And these functions all work over a
single instance of device (virDomainVideoDef *, virDomainFSDef *,
etc.), except for TPM handling code which takes virDomainDef *
and iterates over it inside its module.
Remove this oddness and make qemuExtTPM*() functions look closer
to the rest of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When vTPM is secured via virSecret libvirt passes the secret
value via an FD when swtpm is started (arguments --key and
--migration-key). The writing of the secret into the FDs is
handled via virCommand, specifically qemu_tpm calls
virCommandSetSendBuffer()) and then virCommandRunAsync() spawns a
thread to handle writing into the FD via
virCommandDoAsyncIOHelper. But the thread is not created unless
VIR_EXEC_ASYNC_IO flag is set, which it isn't. In order to fix
it, virCommandDoAsyncIO() must be called.
The credit goes to Marc-André Lureau
<marcandre.lureau@redhat.com> who has done all the debugging and
proposed fix in the bugzilla.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2064115
Fixes: a9c500d2b50c5c041a1bb6ae9724402cf1cec8fe
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This directory contains runtime state, not persistent state.
The latter goes into swtpmStorageDir.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When looping over TPM devices for a domain, we can avoid calling
this function for each iteration and call it once per domain
instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Using the word "create" can give users the impression that disk
operations will be performed, when in reality all these functions
do is string formatting.
Follow the naming convention established by virBuildPath(),
virFileBuildPath() and virPidFileBuildPath().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This leaves qemuExtTPMCleanupHost() to only deal with looping
over TPM devices, same as other qemuExtTPMDoThing() functions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This leaves qemuExtTPMSetupCgroup() to only deal with looping
over TPM devices, same as other qemuExtTPMDoThing() functions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Other functions that operate on a single TPM emulator follow
the qemuTPMEmulatorDoThing() naming convention.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
in containers) and any attempt to stat(2) or readlink(2) the file will
result in 'permission denied' error if the calling process does not have
CAP_SYS_PTRACE capability. According to proc(5) manpage:
Permission to dereference or read (readlink(2)) this symbolic link is
governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
ptrace(2).
The binary validation in virPidFileReadPathIfAlive may fail with EACCES.
Therefore instead do only the check that the pidfile is locked by the
correct process. To ensure this is always the case the daemonization and
pidfile handling of the swtpm command is now controlled by libvirt.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Every other exported API from virtpm.h will internally call
virTPMEmulatorInit, so there is no reason for this initializer
to be exported on its own.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'virDomainChrSourceDef' contains private data so 'virDomainChrSourceDefNew'
must be used to allocate it. 'virDomainTPMDef' was using it directly
which won't work with the chardev helper functions.
Convert it to a pointer to properly allocate private data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
1) 'activePcrBanksStr' is not initialized:
../../../libvirt/src/qemu/qemu_tpm.c: In function ‘qemuExtTPMStart’:
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: ‘activePcrBanksStr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
28 | g_free (*pp);
| ^~~~~~~~~~~~
../../../libvirt/src/qemu/qemu_tpm.c:613:22: note: ‘activePcrBanksStr’ was declared here
613 | g_autofree char *activePcrBanksStr;
| ^~~~~~~~~~~~~~~~~
2) 'pwdfile_fd' is unused:
../../../libvirt/src/qemu/qemu_tpm.c:615:19: error: unused variable 'pwdfile_fd' [-Werror,-Wunused-variable]
VIR_AUTOCLOSE pwdfile_fd = -1;
Fixes: a5bbe1a8b63
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Extend the TPM backend XML with a node 'active_pcr_banks' that allows a
user to specify the PCR banks to activate before starting a VM. Valid
choices for PCR banks are sha1, sha256, sha384 and sha512. When the XML
node is provided, the set of active PCR banks is 'enforced' by running
swtpm_setup before every start of the VM. The activation requires that
swtpm_setup v0.7 or later is installed and may not have any effect
otherwise.
<tpm model='tpm-tis'>
<backend type='emulator' version='2.0'>
<active_pcr_banks>
<sha256/>
<sha384/>
</active_pcr_banks>
</backend>
</tpm>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the code that adds encryption options for the swtpm_setup command
line into its own function.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When 'swtpm_setup --print-capabilities' shows the 'tpm12-not-need-root'
flag, then it is possible to create certificates for the TPM 1.2 also
in non-privileged mode since swtpm_setup doesn't need tcsd anymore.
Check for this flag and create the certificates if this flag is found.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Using swtpm v0.7.0 we can run swtpm_setup to create default config files
for swtpm_setup and swtpm-localca in session mode. Now a user can start
a VM with an attached TPM without having to run this program on the
command line before. This program needs to run once.
This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Launch swtpm(8) with the --terminate switch, which guarantees that
the daemon will shut itself down when QEMU dies (current behavior).
We had so far been getting this "for free" (i.e. without --terminate)
due to a defect in upstream's connection handling logic [1], on which
libvirt should not rely since it will eventually be fixed. Adding
--terminate preserves and guarantees the current behavior.
[1] https://github.com/stefanberger/swtpm/pull/509
Signed-off-by: Nick Chevsky <nchevsky@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When fetching the value of a private secret, we need to use an elevated
identity otherwise the secret driver will deny access.
When using the modular daemons, the elevated identity needs to be active
before the secret driver connection is opened, and it will apply to all
APIs calls made on that conncetion.
When using the monolithic daemon, the identity at time of opening the
connection is ignored, and the elevated identity needs to be active
precisely at the time the virSecretGetValue API call is made.
After acquiring the secret value, the elevated identity should be
cleared.
This sounds complex, but is fairly straightfoward with the automatic
cleanup callbacks.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
