[libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file

[Daniel P. Berrangé]

On Tue, Jan 21, 2020 at 02:43:44PM +0100, Peter Krempa wrote:
> On Tue, Jan 21, 2020 at 13:38:13 +0000, Daniel Berrange wrote:
> > On Fri, Jan 10, 2020 at 04:42:43PM +0100, Peter Krempa wrote:
> > > The necessity to specify the secret value as command argument is
> > > insecure. Allow reading the secret from a file.
> > > 
> > > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > > ---
> > >  docs/manpages/virsh.rst |  5 +++--
> > >  tools/virsh-secret.c    | 30 +++++++++++++++++++++++++++---
> > >  2 files changed, 30 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> > > index fcc8ef6758..992b1daf90 100644
> > > --- a/docs/manpages/virsh.rst
> > > +++ b/docs/manpages/virsh.rst
> > > @@ -6558,10 +6558,11 @@ secret-set-value
> > > 
> > >  .. code-block::
> > > 
> > > -   secret-set-value secret base64
> > > +   secret-set-value secret (--file filename | base64)
> > > 
> > >  Set the value associated with *secret* (specified by its UUID) to the value
> > > -Base64-encoded value *base64*.
> > > +Base64-encoded value *base64* or from file named *filename*. Note that *--file*
> > > +and *base64* options are mutually exclusive.
> > 
> > You added a --plain option to secret-get-value.
> > 
> > It would naturally suggest that we do the same here, then we can
> > support
> > 
> >   secret-set-value $BASE64STR
> >   secret-set-value --plain $RAWSTR
> 
> I think that both of the above should not have existed in the first
> place. Adding the possibility to add plain secrets via argument looks to
> me as a step back. If I could do it, I'd remove the base64 via command
> line arguments as well.

True, we probably should forbid  --plain RAWSTR.

Removing existing option is not viable, but perhaps we can at least print
a warning message to stderr 

> >   secret-set-value --file FILENAME-WITH-BASE64-STR
> 
> This seems a bit pointless to me.

Bear in mind that the place you are getting the secret from may already
be giving it to you in base64 format, so it is useful to write it to the
file as-is, as you would do today when using set-value base64

> >   secret-set-value --plain --file FILENAME-WITH-RAW-STR

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|