Re: [libvirt] [PATCH 06/15] Move encryption lookup back into qemu driver file

Decouple the monitor code from the virDomainDefPtr structure by moving the disk encryption lookup code back into the qemu_driver.c file. Instead provide a function callback to the monitor code which can be invoked to retrieve encryption data as required. * src/qemu/qemu_driver.c: Add findDomainDiskEncryption, and findVolumeQcowPassphrase. Pass address of the method findVolumeQcowPassphrase into qemuMonitorOpen() * src/qemu/qemu_monitor.c: Associate a disk encryption function callback with the qemuMonitorPtr object. * src/qemu/qemu_monitor_text.c: Remove findDomainDiskEncryption and findVolumeQcowPassphrase. --- src/qemu/qemu_driver.c | 110 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 22 ++++++++ src/qemu/qemu_monitor.h | 20 ++++++++ src/qemu/qemu_monitor_text.c | 103 ++------------------------------------- 4 files changed, 157 insertions(+), 98 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e985543..30cc66e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -348,6 +348,113 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static virStorageEncryptionPtr +findDomainDiskEncryption(virConnectPtr conn, virDomainObjPtr vm, + const char *path) +{ + bool seen_volume; + int i; + + seen_volume = false; + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk; + + disk = vm->def->disks[i]; + if (disk->src != NULL && STREQ(disk->src, path)) { + seen_volume = true; + if (disk->encryption != NULL) + return disk->encryption; + } + } + if (seen_volume) + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, + _("missing <encryption> for volume %s"), path); + else + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected passphrase request for volume %s"), + path); + return NULL; +} + + +static int +findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virConnectPtr conn, + virDomainObjPtr vm, + const char *path, + char **secretRet, + size_t *secretLen) +{ + virStorageEncryptionPtr enc; + virSecretPtr secret; + char *passphrase; + unsigned char *data; + size_t size; + + if (!conn) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_SUPPORT, + "%s", _("cannot find secrets without a connection")); + return -1; + } + + if (conn->secretDriver == NULL || + conn->secretDriver->lookupByUUID == NULL || + conn->secretDriver->getValue == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", + _("secret storage not supported")); + return -1; + } + + enc = findDomainDiskEncryption(conn, vm, path); + if (enc == NULL) + return -1; + + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || + enc->nsecrets != 1 || + enc->secrets[0]->type != + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, + _("invalid <encryption> for volume %s"), path); + return -1; + } + + secret = conn->secretDriver->lookupByUUID(conn, + enc->secrets[0]->uuid); + if (secret == NULL) + return -1; + data = conn->secretDriver->getValue(secret, &size, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); + virUnrefSecret(secret); + if (data == NULL) + return -1; + + if (memchr(data, '\0', size) != NULL) { + memset(data, 0, size); + VIR_FREE(data); + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET, + _("format='qcow' passphrase for %s must not contain a " + "'\\0'"), path); + return -1; + } + + if (VIR_ALLOC_N(passphrase, size + 1) < 0) { + memset(data, 0, size); + VIR_FREE(data); + virReportOOMError(conn); + return -1; + } + memcpy(passphrase, data, size); + passphrase[size] = '\0'; + + memset(data, 0, size); + VIR_FREE(data); + + *secretRet = passphrase; + *secretLen = size; + + return 0; +} + static int qemuConnectMonitor(virDomainObjPtr vm, int reconnect) { @@ -358,6 +465,9 @@ qemuConnectMonitor(virDomainObjPtr vm, int reconnect) return -1; } + qemuMonitorRegisterDiskSecretLookup(priv->mon, + findVolumeQcowPassphrase); + return 0; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5f7e20c..352328d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -45,6 +45,7 @@ struct _qemuMonitor { virDomainObjPtr vm; qemuMonitorEOFNotify eofCB; + qemuMonitorDiskSecretLookup secretCB; }; /* Return -1 for error, 1 to continue reading and 0 for success */ @@ -322,6 +323,7 @@ qemuMonitorOpen(virDomainObjPtr vm, goto cleanup; } + return mon; cleanup: @@ -344,6 +346,13 @@ void qemuMonitorClose(qemuMonitorPtr mon) } +void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon, + qemuMonitorDiskSecretLookup secretCB) +{ + mon->secretCB = secretCB; +} + + int qemuMonitorWrite(qemuMonitorPtr mon, const char *data, size_t len) @@ -410,3 +419,16 @@ retry: } return 0; } + + +int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, + virConnectPtr conn, + const char *path, + char **secret, + size_t *secretLen) +{ + *secret = NULL; + *secretLen = 0; + + return mon->secretCB(mon, conn, mon->vm, path, secret, secretLen); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e863e20..6fb99a9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -36,12 +36,27 @@ typedef void (*qemuMonitorEOFNotify)(qemuMonitorPtr mon, virDomainObjPtr vm, int withError); +/* XXX we'd really like to avoid virCOnnectPtr here + * It is required so the callback can find the active + * secret driver. Need to change this to work like the + * security drivers do, to avoid this + */ +typedef int (*qemuMonitorDiskSecretLookup)(qemuMonitorPtr mon, + virConnectPtr conn, + virDomainObjPtr vm, + const char *path, + char **secret, + size_t *secretLen); + qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, int reconnect, qemuMonitorEOFNotify eofCB); void qemuMonitorClose(qemuMonitorPtr mon); +void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon, + qemuMonitorDiskSecretLookup secretCB); + int qemuMonitorWrite(qemuMonitorPtr mon, const char *data, size_t len); @@ -57,5 +72,10 @@ int qemuMonitorRead(qemuMonitorPtr mon, int qemuMonitorWaitForInput(qemuMonitorPtr mon); +int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, + virConnectPtr conn, + const char *path, + char **secret, + size_t *secretLen); #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index fa17971..a0146ae 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -334,101 +334,6 @@ qemuMonitorCommand(const virDomainObjPtr vm, } - -static virStorageEncryptionPtr -findDomainDiskEncryption(virConnectPtr conn, virDomainObjPtr vm, - const char *path) -{ - bool seen_volume; - int i; - - seen_volume = false; - for (i = 0; i < vm->def->ndisks; i++) { - virDomainDiskDefPtr disk; - - disk = vm->def->disks[i]; - if (disk->src != NULL && STREQ(disk->src, path)) { - seen_volume = true; - if (disk->encryption != NULL) - return disk->encryption; - } - } - if (seen_volume) - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, - _("missing <encryption> for volume %s"), path); - else - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unexpected passphrase request for volume %s"), - path); - return NULL; -} - -static char * -findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, - const char *path, size_t *passphrase_len) -{ - virStorageEncryptionPtr enc; - virSecretPtr secret; - char *passphrase; - unsigned char *data; - size_t size; - - if (conn->secretDriver == NULL || - conn->secretDriver->lookupByUUID == NULL || - conn->secretDriver->getValue == NULL) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", - _("secret storage not supported")); - return NULL; - } - - enc = findDomainDiskEncryption(conn, vm, path); - if (enc == NULL) - return NULL; - - if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || - enc->nsecrets != 1 || - enc->secrets[0]->type != - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, - _("invalid <encryption> for volume %s"), path); - return NULL; - } - - secret = conn->secretDriver->lookupByUUID(conn, - enc->secrets[0]->uuid); - if (secret == NULL) - return NULL; - data = conn->secretDriver->getValue(secret, &size, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); - virUnrefSecret(secret); - if (data == NULL) - return NULL; - - if (memchr(data, '\0', size) != NULL) { - memset(data, 0, size); - VIR_FREE(data); - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET, - _("format='qcow' passphrase for %s must not contain a " - "'\\0'"), path); - return NULL; - } - - if (VIR_ALLOC_N(passphrase, size + 1) < 0) { - memset(data, 0, size); - VIR_FREE(data); - virReportOOMError(conn); - return NULL; - } - memcpy(passphrase, data, size); - passphrase[size] = '\0'; - - memset(data, 0, size); - VIR_FREE(data); - - *passphrase_len = size; - return passphrase; -} - static int qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm, const char *buf, @@ -436,7 +341,8 @@ qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm, void *data) { virConnectPtr conn = data; - char *passphrase, *path; + qemuDomainObjPrivatePtr priv = vm->privateData; + char *passphrase = NULL, *path; const char *prompt_path; size_t path_len, passphrase_len = 0; int res; @@ -456,9 +362,10 @@ qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm, memcpy(path, prompt_path, path_len); path[path_len] = '\0'; - passphrase = findVolumeQcowPassphrase(conn, vm, path, &passphrase_len); + res = qemuMonitorGetDiskSecret(priv->mon, conn, path, + &passphrase, &passphrase_len); VIR_FREE(path); - if (passphrase == NULL) + if (res < 0) return -1; res = qemuMonitorSend(vm, passphrase, -1);