secret: Alter virSecretGetSecretString

Rather than returning a "char *" indicating perhaps some sized set of
characters that is NUL terminated, alter the function to return 0 or -1
for success/failure and add two parameters to handle returning the
buffer and it's size.

The function no longer encodes the returned secret, rather it returns
the unencoded secret forcing callers to make the necessary adjustments.

Alter the callers to handle the adjusted model.

Signed-off-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6583afb27c3a6127759ec6685c09c6afa7f15ea9..c399f5cfb995ac12173140d07ce5262342b1c27b 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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__ */