Re: [libvirt] [PATCH 1/9] Add volume encryption information handling.
----- "Daniel P. Berrange" <berrange(a)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.
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
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".
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.
Mirek