diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6583afb27c3a6127759ec6685c09c6afa7f15ea9..c399f5cfb995ac12173140d07ce5262342b1c27b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1021,7 +1021,9 @@ static int libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) { virConnectPtr conn = NULL; - char *secret = NULL; + uint8_t *secret = NULL; + char *base64secret = NULL; + size_t secretlen = 0; char *username = NULL; int ret = -1; @@ -1031,20 +1033,24 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) if (!(conn = virConnectOpen("xen:///system"))) goto cleanup; - if (!(secret = virSecretGetSecretString(conn, - true, - src->auth, - VIR_SECRET_USAGE_TYPE_CEPH))) + if (virSecretGetSecretString(conn, src->auth, + VIR_SECRET_USAGE_TYPE_CEPH, + &secret, &secretlen) < 0) + goto cleanup; + + /* RBD expects an encoded secret */ + if (!(base64secret = virStringEncodeBase64(secret, secretlen))) goto cleanup; } - if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, secret))) + if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, base64secret))) goto cleanup; ret = 0; cleanup: - VIR_FREE(secret); + VIR_DISPOSE_N(secret, secretlen); + VIR_DISPOSE_STRING(base64secret); virObjectUnref(conn); return ret; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d6d5f8ba9b7ce7efe0a8a1fb0e630a49f8d4c9e..7e17521f39adaa5d475df37b8d59e0448a215722 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -628,6 +628,12 @@ qemuBuildGeneralSecinfoURI(virURIPtr uri, switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: if (secinfo->s.plain.secret) { + if (!virStringBufferIsPrintable(secinfo->s.plain.secret, + secinfo->s.plain.secretlen)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found non printable characters in secret")); + return -1; + } if (virAsprintf(&uri->user, "%s:%s", secinfo->s.plain.username, secinfo->s.plain.secret) < 0) @@ -662,6 +668,8 @@ static int qemuBuildRBDSecinfoURI(virBufferPtr buf, qemuDomainSecretInfoPtr secinfo) { + char *base64secret = NULL; + if (!secinfo) { virBufferAddLit(buf, ":auth_supported=none"); return 0; @@ -669,11 +677,14 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: - virBufferEscape(buf, '\\', ":", ":id=%s", - secinfo->s.plain.username); + if (!(base64secret = virStringEncodeBase64(secinfo->s.plain.secret, + secinfo->s.plain.secretlen))) + return -1; + virBufferEscape(buf, '\\', ":", ":id=%s", secinfo->s.plain.username); virBufferEscape(buf, '\\', ":", ":key=%s:auth_supported=cephx\\;none", - secinfo->s.plain.secret); + base64secret); + VIR_DISPOSE_STRING(base64secret); break; case VIR_DOMAIN_SECRET_INFO_TYPE_AES: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fcd21ab8b856963d596eacb344fe25ce8fb5ff7b..0733872896a6354afebc7cb3b286aa7aafb97004 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -731,8 +731,7 @@ static void qemuDomainSecretPlainClear(qemuDomainSecretPlain secret) { VIR_FREE(secret.username); - memset(secret.secret, 0, strlen(secret.secret)); - VIR_FREE(secret.secret); + VIR_DISPOSE_N(secret.secret, secret.secretlen); } @@ -870,24 +869,18 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, virStorageNetProtocol protocol, virStorageAuthDefPtr authdef) { - bool encode = false; int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN; if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) return -1; - if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - /* qemu requires the secret to be encoded for RBD */ - encode = true; + if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) secretType = VIR_SECRET_USAGE_TYPE_CEPH; - } - - if (!(secinfo->s.plain.secret = - virSecretGetSecretString(conn, encode, authdef, secretType))) - return -1; - return 0; + return virSecretGetSecretString(conn, authdef, secretType, + &secinfo->s.plain.secret, + &secinfo->s.plain.secretlen); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index dd90e67cde04387559950c87a15bc00592dacfb9..baf8bd880c5195d3bd5c21660f17e8759a2751e8 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -251,7 +251,8 @@ typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain; typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr; struct _qemuDomainSecretPlain { char *username; - char *secret; + uint8_t *secret; + size_t secretlen; }; # define QEMU_DOMAIN_AES_IV_LEN 16 /* 16 bytes for 128 bit random */ diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c index d69f7ba9e0757bca4c8b53801fcfaf67039410d4..560240164d54a9e059ffd1b8b1c1a635c457b97b 100644 --- a/src/secret/secret_util.c +++ b/src/secret/secret_util.c @@ -27,7 +27,6 @@ #include "virlog.h" #include "virobject.h" #include "viruuid.h" -#include "base64.h" #include "datatypes.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -37,25 +36,26 @@ VIR_LOG_INIT("secret.secret_util"); /* virSecretGetSecretString: * @conn: Pointer to the connection driver to make secret driver call - * @encoded: Whether the returned secret needs to be base64 encoded * @authdef: Pointer to the disk storage authentication * @secretUsageType: Type of secret usage for authdef lookup + * @secret: returned secret as a sized stream of unsigned chars + * @secret_size: Return size of the secret - either raw text or base64 * - * Lookup the secret for the authdef usage type and return it either as - * raw text or encoded based on the caller's need. + * Lookup the secret for the authdef usage type and return it as raw text. + * It is up to the caller to encode the secret further. * - * Returns a pointer to memory that needs to be cleared and free'd after - * usage or NULL on error. + * Returns 0 on success, -1 on failure. On success the memory in secret + * needs to be cleared and free'd after usage. */ -char * +int virSecretGetSecretString(virConnectPtr conn, - bool encoded, virStorageAuthDefPtr authdef, - virSecretUsageType secretUsageType) + virSecretUsageType secretUsageType, + uint8_t **secret, + size_t *secret_size) { - size_t secret_size; virSecretPtr sec = NULL; - char *secret = NULL; + int ret = -1; switch (authdef->secretType) { case VIR_STORAGE_SECRET_TYPE_UUID: @@ -71,25 +71,15 @@ virSecretGetSecretString(virConnectPtr conn, if (!sec) goto cleanup; - secret = (char *)conn->secretDriver->secretGetValue(sec, &secret_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); + *secret = conn->secretDriver->secretGetValue(sec, secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); - if (!secret) + if (!*secret) goto cleanup; - if (encoded) { - char *base64 = NULL; - - base64_encode_alloc(secret, secret_size, &base64); - VIR_FREE(secret); - if (!base64) { - virReportOOMError(); - goto cleanup; - } - secret = base64; - } + ret = 0; cleanup: virObjectUnref(sec); - return secret; + return ret; } diff --git a/src/secret/secret_util.h b/src/secret/secret_util.h index 00864493a3646af20cbe37820ae1ee448d3df17f..a03966298c5e5b97c2d53474bc6f9976e17143e2 100644 --- a/src/secret/secret_util.h +++ b/src/secret/secret_util.h @@ -25,9 +25,11 @@ # include "internal.h" # include "virstoragefile.h" -char *virSecretGetSecretString(virConnectPtr conn, - bool encoded, - virStorageAuthDefPtr authdef, - virSecretUsageType secretUsageType) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +int virSecretGetSecretString(virConnectPtr conn, + virStorageAuthDefPtr authdef, + virSecretUsageType secretUsageType, + uint8_t **ret_secret, + size_t *ret_secret_size) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_SECRET_H__ */