On Tue, Mar 29, 2016 at 10:33:34AM -0400, John Ferlan wrote:
>
>
> [...]
>
> >>
> >> +/* 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.
> >> + */
> >> +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", NULL);
> >> +}
> >
> > How about calling this master-key.aes to make it explicit what this
> > key is intended to be used with.
> >
> >
>
> OK
>
> >> +/* 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.
> >
> > The QEMU secrets code is happy to get data in either raw or base64
> > format. I wonder if there's a compelling reason to use base64 instead
> > of just writing it out as raw bytes.
> >
>
> No compelling reason comes to mind - one extra level of obscurity
> mostly. One less place to have an error in the Write/Read functions for
> base64 conversions.
>
> [...]
>
> >> +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);
> >
> > It is probably better to just read nbytes from /dev/urandom
> > directly, as that's much closer to what gnutls_rnd would
> > do as compared to virRandomBits.
> >
>
> OK
>
> >> +#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 */
> >> + ignore_value(virFileWriteStr(path, "0", 0600));
> >
> > In the src/storage/storage_backend.c we have a fnuction that can use
> > scrub to wipe out a file. We should probably put that function into
> > src/util/virfile.c as virFileWipe() or something like that.
>
> Given Peter's continued objection, should we just skip this. IDC either
> way, but don't
>
> If it was desired, then I assume you are looking for just something that
> will move/rename virStorageBackendWipeLocal (adjusting params), right?
Ok lets just ignore it - we can easily add it later if desired.
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