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

Re: [libvirt] [PATCH 2/9] Attach encryption information to virStorageVolDef.



On Tue, Jul 21, 2009 at 01:11:58PM +0200, Miloslav Trma?? wrote:
> The XML allows <encryption format='unencrypted'/>, this implementation
> canonicalizes the internal representation so that "vol->encryption" is
> non-NULL iff the volume is encrypted.
> 
> Note that partial encryption information (e.g. specifying an encryption
> format, but not the key/passphrase) is valid:
> * virStorageVolGetXMLDesc() will never reveal the key/passphrase, even
>   if known by libvirt.

I don't think that restriction really adds anything in the scenario 
that we're using domain XML files for persistent storage of keys.

eg, if domain XML lets you see passphrases, then vol XML should
     too (given a suitable VIR_STORAGE_VOL_SECURE flag).

    if we use a keystore, then forbid display of passphrases
    for domain XML and volume XML, and reference everything
    via UUID.

> * Future mechanisms could be set up to allow a libvirt user to specify
>   during volume creation that a volume should be encrypted, leaving
>   libvirt to choose suitable parameters and key and return them:
>   this would allow the libvirt user to automatically support any
>   encryption parameters (and perhaps encryption formats) supported in
>   libvirt, as long as the user can send the same information back when
>   using the volume in the future.

We could allow this now without extra APIs, if we let virStorageVolGetXML
show the ke/passphrase given a new VIR_STORAGE_VOL_SECURE flag.

> @@ -960,6 +961,7 @@ virStorageVolDefParseXML(virConnectPtr conn,
>      char *allocation = NULL;
>      char *capacity = NULL;
>      char *unit = NULL;
> +    xmlNodePtr node;
>  
>      options = virStorageVolOptionsForPoolType(pool->type);
>      if (options == NULL)
> @@ -1047,6 +1049,19 @@ virStorageVolDefParseXML(virConnectPtr conn,
>                                  "./backingStore/permissions", 0600) < 0)
>          goto cleanup;
>  
> +    node = virXPathNode(conn, "./encryption", ctxt);
> +    if (node != NULL) {
> +        virStorageEncryptionPtr enc;
> +
> +        enc = virStorageEncryptionParseNode(conn, ctxt->doc, node);
> +        if (enc == NULL)
> +            goto cleanup;
> +        if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED)
> +            ret->encryption = enc;
> +        else
> +            virStorageEncryptionFree(enc);
> +    }

The <encryption> element should probably be inside the <target>
element, since we might need to have separate <encryption> element
for the <backingstore> too in the future. eg have it alongside
the <permissions> element

> diff --git a/src/storage_conf.h b/src/storage_conf.h
> index a6c3650..cd6944f 100644
> --- a/src/storage_conf.h
> +++ b/src/storage_conf.h
> @@ -26,6 +26,7 @@
>  
>  #include "internal.h"
>  #include "util.h"
> +#include "storage_encryption.h"
>  #include "threads.h"
>  
>  #include <libxml/tree.h>
> @@ -95,6 +96,7 @@ struct _virStorageVolDef {
>      virStorageVolSource source;
>      virStorageVolTarget target;
>      virStorageVolTarget backingStore;
> +    virStorageEncryptionPtr encryption; /* only used if not "unencrypted" */
>  };

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]