[libvirt] [PATCH v4 3/3] qemu: Utilize qemu secret objects for RBD auth/secret
Peter Krempa
pkrempa at redhat.com
Wed May 18 07:49:11 UTC 2016
On Tue, May 17, 2016 at 12:36:09 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1182074
>
> If they're available and we need to pass secrets to qemu, then use the
> qemu domain secret object in order to pass the secrets for RBD volumes
> instead of passing the base64 encoded secret on the command line.
>
> The goal is to make AES secrets the default and have no user interaction
> required in order to allow using the AES mechanism. If the mechanism
> is not available, then fall back to the current plain mechanism using
> a base64 encoded secret.
>
> New API's:
>
> qemu_command.c:
> qemuBuildSecretInfoProps: (private)
> Generate/return a JSON properties object for the AES secret to
> be used by both the command building and eventually the hotplug
> code in order to add the secret object. Code was designed so that
> in the future perhaps hotplug could use it if it made sense.
>
> qemuBuildSecretAESCommandLine (private)
> Generate and add to the command line the -object secret for the
> AES secret. This will be required for the subsequent RBD reference
> to the object.
>
> qemuBuildDiskSecinfoCommandLine (private)
> Handle adding the AES secret object.
>
> Adjustments:
>
> Command Building:
> Adjust the qemuBuildRBDSecinfoURI API's in order to generate the
> specific command options for an AES secret, such as:
>
> -object secret,id=$alias,keyid=$masterKey,data=$base64encodedencrypted,
> format=base64
> -drive file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
> mon_host=mon1.example.org\:6321,password-secret=$alias,...
>
> where the 'id=' value is the secret object alias generated by
> concatenating the disk alias and "-aesKey0". The 'keyid= $masterKey'
> is the master key shared with qemu, and the -drive syntax will
> reference that alias as the 'password-secret'. For the -drive
> syntax, the 'id=myname' is kept to define the username, while the
> 'key=$base64 encoded secret' is removed.
>
> While according to the syntax described for qemu commit '60390a21'
> or as seen in the email archive:
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04083.html
>
> it is possible to pass a plaintext password via a file, the qemu
> commit 'ac1d8878' describes the more feature rich 'keyid=' option
> based upon the shared masterKey.
>
> qemu_domain.c
> Use the qemuDomainSecretSetup in qemuDomainSecretDiskPrepare
> and qemuDomainSecretHostdevPrepare to setup the secret rather
> than assuming plain secret setup.
>
> Add test for checking/comparing output.
>
> NB: For hotplug, since the hotplug code doesn't add command line
> arguments, passing the encoded secret directly to the monitor
> will suffice.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> src/qemu/qemu_command.c | 122 ++++++++++++++++++++-
> src/qemu/qemu_domain.c | 16 +--
> ...muxml2argv-disk-drive-network-rbd-auth-AES.args | 31 ++++++
> ...emuxml2argv-disk-drive-network-rbd-auth-AES.xml | 42 +++++++
> tests/qemuxml2argvtest.c | 2 +
> 5 files changed, 204 insertions(+), 9 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.xml
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4a2fa1d..e2e0b3c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -616,6 +616,110 @@ qemuNetworkDriveGetPort(int protocol,
> }
>
>
> +/**
> + * qemuBuildSecretInfoProps:
> + * @secinfo: pointer to the secret info object
> + * @type: returns a pointer to a character string for object name
> + * @props: json properties to return
> + *
> + * Build the JSON properties for the secret info type.
> + *
> + * Returns 0 on success with the filled in JSON property; otherwise,
> + * returns -1 on failure error message set.
> + */
> +static int
> +qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
> + const char **type,
> + virJSONValuePtr *propsret)
> +{
> + int ret = -1;
> + char *keyid = NULL;
> + virJSONValuePtr props = NULL;
> +
> + *type = "secret";
> +
> + if (!(keyid = qemuDomainGetMasterKeyAlias()))
> + return -1;
> +
> + if (virJSONValueObjectCreate(&props,
Using 'propsret' directly avoids the temp variable.
> + "s:data", secinfo->s.aes.ciphertext,
> + "s:keyid", keyid,
> + "s:iv", secinfo->s.aes.iv,
> + "s:format", "base64", NULL) < 0)
> + goto cleanup;
> +
> + *propsret = props;
> + props = NULL;
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(keyid);
> + virJSONValueFree(props);
This is dead code. props will either be NULL if virJSONValueObjectCreate
succeeds and thus is returned or will be NULL if
virJSONValueObjectCreate fails as no other code is jumping to cleanup.
> +
> + return ret;
> +}
> +
> +
> +/**
> + * qemuBuildSecretAESCommandLine:
qemuBuildObjectSecretCommandLine perhaps? At this point it doesn't
matter which algorithm was used since that is encoded in secinfo and
qemuBuildSecretInfoProps should do the correct thing encoding the type
right away. This code is building the object command line for a object
type = secret.
> + * @cmd: the command to modify
> + * @secinfo: pointer to the secret info object
> + *
> + * If the secinfo is available and associated with an AES secret,
> + * then format the command line for the secret object. This object
> + * will be referenced by the device that needs/uses it, so it needs
> + * to be in place first.
> + *
> + * Returns 0 on success, -1 w/ error message on failure
> + */
> +static int
> +qemuBuildSecretAESCommandLine(virCommandPtr cmd,
> + qemuDomainSecretInfoPtr secinfo)
> +{
> + int ret = -1;
> + virJSONValuePtr props = NULL;
> + const char *type;
> + char *tmp = NULL;
> +
> + if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0)
> + return -1;
> +
> + if (!(tmp = qemuBuildObjectCommandlineFromJSON(type, secinfo->s.aes.alias,
> + props)))
> + goto cleanup;
> +
> + virCommandAddArgList(cmd, "-object", tmp, NULL);
> + ret = 0;
> +
> + cleanup:
> + virJSONValueFree(props);
> + VIR_FREE(tmp);
> +
> + return ret;
> +}
> +
> +
> +/* qemuBuildDiskSecinfoCommandLine:
> + * @cmd: Pointer to the command string
> + * @secinfo: Pointer to a possible secinfo
> + *
> + * Add the secret object for the disks that will be using it to perform
> + * their authentication.
> + *
> + * Returns 0 on success, -1 w/ error on some sort of failure.
> + */
> +static int
> +qemuBuildDiskSecinfoCommandLine(virCommandPtr cmd,
> + qemuDomainSecretInfoPtr secinfo)
> +{
> + /* Not necessary for non AES secrets */
> + if (!secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_TYPE_AES)
> + return 0;
> +
> + return qemuBuildSecretAESCommandLine(cmd, secinfo);
> +}
> +
> +
> /* qemuBuildGeneralSecinfoURI:
> * @uri: Pointer to the URI structure to add to
> * @secinfo: Pointer to the secret info data (if present)
> @@ -697,6 +801,10 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_SECRET_INFO_TYPE_AES:
> + virBufferEscape(buf, '\\', ":", ":id=%s:auth_supported=cephx\\;none",
> + secinfo->s.aes.username);
> + break;
> +
> case VIR_DOMAIN_SECRET_INFO_TYPE_LAST:
> return -1;
> }
> @@ -1094,6 +1202,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
> char *source = NULL;
> int actualType = virStorageSourceGetActualType(disk->src);
> qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
>
> if (idx < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -1175,7 +1284,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
> break;
> }
>
> - if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) < 0)
> + if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0)
> goto error;
>
> if (source &&
> @@ -1227,6 +1336,12 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
> qemuBufferEscapeComma(&opt, source);
> virBufferAddLit(&opt, ",");
>
> + if (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
Looks like this point will not be specific for RBD but rather for
anything using the secret object to store the password so the first part
of the condition should not be necessary.
> + secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> + virBufferAsprintf(&opt, "password-secret=%s,",
> + secinfo->s.aes.alias);
> + }
> +
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160518/e8e68c19/attachment-0001.sig>
More information about the libvir-list
mailing list