Define an <encryption> tag specifying volume encryption format and format-depenedent parameters (e.g. passphrase, cipher name, key length, key). In most cases, the "secrets" (passphrases/keys) should only be transferred from libvirt users to libvirt, not the other way around. (Volume creation, when libvirt generates secrets for the user, is the only planned exception). Permanent storage of the secrets should be implemented outside of libvirt, although virDomainDefineXML() will cause libvirtd to store the secret locally with a domain. Only the qcow/qcow2 encryption format is currently supported, with the key/passphrase represented using base64. This patch does not add any users; the <encryption> tag is added in the following patches to both volumes (to support encrypted volume creation) and domains. --- bootstrap | 1 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 + src/storage_encryption.c | 252 ++++++++++++++++++++++++++++++++++++++++++++++ src/storage_encryption.h | 62 +++++++++++ 6 files changed, 322 insertions(+), 0 deletions(-) create mode 100644 src/storage_encryption.c create mode 100644 src/storage_encryption.h diff --git a/bootstrap b/bootstrap index 8b81e0e..885b299 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit modules=' +base64 c-ctype close connect diff --git a/po/POTFILES.in b/po/POTFILES.in index 0ea21fd..cc99b48 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -37,6 +37,7 @@ src/storage_backend_logical.c src/storage_backend_scsi.c src/storage_conf.c src/storage_driver.c +src/storage_encryption.c src/test.c src/uml_conf.c src/uml_driver.c diff --git a/src/Makefile.am b/src/Makefile.am index 9b662ae..6c628bd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,6 +51,7 @@ UTIL_SOURCES = \ memory.c memory.h \ pci.c pci.h \ qparams.c qparams.h \ + storage_encryption.h storage_encryption.c \ threads.c threads.h \ threads-pthread.h \ threads-win32.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 59c78d5..9850daa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -324,6 +324,11 @@ virStoragePartedFsTypeTypeToString; virStoragePoolObjLock; virStoragePoolObjUnlock; +virStorageEncryptionFree; +virStorageEncryptionDropSecrets; +virStorageEncryptionParseNode; +virStorageEncryptionFormat; + # threads.h virMutexInit; diff --git a/src/storage_encryption.c b/src/storage_encryption.c new file mode 100644 index 0000000..c3e3219 --- /dev/null +++ b/src/storage_encryption.c @@ -0,0 +1,252 @@ +/* + * storage_encryption.h: volume encryption information + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr(a)redhat.com&gt; + */ + +#include <config.h> + +#include "internal.h" + +#include "base64.h" +#include "buf.h" +#include "memory.h" +#include "storage_conf.h" +#include "storage_encryption.h" +#include "util.h" +#include "xml.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +VIR_ENUM_IMPL(virStorageEncryptionFormat, + VIR_STORAGE_ENCRYPTION_FORMAT_LAST, "unencrypted", "qcow") + +void +virStorageEncryptionFree(virStorageEncryptionPtr enc) +{ + if (!enc) + return; + + switch (enc->format) { + case VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED: + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW: + if (enc->v.qcow.passphrase != NULL) { + memset(enc->v.qcow.passphrase, 0, + strlen(enc->v.qcow.passphrase)); + VIR_FREE(enc->v.qcow.passphrase); + } + break; + + default: + virStorageReportError(NULL, VIR_ERR_INTERNAL_ERROR, + _("unhandled volume encryption format %d"), + enc->format); + break; + } + VIR_FREE(enc); +} + +void +virStorageEncryptionDropSecrets(virStorageEncryptionPtr enc) +{ + if (!enc) + return; + + switch (enc->format) { + case VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED: + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW: + if (enc->v.qcow.passphrase != NULL) { + memset(enc->v.qcow.passphrase, 0, + strlen(enc->v.qcow.passphrase)); + VIR_FREE(enc->v.qcow.passphrase); + } + break; + + default: + virStorageReportError(NULL, VIR_ERR_INTERNAL_ERROR, + _("unhandled volume encryption format %d"), + enc->format); + break; + } +} + +static virStorageEncryptionPtr +virStorageEncryptionParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) +{ + virStorageEncryptionPtr ret; + char *format_str; + int format; + + if (VIR_ALLOC(ret) < 0) { + virReportOOMError(conn); + return NULL; + } + + format_str = virXPathString(conn, "string(./@format)", ctxt); + if (format_str == NULL) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("unknown volume encryption format")); + goto cleanup; + } + format = virStorageEncryptionFormatTypeFromString(format_str); + if (format < 0) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("unknown volume encryption format type %s"), + format_str); + VIR_FREE(format_str); + goto cleanup; + } + VIR_FREE(format_str); + ret->format = format; + + switch (ret->format) { + case VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED: + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:{ + char *base64; + + base64 = virXPathString(conn, "string(./passphrase)", ctxt); + if (base64 != NULL) { + size_t base64_len, raw_len; + char *raw; + bool base64_ok; + + base64_len = strlen(base64); + base64_ok = base64_decode_alloc(base64, base64_len, &raw, + &raw_len); + memset(base64, 0, base64_len); + VIR_FREE(base64); + + if (!base64_ok) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("invalid base64 in passphrase")); + goto cleanup; + } + if (raw == NULL) { + virReportOOMError(conn); + goto cleanup; + } + if (VIR_ALLOC_N(ret->v.qcow.passphrase, raw_len + 1) < 0) { + virReportOOMError(conn); + memset(raw, 0, raw_len); + free(raw); + goto cleanup; + } + memcpy(ret->v.qcow.passphrase, raw, raw_len); + memset(raw, 0, raw_len); + free(raw); + } + break; + } + + default: + virStorageReportError(NULL, VIR_ERR_INTERNAL_ERROR, + _("unhandled volume encryption format %d"), + ret->format); + break; + } + + return ret; + + cleanup: + virStorageEncryptionFree(ret); + return NULL; +} + +virStorageEncryptionPtr +virStorageEncryptionParseNode(virConnectPtr conn, + xmlDocPtr xml, xmlNodePtr root) +{ + xmlXPathContextPtr ctxt = NULL; + virStorageEncryptionPtr enc = NULL; + + if (STRNEQ((const char *) root->name, "encryption")) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("unknown root element for volume " + "encryption information")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + ctxt->node = root; + enc = virStorageEncryptionParseXML(conn, ctxt); + + cleanup: + xmlXPathFreeContext(ctxt); + return enc; +} + +int +virStorageEncryptionFormat(virConnectPtr conn, + virBufferPtr buf, + virStorageEncryptionPtr enc, bool with_secrets) +{ + const char *format; + + format = virStorageEncryptionFormatTypeToString(enc->format); + if (!format) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected encryption format")); + return -1; + } + virBufferVSprintf(buf, " <encryption format='%s'>\n", format); + + switch (enc->format) { + case VIR_STORAGE_ENCRYPTION_FORMAT_UNENCRYPTED: + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW: + if (with_secrets && enc->v.qcow.passphrase != NULL) { + char *base64; + + base64_encode_alloc(enc->v.qcow.passphrase, + strlen(enc->v.qcow.passphrase), + &base64); + if (base64 == NULL) { + virReportOOMError(conn); + return -1; + } + virBufferVSprintf(buf, " <passphrase>%s</passphrase>\n", + base64); + free(base64); + } + break; + + default: + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unhandled volume encryption format %d"), + enc->format); + return -1; + } + + virBufferAddLit(buf, " </encryption>\n"); + + return 0; +} diff --git a/src/storage_encryption.h b/src/storage_encryption.h new file mode 100644 index 0000000..bd0b417 --- /dev/null +++ b/src/storage_encryption.h @@ -0,0 +1,62 @@ +/* + * storage_encryption.h: volume encryption information + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr(a)redhat.com&gt; + */ + +#ifndef __VIR_STORAGE_ENCRYPTION_H__ +#define __VIR_STORAGE_ENCRYPTION_H__ + +#include "internal.h" +#include "buf.h" +#include "util.h" + +#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; +}; + +void virStorageEncryptionFree(virStorageEncryptionPtr enc); +void virStorageEncryptionDropSecrets(virStorageEncryptionPtr enc); +virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, + xmlDocPtr xml, + xmlNodePtr root); +int virStorageEncryptionFormat(virConnectPtr conn, virBufferPtr buf, + virStorageEncryptionPtr enc, + bool with_secrets); + +#endif /* __VIR_STORAGE_ENCRYPTION_H__ */ -- 1.6.2.5

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. * 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. --- src/storage_conf.c | 19 +++++++++++++++++++ src/storage_conf.h | 2 ++ 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/src/storage_conf.c b/src/storage_conf.c index 075279c..4a77e87 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -267,6 +267,7 @@ virStorageVolDefFree(virStorageVolDefPtr def) { VIR_FREE(def->target.perms.label); VIR_FREE(def->backingStore.path); VIR_FREE(def->backingStore.perms.label); + virStorageEncryptionFree(def->encryption); VIR_FREE(def); } @@ -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); + } + return ret; cleanup: @@ -1254,6 +1269,10 @@ virStorageVolDefFormat(virConnectPtr conn, &def->backingStore, "backingStore") < 0) goto cleanup; + if (def->encryption != NULL && + virStorageEncryptionFormat(conn, &buf, def->encryption, false) < 0) + goto cleanup; + virBufferAddLit(&buf,"</volume>\n"); if (virBufferError(&buf)) 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" */ }; typedef struct _virStorageVolDefList virStorageVolDefList; -- 1.6.2.5

