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 <jfer...@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 {
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list