> On 5 Oct 2026, at 1:30 PM, Abhisek Panda <[email protected]> wrote: > > > >> On 2 Oct 2026, at 5:59 PM, Peter Krempa <[email protected]> wrote: >> >> !-------------------------------------------------------------------| >> CAUTION: External Email >> >> |-------------------------------------------------------------------! >> >> On Mon, Sep 07, 2026 at 08:55:20 +0000, Abhisek Panda wrote: >>> Libvirt by default attempt to use the TLS-PSK-enabled VM migration, >>> if the both source and destination support the tls-creds-psk object. >>> The source host adds the pre-shared key in the migration cookie if the >>> source supports the tls-creds-psk object. Upon parsing the migration >>> cookie, the destination host checks for the same capability and informs >>> the source host whether to use TLS X.509 or TLS PSK during VM migration >>> via the migration cookie. >>> >>> For a migration session, Libvirt generates a random key of the >>> specified length, and then stores the content, "qemu:<random key>", at >>> <tls_psk_state_dir>/$ID-$VMNAME/keys.psk on the source host. This is >>> because QEMU's tls-creds-psk object does not accept a raw key string >>> as a parameter, it only accepts a dir argument pointing to a directory >>> from which it can read the key file. Subsequently, it sends the key to >>> destination by embedding it within the migration cookie. The >>> destination's Libvirt extracts the key from the migration cookie. >>> Upon migration completion or any failure, both source and destination >>> Libvirt must delete the directory containing the session's keys.psk. >>> >>> Signed-off-by: Abhisek Panda <[email protected]> >>> --- >>> src/qemu/qemu_capabilities.c | 2 + >>> src/qemu/qemu_capabilities.h | 1 + >>> src/qemu/qemu_conf.c | 4 + >>> src/qemu/qemu_conf.h | 1 + >>> src/qemu/qemu_domain.c | 1 + >>> src/qemu/qemu_domain.h | 1 + >>> src/qemu/qemu_driver.c | 6 ++ >>> src/qemu/qemu_migration.c | 98 +++++++++++++++++++ >>> src/qemu/qemu_migration.h | 3 + >>> src/qemu/qemu_migration_cookie.c | 79 ++++++++++++++- >>> src/qemu/qemu_migration_cookie.h | 5 + >>> src/qemu/qemu_process.c | 3 + >>> .../caps_10.0.0_aarch64.xml | 1 + >>> .../caps_10.0.0_ppc64.xml | 1 + >>> .../caps_10.0.0_s390x.xml | 1 + >>> .../caps_10.0.0_x86_64+amdsev.xml | 1 + >>> .../caps_10.0.0_x86_64.xml | 1 + >>> .../caps_10.1.0_s390x.xml | 1 + >>> .../caps_10.1.0_x86_64+inteltdx.xml | 1 + >>> .../caps_10.1.0_x86_64.xml | 1 + >>> .../caps_10.2.0_aarch64.xml | 1 + >>> .../caps_10.2.0_x86_64+mshv.xml | 1 + >>> .../caps_10.2.0_x86_64.xml | 1 + >>> .../caps_11.0.0_aarch64.xml | 1 + >>> .../caps_11.0.0_s390x.xml | 1 + >>> .../caps_11.0.0_x86_64+sgx.xml | 1 + >>> .../caps_11.0.0_x86_64.xml | 1 + >>> .../caps_11.1.0_aarch64.xml | 1 + >>> .../caps_11.1.0_s390x.xml | 1 + >>> .../caps_11.1.0_x86_64.xml | 1 + >>> tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + >>> .../caps_7.2.0_x86_64+hvf.xml | 1 + >>> .../caps_7.2.0_x86_64.xml | 1 + >>> .../caps_8.0.0_x86_64.xml | 1 + >>> .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + >>> .../caps_8.1.0_x86_64.xml | 1 + >>> .../caps_8.2.0_aarch64.xml | 1 + >>> .../caps_8.2.0_armv7l.xml | 1 + >>> .../caps_8.2.0_loongarch64.xml | 1 + >>> .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + >>> .../caps_8.2.0_x86_64.xml | 1 + >>> .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 + >>> .../caps_9.0.0_x86_64.xml | 1 + >>> .../caps_9.1.0_riscv64.xml | 1 + >>> .../qemucapabilitiesdata/caps_9.1.0_s390x.xml | 1 + >>> .../caps_9.1.0_x86_64.xml | 1 + >>> .../caps_9.2.0_aarch64+hvf.xml | 1 + >>> .../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 1 + >>> .../caps_9.2.0_x86_64+amdsev.xml | 1 + >>> .../caps_9.2.0_x86_64.xml | 1 + >>> tests/qemumigrationcookiexmltest.c | 12 ++- >>> 51 files changed, 249 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >>> index bc2b0412dc..3db4710dee 100644 >>> --- a/src/qemu/qemu_capabilities.c >>> +++ b/src/qemu/qemu_capabilities.c >>> @@ -776,6 +776,7 @@ VIR_ENUM_IMPL(virQEMUCaps, >>> "arm-smmuv3.cmdqv", /* QEMU_CAPS_ARM_SMMUV3_CMDQV */ >>> "iothread.poll-weight", /* QEMU_CAPS_IOTHREAD_POLL_WEIGHT */ >>> "win-dmp.guest-aware", /* QEMU_CAPS_WIN_DMP_GUEST_AWARE */ >>> + "tls-creds-psk", /* QEMU_CAPS_OBJECT_TLS_CREDS_PSK */ >>> ); >>> >>> >>> @@ -1490,6 +1491,7 @@ struct virQEMUCapsStringFlags >>> virQEMUCapsObjectTypes[] = { >>> { "uefi-vars-sysbus", QEMU_CAPS_DEVICE_UEFI_VARS }, >>> { "monitor-qmp", QEMU_CAPS_OBJECT_MONITOR_QMP }, >>> { "arm-smmuv3", QEMU_CAPS_DEVICE_ARM_SMMUV3 }, >>> + { "tls-creds-psk", QEMU_CAPS_OBJECT_TLS_CREDS_PSK }, >> >> Any reason why the capability is introduced? I didn't see it mentioned >> in our previous conversation. >> >> In case if the support can be compiled out it's okay to add a >> capability, but new capabilities need to go into a separate commit >> because they make existing commits really massive and hard to backport. >> >> So if the capability is needed you'll have to split the hunks that add >> the capability and the detection into a separate commit which will need >> to preceed this one which makes use of it. > > The rationale behind adding this capability was to check if the source and > destination’s > QEMU supports PSK before attempting to perform PSK-based migration. Since > Libvirt’s > minimum supported QEMU version is now >= 7.2, and tls-creds-psk objects have > been > supported since QEMU 3.0, we can safely assume this capability exists on both > the source > and destination and drop this capability stuff. > >> >>> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h >>> index 28303fd8f0..1e432cdc41 100644 >>> --- a/src/qemu/qemu_capabilities.h >>> +++ b/src/qemu/qemu_capabilities.h >>> @@ -750,6 +750,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for >>> syntax-check */ >>> QEMU_CAPS_ARM_SMMUV3_CMDQV, /* arm-smmuv3.cmdqv */ >>> QEMU_CAPS_IOTHREAD_POLL_WEIGHT, /* -object iothread.poll-weight */ >>> QEMU_CAPS_WIN_DMP_GUEST_AWARE, /* 'win-dmp' is offered only to a guest >>> that can use it */ >>> + QEMU_CAPS_OBJECT_TLS_CREDS_PSK, /* -object tls-creds-psk */ >>> >>> QEMU_CAPS_LAST /* this must always be the last item */ >>> } virQEMUCapsFlags; >>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c >>> index cca7776b80..f5c5437c89 100644 >>> --- a/src/qemu/qemu_conf.c >>> +++ b/src/qemu/qemu_conf.c >>> @@ -168,6 +168,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool >>> privileged, >>> cfg->cacheDir = g_strdup_printf("%s/cache/qemu", root); >>> cfg->libDir = g_strdup_printf("%s/lib/qemu", root); >>> cfg->swtpmStorageDir = g_strdup_printf("%s/lib/swtpm", root); >>> + cfg->tlsPSKStateDir = g_strdup_printf("%s/run/psk", root); >>> >>> cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir); >>> cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir); >>> @@ -187,6 +188,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool >>> privileged, >>> cfg->stateDir = g_strdup_printf("%s/libvirt/qemu", RUNSTATEDIR); >>> cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir); >>> cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->stateDir); >>> + cfg->tlsPSKStateDir = g_strdup_printf("%s/psk", cfg->stateDir); >>> >>> cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", >>> LOCALSTATEDIR); >>> >>> @@ -214,6 +216,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool >>> privileged, >>> cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir); >>> cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir); >>> cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->stateDir); >>> + cfg->tlsPSKStateDir = g_strdup_printf("%s/psk", cfg->stateDir); >>> >>> cfg->configBaseDir = virGetUserConfigDirectory(); >>> >>> @@ -375,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj) >>> g_free(cfg->dbusStateDir); >>> g_free(cfg->rdpStateDir); >>> g_free(cfg->vncStateDir); >>> + g_free(cfg->tlsPSKStateDir); >>> >>> g_free(cfg->libDir); >>> g_free(cfg->cacheDir); >>> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h >>> index b700829342..2db3440514 100644 >>> --- a/src/qemu/qemu_conf.h >>> +++ b/src/qemu/qemu_conf.h >>> @@ -113,6 +113,7 @@ struct _virQEMUDriverConfig { >>> char *dbusStateDir; >>> char *rdpStateDir; >>> char *vncStateDir; >>> + char *tlsPSKStateDir; >>> /* These two directories are ones QEMU processes use (so must match >>> * the QEMU user/group */ >>> char *libDir; >>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >>> index a4e5f92840..f4d470553d 100644 >>> --- a/src/qemu/qemu_domain.c >>> +++ b/src/qemu/qemu_domain.c >>> @@ -1990,6 +1990,7 @@ qemuDomainObjPrivateFree(void *data) >>> virObjectUnref(priv->monConfig); >>> g_free(priv->lockState); >>> g_free(priv->origname); >>> + g_free(priv->migTLSPSK); >>> >>> virChrdevFree(priv->devs); >>> >>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h >>> index 23e99dc68c..053140f7ec 100644 >>> --- a/src/qemu/qemu_domain.h >>> +++ b/src/qemu/qemu_domain.h >>> @@ -139,6 +139,7 @@ struct _qemuDomainObjPrivate { >>> char *origname; >>> int nbdPort; /* Port used for migration with NBD */ >>> unsigned short migrationPort; >>> + char *migTLSPSK; /* Hex-encoded pre-shared key for TLS-PSK-enabled VM >>> migration session */ >>> unsigned short backupNBDPort; >>> int preMigrationState; >>> unsigned long long preMigrationMemlock; /* Original RLIMIT_MEMLOCK in >>> case >>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >>> index 8498568623..5548d6f7bd 100644 >>> --- a/src/qemu/qemu_driver.c >>> +++ b/src/qemu/qemu_driver.c >>> @@ -664,6 +664,12 @@ qemuStateInitialize(bool privileged, >>> cfg->vncStateDir); >>> goto error; >>> } >>> + if (virDirCreate(cfg->tlsPSKStateDir, 0700, cfg->user, cfg->group, >>> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) { >>> + virReportSystemError(errno, _("Failed to create TLS PSK state dir >>> %1$s"), >>> + cfg->tlsPSKStateDir); >>> + goto error; >> >> So, recently we've had a few reports about "security" bugs regarding >> directories owned by "qemu:qemu" which are filled by virtqemud with some >> objects. >> >> That makes me think two things: >> 1) does this need to be in a separate directory? We really should have >> a per-VM directory for all the per-instance helper files which will >> live in /run so that it's removed on reboot >> >> 2) the directory shouldn't be owned by qemu:qemu, but rather root:root >> for the system instance and have 0711 mode so that qemu can open the >> files but not create them. >> >> I know this is a more general thing, as our directory in /run/ is >> nonsensically split to e.g. /run/libvirt/qemu/channel/$VMID rather than >> have /run/libvirt/qemu/(possibly some subdir such as 'instance'/$ID' >> which will include all the files. >> >> For this series, what definitely applies is the ownership and >> permissions point. >> >> >> >>> + } >>> >>> qemu_driver->inhibitor = virInhibitorNew( >>> VIR_INHIBITOR_WHAT_SHUTDOWN, >>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >>> index 4a43ab83b0..b9971286fd 100644 >>> --- a/src/qemu/qemu_migration.c >>> +++ b/src/qemu/qemu_migration.c >>> @@ -59,6 +59,7 @@ >>> #include "virprocess.h" >>> #include "virdomainsnapshotobjlist.h" >>> #include "virutil.h" >>> +#include "virsecureerase.h" >>> >>> #define VIR_FROM_THIS VIR_FROM_QEMU >>> >>> @@ -1503,6 +1504,89 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef >>> *def) >>> } >>> >>> >>> +void >>> +qemuMigrationDeletePSKDir(virQEMUDriver *driver, virDomainObj *vm) >>> +{ >>> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); >>> + qemuDomainObjPrivate *priv = vm->privateData; >>> + g_autofree char *dir_path = NULL; >>> + g_autofree char *shortName = NULL; >>> + >>> + if (priv->migTLSPSK) { >>> + virSecureEraseString(priv->migTLSPSK); >>> + g_clear_pointer(&priv->migTLSPSK, g_free); >>> + } >>> + >>> + if (!(shortName = virDomainDefGetShortName(vm->def))) >> >> This failure ... >> >>> + return; >>> + >>> + dir_path = g_strdup_printf("%s/%s", cfg->tlsPSKStateDir, shortName); >>> + >>> + if (virFileIsDir(dir_path) && >>> + virFileDeleteTree(dir_path) < 0) >>> + VIR_WARN("Failed to delete the directory %s containing the >>> pre-shared keys for migration of domain %s", >>> + dir_path, vm->def->name); >> >> ... has the same implication. >> >> >>> +} >>> + >>> + >>> +static int >>> +qemuPersistTLSPSKHelper(int pskFD, >>> + const char *pskPath, >>> + const void *opaque) >>> +{ >>> + const char *key = opaque; >>> + >>> + if (safewrite(pskFD, "qemu:", 5) < 0 || >>> + safewrite(pskFD, key, strlen(key)) < 0) { >>> + virReportSystemError(errno, >>> + _("Unable to write the pre-shared key to file >>> '%1$s'"), >>> + pskPath); >>> + return -1; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> + >>> +static int >>> +qemuMigrationPersistPSK(virQEMUDriver *driver, virDomainObj *vm, const >>> char *tlsPSK) >>> +{ >>> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); >>> + g_autofree char *dir_path = NULL; >>> + g_autofree char *key_path = NULL; >>> + g_autofree char *shortName = NULL; >>> + >>> + if (!(shortName = virDomainDefGetShortName(vm->def))) { >>> + virReportError(VIR_ERR_INTERNAL_ERROR, >>> + _("Could not get the shortened domain name for >>> domain: %1$s"), >>> + vm->def->name); >>> + return -1; >>> + } >>> + >>> + dir_path = g_strdup_printf("%s/%s", cfg->tlsPSKStateDir, shortName); >>> + key_path = g_strdup_printf("%s/keys.psk", dir_path); >>> + >>> + if (virDirCreate(dir_path, 0700, cfg->user, cfg->group, >>> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) { >> >> Same here, the directory IMO doesn't need to be owned by qemu because >> qemu will never attempt to create a file here. It needs to be accessible >> by 'other' though. >> >> >>> + virReportSystemError(errno, >>> + _("Could not create the directory %1$s for >>> storing PSKs"), >>> + dir_path); >>> + goto error; >>> + } >>> + >>> + if (virFileRewrite(key_path, S_IRUSR, cfg->user, >>> + cfg->group, qemuPersistTLSPSKHelper, >>> + tlsPSK) < 0) >>> + goto error; >>> + >>> + return 0; >>> + >>> + error: >>> + qemuMigrationDeletePSKDir(driver, vm); >>> + return -1; >>> +} >>> + >>> + >>> static int >>> qemuDomainGetMigrationBlockers(virDomainObj *vm, >>> int asyncJob, >>> @@ -2722,6 +2806,9 @@ qemuMigrationSrcBeginXML(virDomainObj *vm, >>> if (priv->origCPU) >>> cookieFlags |= QEMU_MIGRATION_COOKIE_CPU; >>> >>> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_PSK)) >>> + cookieFlags |= QEMU_MIGRATION_COOKIE_TLS_PSK; >>> + >>> if (!(flags & VIR_MIGRATE_OFFLINE)) >>> cookieFlags |= QEMU_MIGRATION_COOKIE_CAPS; >>> >>> @@ -2738,6 +2825,9 @@ qemuMigrationSrcBeginXML(virDomainObj *vm, >>> cookieFlags) < 0) >>> return NULL; >>> >>> + if (mig->tlsPSK && qemuMigrationPersistPSK(driver, vm, mig->tlsPSK) < >>> 0) >>> + return NULL; >>> + >>> if (xmlin) { >>> g_autoptr(virDomainDef) def = NULL; >>> >>> @@ -4232,6 +4322,8 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, >>> privJob->stats.mig.downtime = privMigJob->stats.mig.downtime; >>> } >>> >>> + qemuMigrationDeletePSKDir(driver, vm); >>> + >>> if (flags & VIR_MIGRATE_OFFLINE) >>> return 0; >>> >>> @@ -5274,6 +5366,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, >>> >>> error: >>> virErrorPreserveLast(&orig_err); >>> + qemuMigrationDeletePSKDir(driver, vm); >>> >>> if (qemuDomainObjIsActive(vm)) { >>> int reason; >>> @@ -7029,6 +7122,8 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, >>> QEMU_MIGRATION_COOKIE_STATS) < 0) >>> VIR_WARN("Unable to encode migration cookie"); >>> >>> + qemuMigrationDeletePSKDir(driver, vm); >>> + >>> qemuMigrationDstComplete(driver, vm, inPostCopy, >>> VIR_ASYNC_JOB_MIGRATION_IN, vm->job); >>> >>> @@ -7039,6 +7134,8 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, >>> * overwrites it. */ >>> virErrorPreserveLast(&orig_err); >>> >>> + qemuMigrationDeletePSKDir(driver, vm); >>> + >>> if (qemuDomainObjIsActive(vm)) { >>> if (doKill) { >>> qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, >>> @@ -7197,6 +7294,7 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, >>> else >>> qemuMigrationSrcComplete(driver, vm, job); >>> >>> + qemuMigrationDeletePSKDir(driver, vm); >>> qemuMigrationJobFinish(vm); >>> >>> if (!virDomainObjIsActive(vm)) >>> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h >>> index 7e9410e1f7..ce15f8024b 100644 >>> --- a/src/qemu/qemu_migration.h >>> +++ b/src/qemu/qemu_migration.h >>> @@ -279,3 +279,6 @@ int >>> qemuMigrationAnyRefreshStatus(virDomainObj *vm, >>> virDomainAsyncJob asyncJob, >>> virDomainJobStatus *status); >>> + >>> +void >>> +qemuMigrationDeletePSKDir(virQEMUDriver *driver, virDomainObj *vm); >>> diff --git a/src/qemu/qemu_migration_cookie.c >>> b/src/qemu/qemu_migration_cookie.c >>> index 7311a8294b..0cf1ab9590 100644 >>> --- a/src/qemu/qemu_migration_cookie.c >>> +++ b/src/qemu/qemu_migration_cookie.c >>> @@ -20,6 +20,7 @@ >>> >>> #include <gnutls/gnutls.h> >>> #include <gnutls/x509.h> >>> +#include <inttypes.h> >>> >>> #include "locking/domain_lock.h" >>> #include "virerror.h" >>> @@ -27,6 +28,7 @@ >>> #include "virnetdevopenvswitch.h" >>> #include "virstring.h" >>> #include "virutil.h" >>> +#include "virsecureerase.h" >>> >>> #include "qemu_domain.h" >>> #include "qemu_migration_cookie.h" >>> @@ -52,6 +54,7 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag, >>> "allowReboot", >>> "capabilities", >>> "block-dirty-bitmaps", >>> + "psk", >>> ); >>> >>> >>> @@ -165,6 +168,9 @@ qemuMigrationCookieFree(qemuMigrationCookie *mig) >>> g_free(mig->name); >>> g_free(mig->lockState); >>> g_free(mig->lockDriver); >>> + if (mig->tlsPSK) >>> + virSecureEraseString(mig->tlsPSK); >>> + g_free(mig->tlsPSK); >>> g_clear_pointer(&mig->jobData, virDomainJobDataFree); >>> virCPUDefFree(mig->cpu); >>> qemuMigrationCookieCapsFree(mig->caps); >>> @@ -575,6 +581,51 @@ qemuMigrationCookieAddCaps(qemuMigrationCookie *mig, >>> } >>> >>> >>> +static int >>> +qemuMigrationCookieAddTLSPSK(qemuMigrationCookie *mig, >>> + virQEMUDriver *driver, >>> + virDomainObj *vm) >>> +{ >>> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); >>> + qemuDomainObjPrivate *priv = vm->privateData; >>> + gnutls_datum_t psk_key = {NULL, 0}; >>> + g_autofree char *key = NULL; >>> + size_t key_len; >>> + int ret; >>> + >>> + /* Generate the pre-shared key exactly once for a migration session*/ >>> + if (priv->migTLSPSK) { >>> + mig->tlsPSK = g_strdup(priv->migTLSPSK); >>> + mig->flags |= QEMU_MIGRATION_COOKIE_TLS_PSK; >>> + return 0; >>> + } >>> + >>> + ret = gnutls_key_generate(&psk_key, cfg->migrateTLSPSKLength); >>> + if (ret < 0) { >>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >>> + _("Generation of a pre-shared key failed")); >>> + return -1; >>> + } >>> + key_len = (psk_key.size*2) + 1; >>> + key = g_new0(char, key_len); >>> + >>> + ret = gnutls_hex_encode(&psk_key, key, &key_len); >>> + if (ret < 0) { >>> + gnutls_free(psk_key.data); >>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >>> + _("Hex encoding of a PSK key failed")); >>> + return -1; >>> + } >>> + >>> + priv->migTLSPSK = g_strdup(key); >>> + mig->tlsPSK = g_steal_pointer(&key); >>> + mig->flags |= QEMU_MIGRATION_COOKIE_TLS_PSK; >>> + >>> + gnutls_free(psk_key.data); >>> + return 0; >>> +} >>> + >>> + >>> static void >>> qemuMigrationCookieGraphicsXMLFormat(virBuffer *buf, >>> qemuMigrationCookieGraphics *grap) >>> @@ -890,6 +941,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriver *driver, >>> if (mig->flags & QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS) >>> qemuMigrationCookieBlockDirtyBitmapsFormat(buf, >>> mig->blockDirtyBitmaps); >>> >>> + if ((mig->flags & QEMU_MIGRATION_COOKIE_TLS_PSK) && mig->tlsPSK) >>> + virBufferAsprintf(buf, "<migration-key>%s</migration-key>\n", >>> mig->tlsPSK); >>> + >>> virBufferAdjustIndent(buf, -2); >>> virBufferAddLit(buf, "</qemu-migration>\n"); >>> return 0; >>> @@ -1396,6 +1450,12 @@ qemuMigrationCookieXMLParse(qemuMigrationCookie *mig, >>> qemuMigrationCookieBlockDirtyBitmapsParse(ctxt, mig) < 0) >>> return -1; >>> >>> + if (flags & QEMU_MIGRATION_COOKIE_TLS_PSK) { >>> + mig->tlsPSK = virXPathString("string(./migration-key[1])", ctxt); >>> + if (mig->tlsPSK) >>> + mig->flags |= QEMU_MIGRATION_COOKIE_TLS_PSK; >>> + } >>> + >>> return 0; >>> } >>> >>> @@ -1471,14 +1531,29 @@ qemuMigrationCookieFormat(qemuMigrationCookie *mig, >>> qemuMigrationCookieAddCaps(mig, dom, party) < 0) >>> return -1; >>> >>> + if (flags & QEMU_MIGRATION_COOKIE_TLS_PSK) { >>> + switch (party) { >>> + case QEMU_MIGRATION_SOURCE: >>> + if (qemuMigrationCookieAddTLSPSK(mig, driver, dom) < 0) >>> + return -1; >>> + break; >>> + case QEMU_MIGRATION_DESTINATION: >>> + if (!virQEMUCapsGet(priv->qemuCaps, >>> QEMU_CAPS_OBJECT_TLS_CREDS_PSK)) { >> >> IMO any sane build of qemu must have TLS enabled nowadays. I wonder if >> it even makes sense to have this capability. If tls can be compiled out, >> techincally it'd mean that there could be such a setup but IMO that >> would be still insane to have a unencrypted connection. >> >> I'd be okay just assuming that any qemu we support has TLS support. On >> the offchance that someone uses an insane build they will get an error, >> but I don't think I'd care for such setups. > > IIUC, Libvirt will automatically enforce TLS-PSK encryption for migrations > between > two newer Libvirt hosts, even if the user does not specify the > VIR_MIGRATE_TLS flag. > > Consequently, TLS-X509 would only be triggered during a forward migration > from an older > Libvirt host to a newer Libvirt destination (when VIR_MIGRATE_TLS is set). In > that scenario, > the older source wouldn't include the pre-shared key in the migration cookie, > naturally > bypassing the PSK pathway. > > I am assuming we do not need to account for newer-to-older migrations. > > I wanted to ask a follow up question regarding this design: > Given that with encrypted migration, there will be a performance impact to > the live migrations of the VMs, > due to auxiliary encrypt and decrypt operations at the source and > destination, and also due to a lack of > MSG_ZEROCOPY support for encrypted migrations in QEMU. Some users might be > running VM migration > within a cluster of trusted nodes, for which encrypted migration might not be > a performant solution. > Can we gate this behaviour of by-default enablement of PSK behind a > configuration parameter in qemu.conf. > This design is as follows: > 1. If the configuration parameter is set we do it as per our above discussed > design. > 2. If the configuration parameter is not set, then we attempt PSK if > VIR_MIGRATE_TLS flag > is specified. > > >> >> >>> + if (mig->tlsPSK) >>> + virSecureEraseString(mig->tlsPSK); >>> + mig->flags &= ~QEMU_MIGRATION_COOKIE_TLS_PSK; >>> + g_clear_pointer(&mig->tlsPSK, g_free); >>> + } >>> + break; >>> + } >>> + } >>> + >>> if (qemuMigrationCookieXMLFormat(driver, priv->qemuCaps, &buf, mig) < 0) >>> return -1; >>> >>> *cookieoutlen = virBufferUse(&buf) + 1; >>> *cookieout = virBufferContentAndReset(&buf); >>> >>> - VIR_DEBUG("cookielen=%d cookie=%s", *cookieoutlen, *cookieout); >>> - >>> return 0; >>> } >>> >>> diff --git a/src/qemu/qemu_migration_cookie.h >>> b/src/qemu/qemu_migration_cookie.h >>> index 254372234d..fd3b4c5a56 100644 >>> --- a/src/qemu/qemu_migration_cookie.h >>> +++ b/src/qemu/qemu_migration_cookie.h >>> @@ -35,6 +35,7 @@ typedef enum { >>> QEMU_MIGRATION_COOKIE_FLAG_ALLOW_REBOOT, >>> QEMU_MIGRATION_COOKIE_FLAG_CAPS, >>> QEMU_MIGRATION_COOKIE_FLAG_BLOCK_DIRTY_BITMAPS, >>> + QEMU_MIGRATION_COOKIE_FLAG_TLS_PSK, >>> >>> QEMU_MIGRATION_COOKIE_FLAG_LAST >>> } qemuMigrationCookieFlags; >>> @@ -53,6 +54,7 @@ typedef enum { >>> QEMU_MIGRATION_COOKIE_CPU = (1 << QEMU_MIGRATION_COOKIE_FLAG_CPU), >>> QEMU_MIGRATION_COOKIE_CAPS = (1 << QEMU_MIGRATION_COOKIE_FLAG_CAPS), >>> QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS = (1 << >>> QEMU_MIGRATION_COOKIE_FLAG_BLOCK_DIRTY_BITMAPS), >>> + QEMU_MIGRATION_COOKIE_TLS_PSK = (1 << >>> QEMU_MIGRATION_COOKIE_FLAG_TLS_PSK), >>> } qemuMigrationCookieFeatures; >>> >>> typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; >>> @@ -171,6 +173,9 @@ struct _qemuMigrationCookie { >>> >>> /* If flags & QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS */ >>> GSList *blockDirtyBitmaps; >>> + >>> + /* If flags & QEMU_MIGRATION_COOKIE_TLS_PSK */ >>> + char *tlsPSK; >>> };
Ping for comments. Thanks and warm regards Abhisek Panda
