On Thu, May 12, 2016 at 08:39:28 -0400, John Ferlan wrote:
On 05/12/2016 08:26 AM, Peter Krempa wrote:
On Thu, May 12, 2016 at 07:49:31 -0400, John Ferlan wrote:
Rather than returning a "char *" indicating perhaps some sized set of characters that is NUL terminated, return the value as "uint8_t *" indicating a stream of raw bytes. In doing so, we also need to return the size of the secret returned.
Alter the callers to handle the adjusted model.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_conf.c | 18 +++++++++++------- src/qemu/qemu_command.c | 7 ++++--- src/qemu/qemu_domain.c | 5 +++-- src/qemu/qemu_domain.h | 3 ++- src/secret/secret_util.c | 19 +++++++++++++++---- src/secret/secret_util.h | 13 +++++++------ 6 files changed, 42 insertions(+), 23 deletions(-)
[...]
Not needed here, you can just discard the length.
Ironically what I had originally; however, in an oh sh* moment I altered things... It does mean though, that the "secret" in both instances will either need to be "memcpy()'d" into an allocated "char *" buffer or just cast (const char *). Is there a preference?
I think the correct approach would be that virSecretGetSecretString is returning just the binary buffer and the code using it is handling the correct encoding ... [1] [...]
diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c index 217584f..edc1104 100644 --- a/src/secret/secret_util.c +++ b/src/secret/secret_util.c @@ -41,6 +41,7 @@ VIR_LOG_INIT("secret.secret_util"); * @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 + * @ret_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. @@ -48,17 +49,19 @@ VIR_LOG_INIT("secret.secret_util"); * Returns a pointer to memory that needs to be cleared and free'd after * usage or NULL on error. */ -char * +uint8_t * virSecretGetSecretString(virConnectPtr conn, const char *scheme, bool encoded,
Shouldn't we drop this argument altogether and encode the data to base64 when it's used? That way this will work for all general secrets without the duality that the returned bufer is either binary or base64 encoded and for other encodings it will be encoded at the point where it's used.
iSCSI uses/expects the unencoded value - that's one of the factors behind the need for IV/AES secrets. Unfortunately, the insane way one has to add the secret to the command line via the '-iscsi' option makes it impossible since there's no way to add a unique "-id" as well without a colon (and/or slash) since that's the way an IQN is described (iqn.date.reverse-host-name:storage-specific-string).
The current algorithm for iSCSI in QEMU uses that plaintext password instead of an encoded one. Competing requirements.
In that case you need a function that checks the buffer containing the secret if it can be put in plaintext on the commandline. After that you can treat it as a string and put it on the commandline. Of course if it's not printable you need to report an error since there's no alternate way to handle arbitrary strings. [1] ... With the approach outlined above both iSCSI and RBD will have the same semantics. You get it as a binary string. You encode (make sure it's encoded) properly at the point where it's used and then pass it on to qemu. Peter