Re: [PATCH v2 3/4] qemu: Create domain master key

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 29, 2016 at 07:11:35PM -0400, John Ferlan wrote:
> Add a masterKey and masterKeyLen to _qemuDomainObjPrivate to store a
> random domain master key and its length 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)
>     Open (create/trunc) the masterKey path and write the masterKey
> 
>   qemuDomainMasterKeyReadFile:
>     Using the master key path, open/read the file, and store the
>     masterKey and masterKeyLen. 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 mechanism using
>     virGenerateRandomBytes
> 
>    qemuDomainMasterKeyRemove:
>     Remove traces of the master key, remove the *KeyFilePath
> 
>   qemuDomainMasterKeyCreate:
>     Generate the domain master key and save 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 file and restore the
> masterKey and masterKeyLen.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/qemu/qemu_domain.c  | 252 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  15 +++
>  src/qemu/qemu_process.c |  11 +++
>  3 files changed, 278 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 53cb2b6..99a91d2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -44,6 +44,12 @@
>  #include "virthreadjob.h"
>  #include "viratomic.h"
>  #include "virprocess.h"
> +#include "virrandom.h"
> +#include "base64.h"
> +#include <gnutls/gnutls.h>
> +#if HAVE_GNUTLS_CRYPTO_H
> +# include <gnutls/crypto.h>
> +#endif
>  #include "logging/log_manager.h"
>  
>  #include "storage/storage_driver.h"
> @@ -465,6 +471,252 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
>  }
>  
>  
> +/* qemuDomainGetMasterKeyFilePath:
> + * @libDir: Directory path to domain lib files
> + *
> + * Generate a path to the domain master key file for libDir.
> + * It's up to the caller to handle checking if path exists.
> + *
> + * Returns path to memory containing the name of the file. It is up to the
> + * caller to free; otherwise, NULL on failure.
> + */
> +char *
> +qemuDomainGetMasterKeyFilePath(const char *libDir)
> +{
> +    if (!libDir) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("invalid path for master key file"));
> +        return NULL;
> +    }
> +    return virFileBuildPath(libDir, "master-key.aes", NULL);
> +}
> +
> +
> +/* qemuDomainWriteMasterKeyFile:
> + * @priv: pointer to domain private object
> + *
> + * Get the desired path to the masterKey file and store it in the path.
> + *
> + * Returns 0 on success, -1 on failure with error message indicating failure
> + */
> +static int
> +qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv)
> +{
> +    char *path;
> +    int fd = -1;
> +    int ret = -1;
> +
> +    if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir)))
> +        return -1;
> +
> +    if ((fd = open(path, O_WRONLY|O_TRUNC|O_CREAT, 0600)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("failed to open domain master key file for write"));
> +        goto cleanup;
> +    }
> +
> +    if (safewrite(fd, priv->masterKey, priv->masterKeyLen) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("failed to write master key file for domain"));
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FORCE_CLOSE(fd);
> +    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 contents of the file saving 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;
> +    int fd = -1;
> +    uint8_t *masterKey = NULL;
> +    ssize_t masterKeyLen = 0;
> +
> +    /* 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 error;
> +    }
> +
> +    if ((fd = open(path, O_RDONLY)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("failed to open domain master key file for read"));
> +        goto error;
> +    }
> +
> +    if (VIR_ALLOC_N(masterKey, 1024) < 0)
> +        goto error;
> +
> +    if ((masterKeyLen = saferead(fd, masterKey, 1024)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("unable to read domain master key file"));
> +        goto error;
> +    }
> +
> +    if (masterKeyLen != QEMU_DOMAIN_MASTER_KEY_LEN) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("invalid master key read, size=%zd"), masterKeyLen);
> +        goto error;
> +    }
> +
> +    masterKey[masterKeyLen] = '\0';
> +    ignore_value(VIR_REALLOC_N_QUIET(masterKey, masterKeyLen + 1));

We're storing the key as raw binary data, so NUL terminating
it doesn't make any sense - it may already contain embedded
NULs and so nothing should try strlen() on it. So just drop
that extra NUL byte,

> +    priv->masterKey = masterKey;
> +    priv->masterKeyLen = masterKeyLen;
> +
> +    VIR_FORCE_CLOSE(fd);
> +    VIR_FREE(path);
> +
> +    return 0;
> +
> + error:
> +    if (masterKeyLen > 0)
> +        memset(masterKey, 0, masterKeyLen);
> +    VIR_FREE(masterKey);
> +
> +    VIR_FORCE_CLOSE(fd);
> +    VIR_FREE(path);
> +
> +    return -1;
> +}
> +


ACK with that fix above

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]