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 APIs: qemu_domain.c: qemuDomainGetSecretAESAlias: Generate/return the secret object alias for an AES Secret Info type. This will be called from qemuDomainSecretAESSetup. qemuDomainSecretAESSetup: (private) This API handles the details of the generation of the AES secret and saves the pieces that need to be passed to qemu in order for the secret to be decrypted. The encrypted secret based upon the domain master key, an initialization vector (16 byte random value), and the stored secret. Finally, the requirement from qemu is the IV and encrypted secret are to be base64 encoded. 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. qemuBuildObjectSecretCommandLine (private) Generate and add to the command line the -object secret for the secret. This will be required for the subsequent RBD reference to the object. qemuBuildDiskSecinfoCommandLine (private) Handle adding the AES secret object. Adjustments: qemu_domain.c: The qemuDomainSecretSetup was altered to call either the AES or Plain Setup functions based upon whether AES secrets are possible (we have the encryption API) or not, we have secrets, and of course if the protocol source is RBD. qemu_command.c: 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. Add tests 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@xxxxxxxxxx> --- src/qemu/qemu_alias.c | 23 ++++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 117 ++++++++++++++++++- src/qemu/qemu_domain.c | 127 +++++++++++++++++++-- ...muxml2argv-disk-drive-network-rbd-auth-AES.args | 31 +++++ ...emuxml2argv-disk-drive-network-rbd-auth-AES.xml | 42 +++++++ tests/qemuxml2argvmock.c | 16 +++ tests/qemuxml2argvtest.c | 5 +- 8 files changed, 354 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_alias.c b/src/qemu/qemu_alias.c index cb102ec..d624071 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -482,3 +482,26 @@ qemuDomainGetMasterKeyAlias(void) return alias; } + + +/* qemuDomainGetSecretAESAlias: + * + * Generate and return an alias for the encrypted secret + * + * Returns NULL or a string containing the alias + */ +char * +qemuDomainGetSecretAESAlias(const char *srcalias) +{ + char *alias; + + if (!srcalias) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("encrypted secret alias requires valid source alias")); + return NULL; + } + + ignore_value(virAsprintf(&alias, "%s-secret0", srcalias)); + + return alias; +} diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 2d7bc9b..e328a9b 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -69,4 +69,6 @@ char *qemuAliasFromDisk(const virDomainDiskDef *disk); char *qemuDomainGetMasterKeyAlias(void); +char *qemuDomainGetSecretAESAlias(const char *srcalias); + #endif /* __QEMU_ALIAS_H__*/ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a2fa1d..316aab9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -616,6 +616,106 @@ 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; + + *type = "secret"; + + if (!(keyid = qemuDomainGetMasterKeyAlias())) + return -1; + + if (virJSONValueObjectCreate(propsret, + "s:data", secinfo->s.aes.ciphertext, + "s:keyid", keyid, + "s:iv", secinfo->s.aes.iv, + "s:format", "base64", NULL) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(keyid); + + return ret; +} + + +/** + * qemuBuildObjectSecretCommandLine: + * @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 +qemuBuildObjectSecretCommandLine(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 qemuBuildObjectSecretCommandLine(cmd, secinfo); +} + + /* qemuBuildGeneralSecinfoURI: * @uri: Pointer to the URI structure to add to * @secinfo: Pointer to the secret info data (if present) @@ -697,6 +797,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 +1198,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 +1280,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 +1332,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, qemuBufferEscapeComma(&opt, source); virBufferAddLit(&opt, ","); + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + virBufferAsprintf(&opt, "password-secret=%s,", + secinfo->s.aes.alias); + } + if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) virBufferAsprintf(&opt, "format=%s,", @@ -1906,6 +2016,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk = def->disks[i]; bool withDeviceArg = false; bool deviceFlagMasked = false; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; /* Unless we have -device, then USB disks need special handling */ @@ -1948,6 +2060,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, break; } + if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) + return -1; + virCommandAddArg(cmd, "-drive"); /* Unfortunately it is not possible to use diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 754536e..98fc337 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -835,23 +835,136 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, } +/* qemuDomainSecretAESSetup: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @secinfo: Pointer to secret info + * @srcalias: Alias of the disk/hostdev used to generate the secret alias + * @protocol: Protocol for secret + * @authdef: Pointer to auth data + * + * Taking a secinfo, fill in the AES specific information using the + * + * Returns 0 on success, -1 on failure with error message + */ +static int +qemuDomainSecretAESSetup(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + qemuDomainSecretInfoPtr secinfo, + const char *srcalias, + virStorageNetProtocol protocol, + virStorageAuthDefPtr authdef) +{ + int ret = -1; + uint8_t *raw_iv = NULL; + size_t ivlen = QEMU_DOMAIN_AES_IV_LEN; + uint8_t *secret = NULL; + size_t secretlen = 0; + uint8_t *ciphertext = NULL; + size_t ciphertextlen = 0; + int secretType = VIR_SECRET_USAGE_TYPE_NONE; + + secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; + if (VIR_STRDUP(secinfo->s.aes.username, authdef->username) < 0) + return -1; + + switch ((virStorageNetProtocol)protocol) { + case VIR_STORAGE_NET_PROTOCOL_RBD: + secretType = VIR_SECRET_USAGE_TYPE_CEPH; + break; + + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' cannot be used for encrypted secrets"), + virStorageNetProtocolTypeToString(protocol)); + return -1; + } + + if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) + return -1; + + /* Create a random initialization vector */ + if (!(raw_iv = virCryptoGenerateRandom(ivlen))) + return -1; + + /* Encode the IV and save that since qemu will need it */ + if (!(secinfo->s.aes.iv = virStringEncodeBase64(raw_iv, ivlen))) + goto cleanup; + + /* Grab the unencoded secret */ + if (virSecretGetSecretString(conn, authdef, secretType, + &secret, &secretlen) < 0) + goto cleanup; + + if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC, + priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN, + raw_iv, ivlen, secret, secretlen, + &ciphertext, &ciphertextlen) < 0) + goto cleanup; + + /* Clear out the secret */ + memset(secret, 0, secretlen); + + /* Now encode the ciphertext and store to be passed to qemu */ + if (!(secinfo->s.aes.ciphertext = virStringEncodeBase64(ciphertext, + ciphertextlen))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_DISPOSE_N(raw_iv, ivlen); + VIR_DISPOSE_N(secret, secretlen); + VIR_DISPOSE_N(ciphertext, ciphertextlen); + + return ret; +} + + /* qemuDomainSecretSetup: * @conn: Pointer to connection + * @priv: pointer to domain private object * @secinfo: Pointer to secret info + * @srcalias: Alias of the disk/hostdev used to generate the secret alias * @protocol: Protocol for secret * @authdef: Pointer to auth data * - * A shim to call plain setup. + * If we have the encryption API present and can support a secret object, then + * build the AES secret; otherwise, build the Plain secret. This is the magic + * decision point for utilizing the AES secrets for an RBD disk. For now iSCSI + * disks and hostdevs will not be able to utilize this mechanism. * * Returns 0 on success, -1 on failure */ static int qemuDomainSecretSetup(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, qemuDomainSecretInfoPtr secinfo, + const char *srcalias, virStorageNetProtocol protocol, virStorageAuthDefPtr authdef) { - return qemuDomainSecretPlainSetup(conn, secinfo, protocol, authdef); + if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && + protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + if (qemuDomainSecretAESSetup(conn, priv, secinfo, + srcalias, protocol, authdef) < 0) + return -1; + } else { + if (qemuDomainSecretPlainSetup(conn, secinfo, protocol, authdef) < 0) + return -1; + } + return 0; } @@ -883,7 +996,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) */ int qemuDomainSecretDiskPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, + qemuDomainObjPrivatePtr priv, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; @@ -900,8 +1013,8 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, if (VIR_ALLOC(secinfo) < 0) return -1; - if (qemuDomainSecretSetup(conn, secinfo, src->protocol, - src->auth) < 0) + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, + src->protocol, src->auth) < 0) goto error; diskPriv->secinfo = secinfo; @@ -944,7 +1057,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev) */ int qemuDomainSecretHostdevPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, + qemuDomainObjPrivatePtr priv, virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; @@ -965,7 +1078,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, if (VIR_ALLOC(secinfo) < 0) return -1; - if (qemuDomainSecretSetup(conn, secinfo, + if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, VIR_STORAGE_NET_PROTOCOL_ISCSI, iscsisrc->auth) < 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args new file mode 100644 index 0000000..7100d2d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-object secret,id=virtio-disk0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive 'file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\ +mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\ +password-secret=virtio-disk0-secret0,format=raw,if=none,id=drive-virtio-disk0' \ +-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\ +id=virtio-disk0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.xml new file mode 100644 index 0000000..ac2e942 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <auth username='myname'> + <secret type='ceph' usage='mycluster_myname'/> + </auth> + <source protocol='rbd' name='pool/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 1616eed..c6a1f98 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -21,11 +21,14 @@ #include <config.h> #include "internal.h" +#include "viralloc.h" #include "vircommand.h" +#include "vircrypto.h" #include "virmock.h" #include "virnetdev.h" #include "virnetdevtap.h" #include "virnuma.h" +#include "virrandom.h" #include "virscsi.h" #include "virstring.h" #include "virtpm.h" @@ -145,3 +148,16 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED, { /* nada */ } + +uint8_t * +virCryptoGenerateRandom(size_t nbytes) +{ + uint8_t *buf; + + if (VIR_ALLOC_N(buf, nbytes) < 0) + return NULL; + + ignore_value(virRandomBytes(buf, nbytes)); + + return buf; +} diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3bfb5c4..fd15186 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -799,6 +799,8 @@ mymain(void) DO_TEST("disk-drive-network-rbd", NONE); DO_TEST("disk-drive-network-sheepdog", NONE); DO_TEST("disk-drive-network-rbd-auth", NONE); + DO_TEST("disk-drive-network-rbd-auth-AES", + QEMU_CAPS_OBJECT_SECRET); DO_TEST("disk-drive-network-rbd-ipv6", NONE); DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE); DO_TEST("disk-drive-no-boot", @@ -1980,7 +1982,8 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/qemuxml2argvmock.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/qemuxml2argvmock.so," + abs_builddir "/.libs/virrandommock.so") #else -- 2.5.5 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list