(The implementation is not very generic, but that can be very easily rectified if/when new encryption formats appear.) --- src/storage_backend_fs.c | 61 +++++++++++++++++++++++++++++++++++---------- 1 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 8cfc462..0219eb6 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -26,6 +26,7 @@ #include <sys/statvfs.h> #include <sys/types.h> #include <sys/stat.h> +#include <stdbool.h> #include <stdio.h> #include <dirent.h> #include <errno.h> @@ -81,6 +82,9 @@ struct FileTypeInfo { /* Store a COW base image path (possibly relative), * or NULL if there is no COW base image, to RES; * return BACKING_STORE_* */ + int qcowCryptOffset; /* Byte offset from start of file + * where to find encryption mode, + * -1 if encryption is not used */ int (*getBackingStore)(virConnectPtr conn, char **res, const unsigned char *buf, size_t buf_size); }; @@ -89,54 +93,54 @@ struct FileTypeInfo const fileTypeInfo[] = { /* XXX Untested { VIR_STORAGE_VOL_FILE_BOCHS, "Bochs Virtual HD Image", NULL, LV_LITTLE_ENDIAN, 64, 0x20000, - 32+16+16+4+4+4+4+4, 8, 1, NULL },*/ + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ /* CLoop */ /* XXX Untested { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, LV_LITTLE_ENDIAN, -1, 0, - -1, 0, 0, NULL }, */ + -1, 0, 0, -1, NULL }, */ /* Cow */ { VIR_STORAGE_VOL_FILE_COW, "OOOM", NULL, LV_BIG_ENDIAN, 4, 2, - 4+4+1024+4, 8, 1, cowGetBackingStore }, + 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, /* DMG */ /* XXX QEMU says there's no magic for dmg, but we should check... */ { VIR_STORAGE_VOL_FILE_DMG, NULL, ".dmg", 0, -1, 0, - -1, 0, 0, NULL }, + -1, 0, 0, -1, NULL }, /* XXX there's probably some magic for iso we can validate too... */ { VIR_STORAGE_VOL_FILE_ISO, NULL, ".iso", 0, -1, 0, - -1, 0, 0, NULL }, + -1, 0, 0, -1, NULL }, /* Parallels */ /* XXX Untested { VIR_STORAGE_VOL_FILE_PARALLELS, "WithoutFreeSpace", NULL, LV_LITTLE_ENDIAN, 16, 2, - 16+4+4+4+4, 4, 512, NULL }, + 16+4+4+4+4, 4, 512, -1, NULL }, */ /* QCow */ { VIR_STORAGE_VOL_FILE_QCOW, "QFI", NULL, LV_BIG_ENDIAN, 4, 1, - 4+4+8+4+4, 8, 1, qcowXGetBackingStore }, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, /* QCow 2 */ { VIR_STORAGE_VOL_FILE_QCOW2, "QFI", NULL, LV_BIG_ENDIAN, 4, 2, - 4+4+8+4+4, 8, 1, qcowXGetBackingStore }, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, /* VMDK 3 */ /* XXX Untested { VIR_STORAGE_VOL_FILE_VMDK, "COWD", NULL, LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 4, 512, NULL }, + 4+4+4, 4, 512, -1, NULL }, */ /* VMDK 4 */ { VIR_STORAGE_VOL_FILE_VMDK, "KDMV", NULL, LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 8, 512, vmdk4GetBackingStore }, + 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, /* Connectix / VirtualPC */ /* XXX Untested { VIR_STORAGE_VOL_FILE_VPC, "conectix", NULL, LV_BIG_ENDIAN, -1, 0, - -1, 0, 0, NULL}, + -1, 0, 0, -1, NULL}, */ }; @@ -282,13 +286,16 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, virStorageVolTargetPtr target, char **backingStore, unsigned long long *allocation, - unsigned long long *capacity) { + unsigned long long *capacity, + virStorageEncryptionPtr *encryption) { int fd; unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ int len, i, ret; if (backingStore) *backingStore = NULL; + if (encryption) + *encryption = NULL; if ((fd = open(target->path, O_RDONLY)) < 0) { virReportSystemError(conn, errno, @@ -317,6 +324,8 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, /* First check file magic */ for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { int mlen; + bool encrypted_qcow = false; + if (fileTypeInfo[i].magic == NULL) continue; @@ -375,6 +384,16 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, *capacity *= fileTypeInfo[i].sizeMultiplier; } + if (fileTypeInfo[i].qcowCryptOffset != -1) { + int crypt_format; + + crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | + (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | + (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | + head[fileTypeInfo[i].qcowCryptOffset+3]; + encrypted_qcow = crypt_format != 0; + } + /* Validation passed, we know the file format now */ target->format = fileTypeInfo[i].type; if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { @@ -400,6 +419,18 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } } } + if (encryption != NULL && encrypted_qcow) { + virStorageEncryptionPtr enc; + + if (VIR_ALLOC(enc) < 0) { + virReportOOMError(conn); + if (backingStore) + VIR_FREE(*backingStore); + return -1; + } + enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + *encryption = enc; + } return 0; } @@ -836,7 +867,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, &vol->target, &backingStore, &vol->allocation, - &vol->capacity) < 0)) { + &vol->capacity, + &vol->encryption) < 0)) { if (ret == -1) goto cleanup; else { @@ -876,7 +908,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, if ((ret = virStorageBackendProbeTarget(conn, &vol->backingStore, - NULL, NULL, NULL)) < 0) { + NULL, NULL, NULL, + NULL)) < 0) { if (ret == -1) goto cleanup; else { -- 1.6.2.5

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); -- 1.6.2.5

