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

Re: [libvirt] [PATCH 4/9] Add support for encrypted (qcow) volume creation.



On Tue, Jul 21, 2009 at 01:12:00PM +0200, Miloslav Trma?? wrote:
> Supports only virStorageVolCreateXML, not virStorageVolCreateXMLFrom.
> 
> Curiously, qemu-img does not need the passphrase for anything to create
> an encrypted volume.  This implementation is sufficient for the qcow2
> format, and for other formats when all encryption parameters are
> pre-specified.
> 
> The qcow2 passphrase is stored in libvirt only during the volume
> creation operation, and automatically deleted afterwards.  Other users
> might be able to query the volume properties while the volume is being
> created, but virStorageVolGetXMLDesc() won't reveal the passphrase to
> them.
> 
> An earlier patch description has mentioned the possibility of a libvirt
> user specifying only the encryption format (or not even the format),
> letting libvirt to choose the parameters and return them.  This would
> require libvirt interface extensions, and it is not supported by this
> patch.
> ---
>  src/storage_backend.c         |   41 +++++++++++++++++++++++++++++++++++++++--
>  src/storage_backend_disk.c    |    7 +++++++
>  src/storage_backend_fs.c      |    7 +++++++
>  src/storage_backend_logical.c |    7 +++++++
>  src/storage_driver.c          |    4 ++++
>  5 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage_backend.c b/src/storage_backend.c
> index 1664804..42c59ad 100644
> --- a/src/storage_backend.c
> +++ b/src/storage_backend.c
> @@ -246,6 +246,13 @@ virStorageBackendCreateRaw(virConnectPtr conn,
>      unsigned long long remain;
>      char *buf = NULL;
>  
> +    if (vol->encryption != NULL) {
> +        virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                              "%s", _("storage pool does not support encrypted "
> +                                      "volumes"));
> +        return -1;
> +    }
> +
>      if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL,
>                     vol->target.perms.mode)) < 0) {
>          virReportSystemError(conn, errno,
> @@ -346,15 +353,17 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>                              NULL;
>  
>      const char **imgargv;
> +    /* The extra NULL field is for indicating encryption (-e). */
>      const char *imgargvnormal[] = {
>          NULL, "create",
>          "-f", type,
>          vol->target.path,
>          size,
>          NULL,
> +        NULL
>      };
>      /* Extra NULL fields are for including "backingType" when using
> -     * kvm-img. It's -F backingType
> +     * kvm-img (-F backingType), and for indicating encryption (-e).
>       */
>      const char *imgargvbacking[] = {
>          NULL, "create",
> @@ -364,6 +373,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>          size,
>          NULL,
>          NULL,
> +        NULL,
>          NULL
>      };
>      const char *convargv[] = {
> @@ -417,6 +427,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>          }
>      }
>  
> +    if (vol->encryption != NULL) {
> +        if (vol->encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) {
> +            virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                                  _("unsupported volume encryption format %d"),
> +                                  vol->encryption->format);
> +            return -1;
> +        }
> +        if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW &&
> +            vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) {
> +            virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                                  _("qcow volume encryption unsupported with "
> +                                    "volume format %s"), type);
> +            return -1;
> +        }
> +    }
> +
>      if ((create_tool = virFindFileInPath("kvm-img")) != NULL)
>          use_kvmimg = 1;
>      else if ((create_tool = virFindFileInPath("qemu-img")) != NULL)
> @@ -437,11 +463,16 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>              imgargvbacking[7] = backingType;
>              imgargvbacking[8] = vol->target.path;
>              imgargvbacking[9] = size;
> -        }
> +            if (vol->encryption != NULL)
> +                imgargvbacking[10] = "-e";
> +        } else if (vol->encryption != NULL)
> +            imgargvbacking[8] = "-e";
>          imgargv = imgargvbacking;
>      } else {
>          imgargvnormal[0] = create_tool;
>          imgargv = imgargvnormal;
> +        if (vol->encryption != NULL)
> +            imgargv[6] = "-e";
>      }
>  
>  
> @@ -490,6 +521,12 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn,
>                                "qcow-create"));
>          return -1;
>      }
> +    if (vol->encryption != NULL) {
> +        virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                              "%s", _("encrypted volumes not supported with "
> +                                      "qcow-create"));
> +        return -1;
> +    }
>  
>      /* Size in MB - yes different units to qemu-img :-( */
>      snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024);
> diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c
> index ae2acae..8f4f0ed 100644
> --- a/src/storage_backend_disk.c
> +++ b/src/storage_backend_disk.c
> @@ -557,6 +557,13 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
>          NULL
>      };
>  
> +    if (vol->encryption != NULL) {
> +        virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                              "%s", _("storage pool does not support encrypted "
> +                                      "volumes"));
> +        return -1;
> +    }
> +
>      if (virStorageBackendDiskPartFormat(conn, pool, vol, partFormat) != 0) {
>          return -1;
>      }
> diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
> index 0219eb6..b43daef 100644
> --- a/src/storage_backend_fs.c
> +++ b/src/storage_backend_fs.c
> @@ -1070,6 +1070,13 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
>      int tool_type;
>  
>      if (inputvol) {
> +        if (vol->encryption != NULL) {
> +            virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                                  "%s", _("storage pool does not support "
> +                                          "building encrypted volumes from "
> +                                          "other volumes"));
> +            return -1;
> +        }
>          create_func = virStorageBackendGetBuildVolFromFunction(conn, vol,
>                                                                 inputvol);
>          if (!create_func)
> diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
> index 6c123ae..822d321 100644
> --- a/src/storage_backend_logical.c
> +++ b/src/storage_backend_logical.c
> @@ -581,6 +581,13 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
>      };
>      const char **cmdargv = cmdargvnew;
>  
> +    if (vol->encryption != NULL) {
> +        virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> +                              "%s", _("storage pool does not support encrypted "
> +                                      "volumes"));
> +        return -1;
> +    }
> +
>      if (vol->backingStore.path) {
>          cmdargv = cmdargvsnap;
>      }
> diff --git a/src/storage_driver.c b/src/storage_driver.c
> index e9ecb20..98db9a0 100644
> --- a/src/storage_driver.c
> +++ b/src/storage_driver.c
> @@ -1305,6 +1305,8 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
>  cleanup:
>      if (volobj)
>          virUnrefStorageVol(volobj);
> +    if (voldef)
> +        virStorageEncryptionDropSecrets(voldef->encryption);
>      virStorageVolDefFree(voldef);
>      if (pool)
>          virStoragePoolObjUnlock(pool);
> @@ -1466,6 +1468,8 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
>  cleanup:
>      if (volobj)
>          virUnrefStorageVol(volobj);
> +    if (newvol)
> +        virStorageEncryptionDropSecrets(newvol->encryption);
>      virStorageVolDefFree(newvol);
>      if (pool)
>          virStoragePoolObjUnlock(pool);

As per other mails, I don't think its neccessary to drop the secrets
from volumes here. At least not until we introduce keystores as an
explicit public API

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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