[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Nov 03, 2009 at 02:50:00PM -0500, Daniel P. Berrange wrote:
> 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);

  ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]