Re: [libvirt] [PATCH v3 1/2] qemu: Introduce new Secret IV API's

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(a)redhat.com&gt; > --- > 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(+) >

> +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.