The if ((nlptr...)) implicitly assumes commptr != NULL. Make the assumption explicit, it will be broken in a future patch. --- src/qemu_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index d2db1a2..f2be27f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2081,10 +2081,11 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm, * occurence, and inbetween the command and the newline starting * the response */ - if ((commptr = strstr(buf, cmd))) + if ((commptr = strstr(buf, cmd))) { memmove(buf, commptr, strlen(commptr)+1); - if ((nlptr = strchr(buf, '\n'))) - memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1); + if ((nlptr = strchr(buf, '\n'))) + memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1); + } break; } -- 1.6.2.5

On Tue, Jul 21, 2009 at 01:12:03PM +0200, Miloslav Trma?? wrote: I have a feeling this patch is going to clash horribly with the refactoring Mark just did of the monitor code for FD passing. Your goal seems sane here - we'll just need to resolve this against Mark's code 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 :|

Also fix a potential infinite loop in qemuDomainCoreDump() if sending cont repeatedly fails. --- src/qemu_driver.c | 43 ++++++++++++++++++------------------------- 1 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 12079f8..9ead5fd 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -128,6 +128,7 @@ static int qemudMonitorCommandExtra(const virDomainObjPtr vm, const char *extra, const char *extraPrompt, char **reply); +static int qemudMonitorSendCont(const virDomainObjPtr vm); static int qemudDomainSetMemoryBalloon(virConnectPtr conn, virDomainObjPtr vm, unsigned long newmem); @@ -1199,7 +1200,6 @@ static int qemudInitCpus(virConnectPtr conn, virDomainObjPtr vm, const char *migrateFrom) { - char *info = NULL; #if HAVE_SCHED_GETAFFINITY cpu_set_t mask; int i, maxcpu = QEMUD_CPUMASK_LEN; @@ -1235,12 +1235,11 @@ qemudInitCpus(virConnectPtr conn, if (migrateFrom == NULL) { /* Allow the CPUS to start executing */ - if (qemudMonitorCommand(vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(vm) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); return -1; } - VIR_FREE(info); } return 0; @@ -2172,6 +2171,17 @@ qemudMonitorCommand(const virDomainObjPtr vm, return qemudMonitorCommandExtra(vm, cmd, NULL, NULL, reply); } +static int +qemudMonitorSendCont(const virDomainObjPtr vm) { + char *reply; + + if (qemudMonitorCommand(vm, "cont", &reply) < 0) + return -1; + qemudDebug ("%s: cont reply: %s", vm->def->name, info); + VIR_FREE(reply); + return 0; +} + static virDrvOpenStatus qemudOpen(virConnectPtr conn, @@ -2633,7 +2643,6 @@ cleanup: static int qemudDomainResume(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - char *info; virDomainObjPtr vm; int ret = -1; virDomainEventPtr event = NULL; @@ -2654,17 +2663,15 @@ static int qemudDomainResume(virDomainPtr dom) { goto cleanup; } if (vm->state == VIR_DOMAIN_PAUSED) { - if (qemudMonitorCommand(vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(vm) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("resume operation failed")); goto cleanup; } vm->state = VIR_DOMAIN_RUNNING; - qemudDebug("Reply %s", info); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); - VIR_FREE(info); } if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0) goto cleanup; @@ -3349,13 +3356,9 @@ cleanup: will support synchronous operations so we always get here after the migration is complete. */ if (resume && paused) { - if (qemudMonitorCommand(vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(vm) < 0) qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("resuming after dump failed")); - goto cleanup; - } - DEBUG ("%s: cont reply: %s", vm->def->name, info); - VIR_FREE(info); } if (vm) virDomainObjUnlock(vm); @@ -3824,13 +3827,11 @@ static int qemudDomainRestore(virConnectPtr conn, /* If it was running before, resume it now. */ if (header.was_running) { - char *info; - if (qemudMonitorCommand(vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(vm) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); goto cleanup; } - VIR_FREE(info); vm->state = VIR_DOMAIN_RUNNING; } ret = 0; @@ -5645,14 +5646,9 @@ qemudDomainMigratePerform (virDomainPtr dom, ret = 0; cleanup: - /* Note that we have to free info *first*, since we are re-using the - * variable below (and otherwise might cause a memory leak) - */ - VIR_FREE(info); - if (paused) { /* we got here through some sort of failure; start the domain again */ - if (qemudMonitorCommand (vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(vm) < 0) { /* Hm, we already know we are in error here. We don't want to * overwrite the previous error, though, so we just throw something * to the logs and hope for the best @@ -5660,16 +5656,13 @@ cleanup: VIR_ERROR(_("Failed to resume guest %s after failure\n"), vm->def->name); } - else { - DEBUG ("%s: cont reply: %s", vm->def->name, info); - VIR_FREE(info); - } event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } + VIR_FREE(info); if (vm) virDomainObjUnlock(vm); if (event) -- 1.6.2.5

--- src/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 50 insertions(+), 1 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 9ead5fd..b57db31 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2172,10 +2172,59 @@ qemudMonitorCommand(const virDomainObjPtr vm, } static int +qemudMonitorSendVolumePassphrase(const virDomainObjPtr vm, + const char *buf, + const char *prompt, + void *data ATTRIBUTE_UNUSED) +{ + const char *path; + size_t path_len; + int i; + + /* The complete prompt looks like this: + ide0-hd0 (/path/to/volume) is encrypted. + Password: + prompt starts with ") is encrypted". Extract /path/to/volume. */ + for (path = prompt; path > buf && path[-1] != '('; path-- ) + ; + if (path == buf) + return -1; + path_len = prompt - path; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk; + + disk = vm->def->disks[i]; + if (disk->src != NULL && memcmp(disk->src, path, path_len) == 0 && + disk->src[path_len] == '\0' && + disk->encryption != NULL && + disk->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) { + const char *passphrase; + + passphrase = disk->encryption->v.qcow.passphrase; + if (passphrase != NULL) { + size_t passphrase_len; + + passphrase_len = strlen(passphrase); + if (safewrite(vm->monitor, passphrase, passphrase_len) != + passphrase_len) + return -1; + if (safewrite(vm->monitor, "\r", 1) != 1) + return -1; + return 0; + } + } + } + return -1; +} + +static int qemudMonitorSendCont(const virDomainObjPtr vm) { char *reply; - if (qemudMonitorCommand(vm, "cont", &reply) < 0) + if (qemudMonitorCommandWithHandler(vm, "cont", ") is encrypted.", + qemudMonitorSendVolumePassphrase, + NULL, &reply) < 0) return -1; qemudDebug ("%s: cont reply: %s", vm->def->name, info); VIR_FREE(reply); -- 1.6.2.5