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@xxxxxxxxxx>
> >> ---
> >> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list