On Thu, May 05, 2016 at 15:00:40 -0400, John Ferlan wrote:
> New APIs:
>
> qemuDomainGetIVKeyAlias:
> Generate/return the secret object alias for an initialization
> vector (IV) secret info type. This will be saved in the secret
> info block. This will be called from qemuDomainSecretIVSetup.
>
> qemuDomainSecretHaveEncrypt: (private)
> Boolean function to determine whether the underly encryption
> API is available. This function will utilize a similar mechanism
> as the 'gnutls_rnd' did in configure.ac.
>
> qemuDomainSecretIVSetup: (private)
> This API handles the details of the generation of the IV 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. They can be passed
> either directly or within a file. This implementation chooses
> to pass directly rather than a file.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> configure.ac | 1 +
> src/qemu/qemu_alias.c | 23 +++++++++
> src/qemu/qemu_alias.h | 2 +
> src/qemu/qemu_domain.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 163 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 88e2e20..3cabd5e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1264,6 +1264,7 @@ if test "x$with_gnutls" != "xno"; then
> ]])
>
> AC_CHECK_FUNCS([gnutls_rnd])
> + AC_CHECK_FUNCS([gnutls_cipher_encrypt])
>
> CFLAGS="$old_CFLAGS"
> LIBS="$old_LIBS"
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index ade2033..de9a74f 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -556,3 +556,26 @@ qemuDomainGetMasterKeyAlias(void)
>
> return alias;
> }
> +
> +
> +/* qemuDomainGetIVKeyAlias:
> + *
> + * Generate and return an initialization vector alias
> + *
> + * Returns NULL or a string containing the IV key alias
> + */
> +char *
> +qemuDomainGetIVKeyAlias(const char *srcalias)
> +{
> + char *alias;
> +
> + if (!srcalias) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("secret iv alias requires valid source alias"));
> + return NULL;
> + }
> +
> + ignore_value(virAsprintf(&alias, "%s-ivKey0", srcalias));
> +
> + return alias;
> +}
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index 2d7bc9b..435747e 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 *qemuDomainGetIVKeyAlias(const char *srcalias);
> +
> #endif /* __QEMU_ALIAS_H__*/
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index cae356c..b174caa 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -893,6 +893,143 @@ qemuDomainSecretPlainSetup(virConnectPtr conn,
> }
>
>
> +/* qemuDomainSecretHaveEncypt:
> + *
> + * Returns true if we can support the encryption code, false otherwise
> + */
> +static bool ATTRIBUTE_UNUSED
> +qemuDomainSecretHaveEncrypt(void)
> +{
> +#ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
> + return true;
> +#else
> + return false;
> +#endif
> +}
> +
> +
> +/* qemuDomainSecretIVSetup:
> + * @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 initialization vector information
> + *
> + * Returns 0 on success, -1 on failure with error message
> + */
> +static int ATTRIBUTE_UNUSED
> +qemuDomainSecretIVSetup(virConnectPtr conn,
> + qemuDomainObjPrivatePtr priv,
> + qemuDomainSecretInfoPtr secinfo,
> + const char *srcalias,
> + virStorageNetProtocol protocol,
> + virStorageAuthDefPtr authdef)
> +{
> + int ret = -1;
> + int rc;
> + uint8_t *raw_iv = NULL;
> + char *secret = NULL;
> + uint8_t *ciphertext = NULL;
> + size_t secretlen = 0, ciphertextlen = 0, paddinglen;
Please one variable per line.
> + int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
Rather than changing to RBD below I think the decision should be done in
a single swithch.
> + const char *protocolstr = virStorageNetProtocolTypeToString(protocol);
> + gnutls_cipher_hd_t handle = NULL;
> + gnutls_datum_t enc_key;
> + gnutls_datum_t iv_key;
This function is compiled even without gnutls.
> +
> + secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_IV;
> + if (VIR_STRDUP(secinfo->s.iv.username, authdef->username) < 0)
> + return -1;
> +
> + if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
> + secretType = VIR_SECRET_USAGE_TYPE_CEPH;
> +
> + if (!(secinfo->s.iv.alias = qemuDomainGetIVKeyAlias(srcalias)))
> + return -1;
> +
> + /* Create a random initialization vector */
> + if (!(raw_iv = qemuDomainGenerateRandomKey(QEMU_DOMAIN_IV_KEY_LEN)))
> + return -1;
> +
> + /* Encode the IV and save that since qemu will need it */
So the initialization vector or IV is a specific of the used cipher. I
don't think we should use it in the names for the secret type.
Either the name of the cipher should be used or just "secret" if it's
unlikey that qemu would add new ciphers.
> + base64_encode_alloc((const char *)raw_iv, QEMU_DOMAIN_IV_KEY_LEN,
> + &secinfo->s.iv.iv);
> + if (!secinfo->s.iv.iv) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to encode initialization vector"));
> + goto cleanup;
> + }
> +
> + /* Grab the unencoded secret */
> + if (!(secret = virSecretGetSecretString(conn, protocolstr, false,
In qemuDomainSecretPlainSetup the secret is requested encoded for RBD,
thus it was possible to use non ascii keys ...
> + authdef, secretType)))
> + goto cleanup;
> +
> + /* Allocate a padded buffer */
> + secretlen = strlen(secret);
... which won't work with this.
> + paddinglen = 16 - (secretlen % 16);
Magic constants?
> + ciphertextlen = secretlen + paddinglen;
VIR_ROUND_UP
> + if (VIR_ALLOC_N(ciphertext, ciphertextlen) < 0)
> + goto cleanup;
> + memcpy(ciphertext, secret, secretlen);
> + memset(ciphertext + secretlen, paddinglen, paddinglen);
You are padding it by the last byte of the padding size? Is this really
desired?
> +
> + /* clear secret so that it doesn't hang around */
> + memset(secret, 0, strlen(secret));
You do it again in cleanup. Is it really necessary?
> +
> + /* Initialize the gnutls cipher */
> + enc_key.size = QEMU_DOMAIN_MASTER_KEY_LEN;
> + enc_key.data = priv->masterKey;
> + iv_key.size = QEMU_DOMAIN_IV_KEY_LEN;
> + iv_key.data = raw_iv;
> + if ((rc = gnutls_cipher_init(&handle, GNUTLS_CIPHER_AES_256_CBC,
> + &enc_key, &iv_key)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to initialize cipher: '%s'"),
> + gnutls_strerror(rc));
> + goto cleanup;
> + }
> +
> + /* Encrypt the secret and free the memory for cipher operations */
> + rc = gnutls_cipher_encrypt(handle, ciphertext, ciphertextlen);
> + gnutls_cipher_deinit(handle);
> + if (rc < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to encrypt the secret: '%s'"),
> + gnutls_strerror(rc));
> + goto cleanup;
> + }
> +
> + /* Now base64 encode the ciphertext and store to be passed to qemu */
> + base64_encode_alloc((const char *)ciphertext, ciphertextlen,
> + &secinfo->s.iv.ciphertext);
> + if (!secinfo->s.iv.ciphertext) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to encode ciphertext"));
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> + cleanup:
> +
> + /* Clear and free the raw_iv, plaintext secret, and ciphertext */
> + memset(raw_iv, 0, QEMU_DOMAIN_IV_KEY_LEN);
> + VIR_FREE(raw_iv);
> +
> + memset(secret, 0, secretlen);
> + VIR_FREE(secret);
> +
> + memset(ciphertext, 0, ciphertextlen);
> + VIR_FREE(ciphertext);
> +
> + return ret;
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list