On Mon, Jun 13, 2016 at 20:27:56 -0400, John Ferlan wrote:
If the volume xml was looking to create a luks volume take the
necessary
steps in order to make that happen.
The processing will be:
1. create a temporary file in the storage pool target path
1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
depending on how the encryption xml specified fetching the secret
1b. create/open the file, initially setting mode to 0600 with current
effective uid:gid as owner
1c. fetch the secret into a buffer and write that into the file
1d. change file protections to 0400
2. create a secret object
2a. use a similarly crafted alias to the file name
2b. add the file to the object
3. create/add luks options to the commandline
3a. at the very least a "key-secret" using the secret object alias
3b. if found in the XML the various "cipher" and "ivgen" options
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/storage/storage_backend.c | 285 ++++++++++++++++++++++++++++++++++++++---
src/storage/storage_backend.h | 3 +-
src/util/virqemu.c | 23 ++++
src/util/virqemu.h | 6 +
tests/storagevolxml2argvtest.c | 3 +-
6 files changed, 300 insertions(+), 21 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fdf06ae..0d6d080 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2153,6 +2153,7 @@ virProcessWait;
# util/virqemu.h
+virQEMUBuildLuksOpts;
virQEMUBuildObjectCommandlineFromJSON;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 4965c9e..fc50807 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -55,11 +55,14 @@
#include "viralloc.h"
#include "internal.h"
#include "secret_conf.h"
+#include "secret_util.h"
#include "viruuid.h"
#include "virstoragefile.h"
#include "storage_backend.h"
#include "virlog.h"
#include "virfile.h"
+#include "virjson.h"
+#include "virqemu.h"
#include "stat-time.h"
#include "virstring.h"
#include "virxml.h"
@@ -880,6 +883,7 @@ virStoragePloopResize(virStorageVolDefPtr vol,
enum {
QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
+ QEMU_IMG_FORMAT_LUKS,
};
static bool
@@ -907,6 +911,27 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
return ret;
}
+
+static bool
+virStorageBackendQemuImgSupportsLuks(const char *qemuimg)
+{
+ bool ret = false;
+ int exitstatus = -1;
+ virCommandPtr cmd = virCommandNewArgList(qemuimg, "create",
"-o", "?",
+ "-f", "luks",
"/dev/null", NULL);
+
+ if (virCommandRun(cmd, &exitstatus) < 0)
+ goto cleanup;
+
+ if (exitstatus == 0)
+ ret = true;
+
+ cleanup:
+ virCommandFree(cmd);
+ return ret;
+}
+
+
static int
virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
{
@@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
* out what else we have */
int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
- /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
- * understands. Since we still support QEMU 0.12 and newer, we need
- * to be able to handle the previous format as can be set via a
- * compat=0.10 option. */
- if (virStorageBackendQemuImgSupportsCompat(qemuimg))
- ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
+ /* QEMU 2.6 added support for luks - let's check for that.
+ * If available, then we can also assume OPTIONS_COMPAT is present */
+ if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
+ ret = QEMU_IMG_FORMAT_LUKS;
+ } else {
+ /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
+ * understands. Since we still support QEMU 0.12 and newer, we need
+ * to be able to handle the previous format as can be set via a
+ * compat=0.10 option. */
+ if (virStorageBackendQemuImgSupportsCompat(qemuimg))
+ ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
+ }
This looks like it's becoming a source of technical debt. Heaps of
comments aren't helping.
return ret;
}
@@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
const char *inputPath;
const char *inputFormatStr;
int inputFormat;
+
+ char *secretAlias;
+ const char *secretPath;
};
+
static int
-virStorageBackendCreateQemuImgOpts(char **opts,
+virStorageBackendCreateQemuImgOpts(virStorageEncryptionPtr enc,
+ char **opts,
struct _virStorageBackendQemuImgInfo info)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (info.backingPath)
- virBufferAsprintf(&buf, "backing_fmt=%s,",
- virStorageFileFormatTypeToString(info.backingFormat));
- if (info.encryption)
- virBufferAddLit(&buf, "encryption=on,");
- if (info.preallocate)
- virBufferAddLit(&buf, "preallocation=metadata,");
+ if (info.format == VIR_STORAGE_FILE_LUKS) {
+ virQEMUBuildLuksOpts(&buf, enc, info.secretAlias);
+ } else {
+ if (info.backingPath)
+ virBufferAsprintf(&buf, "backing_fmt=%s,",
+ virStorageFileFormatTypeToString(info.backingFormat));
+ if (info.encryption)
+ virBufferAddLit(&buf, "encryption=on,");
+ if (info.preallocate)
+ virBufferAddLit(&buf, "preallocation=metadata,");
+ }
+
if (info.nocow)
virBufferAddLit(&buf, "nocow=on,");
@@ -1025,6 +1066,22 @@ virStorageBackendCreateQemuImgCheckEncryption(int format,
if (virStorageGenerateQcowEncryption(conn, vol) < 0)
return -1;
}
+ } else if (format == VIR_STORAGE_FILE_LUKS) {
+ if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported volume encryption format %d"),
+ vol->target.encryption->format);
+ return -1;
+ }
+ if (enc->nsecrets > 1) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("too many secrets for luks encryption"));
+ return -1;
+ }
+ if (enc->nsecrets == 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("no secret provided for luks encryption"));
+ }
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("volume encryption unsupported with format %s"),
type);
@@ -1069,6 +1126,12 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr
pool,
int accessRetCode = -1;
char *absolutePath = NULL;
+ if (info->format == VIR_STORAGE_FILE_LUKS) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot set backing store for luks volume"));
+ return -1;
+ }
+
info->backingFormat = vol->target.backingStore->format;
info->backingPath = vol->target.backingStore->path;
@@ -1121,6 +1184,7 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr
pool,
static int
virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
+ virStorageVolDefPtr vol,
int imgformat,
struct _virStorageBackendQemuImgInfo info)
{
@@ -1130,7 +1194,8 @@ virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
info.compat = "0.10";
- if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0)
+ if (virStorageBackendCreateQemuImgOpts(vol->target.encryption,
+ &opts, info) < 0)
return -1;
if (opts)
virCommandAddArgList(cmd, "-o", opts, NULL);
@@ -1140,6 +1205,66 @@ virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
}
+/* Create a hopefully unique enough name to be used for both the
+ * secretPath and secretAlias generation
This won't fly. There's only one way to guarantee that there won't be a
collision. You need to create a file in a private path.
+ */
+static char *
+virStorageBackendCreateQemuImgLuksName(const char *volname,
+ virStorageEncryptionPtr enc)
+{
+ char *ret;
+
+ if (enc->secrets[0]->secdef.type == VIR_SECRET_LOOKUP_TYPE_UUID)
+ ignore_value(virAsprintf(&ret, "%s_%s", volname,
+ enc->secrets[0]->secdef.u.uuid));
+ else
+ ignore_value(virAsprintf(&ret, "%s_%s", volname,
+ enc->secrets[0]->secdef.u.usage));
usage is user provided text. Also your example in previous patch uses
slashes in it. I don't think it will end up well in this case.
+ return ret;
+}
+
+
+/* Add a secret object to the command line:
+ * --object secret,id=$secretAlias,file=$secretPath
+ *
+ * NB: format=raw is assumed
+ */
+static int
+virStorageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
+ virStorageVolDefPtr vol,
+ struct _virStorageBackendQemuImgInfo *info)
+{
+ virStorageEncryptionPtr enc = vol->target.encryption;
+ char *str = NULL;
+ virJSONValuePtr props = NULL;
+ char *commandStr = NULL;
+
+ if (!(str = virStorageBackendCreateQemuImgLuksName(vol->name, enc)))
+ return -1;
+
+ if (virAsprintf(&info->secretAlias, "%s_luks0", str) < 0) {
+ VIR_FREE(str);
+ return -1;
+ }
+ VIR_FREE(str);
+
+ if (virJSONValueObjectCreate(&props, "s:file", info->secretPath,
NULL) < 0)
+ return -1;
+
+ if (!(commandStr = virQEMUBuildObjectCommandlineFromJSON("secret",
+ info->secretAlias,
+ props))) {
+ virJSONValueFree(props);
+ return -1;
+ }
+ virJSONValueFree(props);
+
+ virCommandAddArgList(cmd, "--object", commandStr, NULL);
+
+ return 0;
+}
+
+
/* Create a qemu-img virCommand from the supplied binary path,
* volume definitions and imgformat
*/
@@ -1150,7 +1275,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
virStorageVolDefPtr inputvol,
unsigned int flags,
const char *create_tool,
- int imgformat)
+ int imgformat,
+ const char *secretPath)
{
virCommandPtr cmd = NULL;
const char *type;
@@ -1162,6 +1288,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
.compat = vol->target.compat,
.features = vol->target.features,
.nocow = vol->target.nocow,
+ .secretPath = secretPath,
+ .secretAlias = NULL,
};
virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
@@ -1192,6 +1320,18 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
_("format features only available with qcow2"));
return NULL;
}
+ if (info.format == VIR_STORAGE_FILE_LUKS) {
+ if (inputvol) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot use inputvol with luks volume"));
+ return NULL;
+ }
+ if (!info.encryption) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing encryption description"));
+ return NULL;
+ }
+ }
if (inputvol &&
virStorageBackendCreateQemuImgSetInput(inputvol, &info) < 0)
@@ -1207,7 +1347,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
conn, vol) < 0)
return NULL;
-
/* Size in KB */
info.size_arg = VIR_DIV_UP(vol->target.capacity, 1024);
@@ -1226,10 +1365,20 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
if (info.backingPath)
virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
- if (virStorageBackendCreateQemuImgSetOptions(cmd, imgformat, info) < 0) {
+ if (info.format == VIR_STORAGE_FILE_LUKS &&
+ virStorageBackendCreateQemuImgSecretObject(cmd, vol, &info) < 0) {
+ VIR_FREE(info.secretAlias);
+ virCommandFree(cmd);
+ return NULL;
+ }
+
+ if (virStorageBackendCreateQemuImgSetOptions(cmd, vol, imgformat,
+ info) < 0) {
+ VIR_FREE(info.secretAlias);
virCommandFree(cmd);
return NULL;
}
+ VIR_FREE(info.secretAlias);
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
@@ -1240,6 +1389,86 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
return cmd;
}
+
+static char *
+virStorageBackendCreateQemuImgSecretPath(virConnectPtr conn,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol)
+{
+ virStorageEncryptionPtr enc = vol->target.encryption;
+ char *str = NULL;
+ char *secretPath = NULL;
+ int fd = -1;
+ uint8_t *secret = NULL;
+ size_t secretlen = 0;
+
+ if (!enc) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing encryption description"));
+ return NULL;
+ }
+
+ if (!conn || !conn->secretDriver ||
+ !conn->secretDriver->secretLookupByUUID ||
+ !conn->secretDriver->secretLookupByUsage ||
+ !conn->secretDriver->secretGetValue) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to look up encryption secret"));
+ return NULL;
+ }
+
+ /* Create a temporary secret file in the pool using */
+ if (!(str = virStorageBackendCreateQemuImgLuksName(vol->name, enc)))
+ return NULL;
+
+ /* Since we don't have a file, just go to cleanup using NULL secretPath */
+ if (!(secretPath = virFileBuildPath(pool->def->target.path, str,
".tmp")))a
Apart from creating colliding paths and making possibly volumes appear
after a refresh this isn't a good idea also due to the fact that
creating the file with the secret may not be possible on NETFS pools due
to root squashing.
+ goto cleanup;
+
+ if ((fd = open(secretPath, O_WRONLY|O_TRUNC|O_CREAT, 0600)) < 0) {
+ virReportSystemError(errno, "%s",
+ _("failed to open luks secret file for write"));
+ goto error;
+ }
+
+ if (virSecretGetSecretString(conn, &enc->secrets[0]->secdef,
+ VIR_SECRET_USAGE_TYPE_KEY,
+ &secret, &secretlen) < 0)
+ goto error;
+
+ if (safewrite(fd, secret, secretlen) < 0) {
+ virReportSystemError(errno, "%s",
+ _("failed to write luks secret file"));
+ goto error;
+ }
+ VIR_FORCE_CLOSE(fd);
+
+ if (chown(secretPath, geteuid(), getegid()) < 0) {
You also need to take into account the uid and gid the qemu-img process
will be using rather than the effective values.
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to chown luks secret file"));
+ goto error;
+ }
+
+ if (chmod(secretPath, 0400) < 0) {
You've already created this with mode 600. Is 400 really necessary? If
the process manages to destroy the secret, do you care?
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to chown luks secret file"));
+ goto error;
+ }
+
+ cleanup:
+ VIR_FREE(str);
+ VIR_DISPOSE_N(secret, secretlen);
+ VIR_FORCE_CLOSE(fd);
+
+ return secretPath;
+
+ error:
+ unlink(secretPath);
+ VIR_FREE(secretPath);
+ goto cleanup;
+}
+
+
int
virStorageBackendCreateQemuImg(virConnectPtr conn,
virStoragePoolObjPtr pool,
[...]