On Thu, Mar 24, 2016 at 13:53:20 -0400, John Ferlan wrote:
Add a masterKey to _qemuDomainObjPrivate to store a random domain
master
key in order to support the ability to encrypt/decrypt sensitive data
shared between libvirt and qemu. The key will be base64 encoded and written
to a file to be used by the command line building code to share with qemu.
New API's from this patch:
qemuDomainGetMasterKeyFilePath:
Return a path to where the key is located
qemuDomainWriteMasterKeyFile: (private)
Base64 the masterKey and write to the *KeyFilePath
qemuDomainMasterKeyReadFile:
Using the *KeyFilePath, open/read the file, base64 decode, and
store in masterKey. Expected use only from qemuProcessReconnect
qemuDomainGenerateRandomKey: (private)
Generate a random key using available algorithms
The key is generated either from the gnutls_rnd function if it
exists or a less cryptographically strong mechanisum as a series of 8
bit random numbers from virRandomBits stored as a byte string.
qemuDomainMasterKeyRemove:
Remove traces of the master key, remove the *KeyFilePath
qemuDomainMasterKeyCreate:
Generate the key and save a base64 encoded value of the key
in the location returned by qemuDomainGetMasterKeyFilePath.
This API will first ensure the QEMU_CAPS_OBJECT_SECRET is set
in the capabilities. If not, then there's no need to generate
the secret or file.
The creation of the key will be attempted from qemuProcessPrepareHost
once the libDir directory structure exists.
The removal of the key will handled from qemuProcessStop just prior
to deleting the libDir tree.
Since the key will not be written out to the domain object XML file,
the qemuProcessReconnect will read the saved, decode, and restore
the masterKey value after a series of checks.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 269 ++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 13 +++
src/qemu/qemu_process.c | 11 ++
3 files changed, 293 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 53cb2b6..fecf668 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
[...]
@@ -465,6 +471,269 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr
jobInfo,
}
+/* qemuDomainGetMasterKeyFilePath:
+ * @libDir: Directory path to domain lib files
+ *
+ * This API will generate a path of the domain's libDir (e.g.
+ * (/var/lib/libvirt/qemu/domain-#-$NAME/) and a name 'master.key'.
+ *
+ * This API will check and fail if the libDir is NULL as that results
+ * in an invalid path generated.
+ *
+ * This API does not check if the resulting path exists, that is left
+ * up to the caller.
+ *
+ * Returns path to memory containing the name of the file. It is up to the
+ * caller to free; otherwise, NULL on failure.
Whoah, docs longer than the code itself.
+ */
+char *
+qemuDomainGetMasterKeyFilePath(const char *libDir)
+{
+ if (!libDir) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid path for master key file"));
+ return NULL;
And the code is rather self-explanatory.
+ }
+ return virFileBuildPath(libDir, "master.key", NULL);
+}
+
+
+/* qemuDomainWriteMasterKeyFile:
+ * @priv: pointer to domain private object
+ *
+ * Get the desired path to the masterKey file, base64 encode the masterKey,
+ * and store it in the file.
+ *
+ * Returns 0 on success, -1 on failure with error message indicating failure
+ */
+static int
+qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv)
+{
+ char *path;
+ char *base64Key = NULL;
+ int ret = -1;
+
+ if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir)))
+ return -1;
+
+ /* base64 encode the key to store it */
+ base64_encode_alloc(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN,
+ &base64Key);
+ if (!base64Key) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to encode master key"));
+ goto cleanup;
+ }
+
+ if (virFileWriteStr(path, base64Key, 0600) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to write master key file for domain"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (base64Key) {
+ /* clear heap, free memory */
+ memset(base64Key, 0, strlen(base64Key));
+ VIR_FREE(base64Key);
+ }
+
+ VIR_FREE(path);
+
+ return ret;
+}
+
+
+/* qemuDomainMasterKeyReadFile:
+ * @priv: pointer to domain private object
+ *
+ * Expected to be called during qemuProcessReconnect once the domain
+ * libDir has been generated through qemuStateInitialize calling
+ * virDomainObjListLoadAllConfigs which will restore the libDir path
+ * to the domain private object.
+ *
+ * This function will get the path to the master key file and if it
+ * exists, it will read the file, base64 decode the read value, and
+ * save it in priv->masterKey.
+ *
+ * Once the file exists, the validity checks may cause failures; however,
+ * if the file doesn't exist or the capability doesn't exist, we just
+ * return (mostly) quietly.
+ *
+ * Returns 0 on success or lack of capability
+ * -1 on failure with error message indicating failure
+ */
+int
+qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
+{
+ char *path;
+ char *base64Key = NULL;
+ int base64KeySize = 0;
+ char *masterKey = NULL;
+ size_t masterKeySize = 0;
+ int ret = -1;
+
+ /* If we don't have the capability, then do nothing. */
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET))
+ return 0;
+
+ if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir)))
+ return -1;
+
+ if (!virFileExists(path)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("domain master key file doesn't exist in %s"),
+ priv->libDir);
+ goto cleanup;
+ }
+
+ /* Read the base64 encooded secret from the file, decode it, and
+ * store in the domain private object.
+ */
+ if ((base64KeySize = virFileReadAll(path, 1024, &base64Key)) < 0)
+ goto cleanup;
+
+ if (!base64_decode_alloc(base64Key, base64KeySize,
+ &masterKey, &masterKeySize)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid base64 in domain master key file"));
+ goto cleanup;
+ }
+
+ if (!masterKey || masterKeySize != QEMU_DOMAIN_MASTER_KEY_LEN) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid encoded master key read, size=%zd"),
+ masterKeySize);
+ goto cleanup;
+ }
+
+ priv->masterKey = masterKey;
+ masterKey = NULL;
so this is NULL now and masterKeySize > 0
+
+ ret = 0;
+
+ cleanup:
+ if (masterKeySize > 0)
+ memset(masterKey, 0, masterKeySize);
... memset(NULL, 0, 32);
This crashes on success path.
+ VIR_FREE(masterKey);
+
+ if (base64KeySize > 0)
+ memset(base64Key, 0, base64KeySize);
+ VIR_FREE(base64Key);
+
+ VIR_FREE(path);
+
+ return ret;
+}
+
+
+/* qemuDomainGenerateRandomKey
+ * @nbytes: Size in bytes of random key to generate
+ *
+ * Generate a random key of nbytes length and return it.
+ *
+ * Since the gnutls_rnd could be missing, provide an alternate less
+ * secure mechanism to at least have something.
+ *
+ * Returns pointer memory containing key on success, NULL on failure
+ */
+static char *
+qemuDomainGenerateRandomKey(size_t nbytes)
+{
+ char *key;
+#if HAVE_GNUTLS_CRYPTO_H
+ int ret;
+#else
+ size_t i;
+#endif
+
+ if (VIR_ALLOC_N(key, nbytes) < 0)
+ return NULL;
+
+#if HAVE_GNUTLS_CRYPTO_H
+ /* Generate a master key using gnutls if possible */
+ if ((ret = gnutls_rnd(GNUTLS_RND_RANDOM, key, nbytes)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to generate master key, ret=%d"), ret);
+ VIR_FREE(key);
+ return NULL;
+ }
+#else
+ /* Generate a less cryptographically strong master key */
+ for (i = 0; i < nbytes; i++)
+ key[i] = virRandomBits(8);
+#endif
+
+ return key;
+}
+
+
+/* qemuDomainMasterKeyRemove:
+ * @priv: Pointer to the domain private object
+ *
+ * Remove the traces of the master key, clear the heap, clear the file,
+ * delete the file.
+ */
+void
+qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv)
+{
+ char *path = NULL;
+
+ if (!priv->masterKey)
+ return;
+
+ /* Clear the heap */
+ memset(priv->masterKey, 0, QEMU_DOMAIN_MASTER_KEY_LEN);
+ VIR_FREE(priv->masterKey);
+
+ /* Clear and remove the master key file */
+ path = qemuDomainGetMasterKeyFilePath(priv->libDir);
+ if (path && virFileExists(path)) {
+ /* Write a "0" to the file even though we're about to delete it */
This still doesn't make any sense. The filesystem needs to guarantee
this anyways. This is just a false sense of security.
+ ignore_value(virFileWriteStr(path, "0", 0600));
+ unlink(path);
+ }
+ VIR_FREE(path);
+}
+
+
+/* qemuDomainMasterKeyCreate:
+ * @priv: Pointer to the domain private object
+ *
+ * As long as the underlying qemu has the secret capability,
+ * generate and store as a base64 encoded value a random 32-byte
+ * key to be used as a secret shared with qemu to share sensitive data.
+ *
+ * Returns: 0 on success, -1 w/ error message on failure
+ */
+int
+qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv)
+{
+ char *key = NULL;
+
+ /* If we don't have the capability, then do nothing. */
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET))
+ return 0;
+
+ if (!(key = qemuDomainGenerateRandomKey(QEMU_DOMAIN_MASTER_KEY_LEN)))
+ goto error;
+
+ priv->masterKey = key;
+ key = NULL;
key isn't touched after the previous line, thus it doesn't make much
sense to clear that variable.
+
+ if (qemuDomainWriteMasterKeyFile(priv) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ qemuDomainMasterKeyRemove(priv);
+ return -1;
+}
+
+
static virClassPtr qemuDomainDiskPrivateClass;
static int
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 573968c..f85f17f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -147,6 +147,7 @@ struct qemuDomainJobObj {
typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+# define QEMU_DOMAIN_MASTER_KEY_LEN 32 /* For a 32-byte random masterKey */
256 bit.
typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {