[libvirt] [PATCH 1/9] Add volume encryption information handling.

[Daniel P. Berrange]

On Fri, Jul 24, 2009 at 12:08:32AM -0400, Miloslav Trmac wrote:
> ----- "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > On Tue, Jul 21, 2009 at 01:11:57PM +0200, Miloslav Trma?? wrote:
> > > +#include <stdbool.h>
> > > +#include <libxml/tree.h>
> > > +
> > > +enum virStorageEncryptionFormat {
> > > +    VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED = 0,
> > > +    VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
> > > +
> > > +    VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
> > > +};
> > > +VIR_ENUM_DECL(virStorageEncryptionFormat)
> > > +
> > > +typedef struct _virStorageEncryption virStorageEncryption;
> > > +typedef virStorageEncryption *virStorageEncryptionPtr;
> > > +struct _virStorageEncryption {
> > > +    int format;            /* enum virStorageEncryptionFormat */
> > > +
> > > +    union {                /* Format-specific data */
> > > +        struct {
> > > +            char *passphrase;
> > > +        } qcow;
> > > +    } v;
> > > +};
> > 
> > As with the XML format, I'd like to avoid encoding qcow as a 
> > structural element here. Instead go for a generic storage of
> > secrets.
> > 
> >   enum virStorageEncryptionSecret {
> >     VIR_STORAGE_ENCRYPTION_SECRET_PASSPHRASE,
> >   };
> >  
> >   struct virStorageSecret{
> >      int type;    /* enum virStorageSecret */
> > 
> >      union {
> >         char *passphrase;
> >      } data;
> >   };
> > 
> >   struct _virStorageEncryption {
> >     unsigned encrypted : 1;
> > 
> >     int nsecrets;
> >     virStorageSecret *secrets;
> >   }
> > 
> > This allows for > 1 secret should we need that (eg, for
> > LUKS/cryptsetup volume)
>
> As argued in the 0/9 e-mail, the "encryption format" is an independent 
> piece of information that needs to be stored, and the set of other 
> information that needs to be stored can differ among formats.  Should 
> we ever need support for more than one secret in LUKS, that a separate
> struct { ... } luks should be added to the union: it seems to me that 
> support for storing more than one passphrase is not necessary for 
> libvirt's use of LUKS (although LUKS supports it) - on the other hand,
> if such support would be necessary, it would most likely be necessary 
> to store the slot used by each passphrase as well.

For the domain XML I agree that libirt would not need to worry about
multiple LUKS keys, but for the storage volume XML we would need to 
expose the fact that there are multiple keys,or allow creation of
volumes with multiple keys.


> We know that the type and amount of information that will need to be 
> stored will vary depending on the "encryption format"; on the other
> hand, expecting that the information consists of independent "secrets",
> each with a simple and format-independent definition, is probably too
> optimistic.  (As mentioned above, we might need a key slot ID associated
> with a LUKS passphrase; we might also need a password hash algorithm
> associated with a dm-crypt passphrase.  The encryption formats used in
> practice are often complex enough that a "simple passphrase" can not
> be used.)  Once we have to assume that each secret can have an "encryption
> format"-dependent format, I think it is much clearer to use something like

The idea of a 'key slot' and 'password hash algorithm' are not neccessarily
unique to LUKS though. If we get 2 different encryption formats both requiring
the concept of a 'key slot' we need to represent them in the same way in the
XML, not hve a different XML for each format. 

>    enum { FORMAT_1, FORMAT_2, FORMAT_3 } format;
>    union {
>      struct { char *secret_1; } format_1;
>      struct { struct { char *secret, id; } secrets[N]; } format_2;
>      struct { int param_1, param_2, param_3; char *secret_1, *secret_2 }; format_2;
>    }
>
> which explicitly defines what information is requested for each format,
> and how it is related, (and which allows showing the data related to a
> single format in a debugger by simply choosing a single member of the
> union) than something like
>
>   struct {
>     enum { SECRET_FMT_1, SECRET_FMT_2, PARAM_FMT_1, PARAM_FMT_2 } type;
>     union {
>       char *secret_fmt_1;
>       struct { char *secret, id} secret_fmt_2;
>       int param_fmt_1;
>       char *param_fmt_2;
>    }
>   } [...]
> with implicit assumptions about the secret and parameter formats used 
> by various "encryption formats".

This makes life very hard for libvirt clients, because it implies that
every single new disk encryption format we add, results in new XML 
elements being added, which pushes a significant burden onto clients
which want to use encryption, because they now have to be aware of 
multiple different ways of representing the same fundamental concepts.


> Even if the XML uses one or more generic-looking <secret> elements, the
> information will have to be categorized depending on its meaning and
> relations before being used; it seems to me cleaner to do it once, when
> parsing the XML, than to parse the XML without assigning much semantic
> value to it, and then re-parse the internal representation to categorize
> the information.
> 
> Categorizing the information already when parsing the XML will also make
> it easy to detect invalid or missing data already when parsing the XML,
> before any "real" operations that might be costly or difficult to undo
> are started, or before the information is used or stored by other
> components of libvirt.

The libvirt approach to XML representation is to try and define XML format
that are indenpedant of specific implementations. This does imply that the
XML parser cannot really do anything beyond basic syntactic validation,
near zero semantic  validation. The task of semantic validation of the
parsed data, is thus passed off to the internal drivers which provide the
specific implementations. 

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 :|