Re: [libvirt] [PATCH 2/2] qemu: Use secret objects to pass iSCSI passwords
On 09/12/2017 09:36 AM, Peter Krempa wrote:
On Tue, Sep 05, 2017 at 15:09:35 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1425757
>
> The blockdev-add code provides a mechanism to sanely provide user
> and password-secret arguments for iscsi without placing them on the
> command line to be viewable by a 'ps -ef' type command or needing
> to create separate -iscsi devices for each disk/volume found.
>
> So modify the iSCSI command line building to check for the presence
> of the capability in order properly setup and use the domain master
> secret object to encrypt the password in a secret object and alter
> the parameters for the command line to utilize.
>
> Modify the xml2argvtest to exhibit the syntax for both disk and
> hostdev configurations.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 19 ++++++++-
> src/qemu/qemu_domain.c | 4 ++
> ...xml2argv-disk-drive-network-iscsi-auth-AES.args | 39 ++++++++++++++++++
> ...uxml2argv-disk-drive-network-iscsi-auth-AES.xml | 43 +++++++++++++++++++
> ...ml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args | 35 ++++++++++++++++
> ...xml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml | 48 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 10 +++++
> 7 files changed, 196 insertions(+), 2 deletions(-)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.args
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.xml
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml
Most of the stuff here looks reasonable but I don't think we should mix
the URI syntax with the file.param= syntax generated from the JSON
objects. Since there's a capability when this is supported, the command
line generator should use the new syntax.
You can mark it in qemuDiskSourceNeedsProps so that it uses the new
generator if it's needed and supported and implement the JSON generator.
The rest should then work as expected.
This is certainly where things didn't exactly match up the way I thought
you have desired the virstoragefile and virstoragetest code to work.
In particular, it's the "user" and "password-secret" options that
cause
the disconnect since they're a @disk private object and not a @disk->src
object. I considered a number of different ways to cheat, but came up
empty on each, so I just followed the existing RBD code.
One cannot reconstruct the <auth> element properly given that all the
arguments have is a username and an alias, but would need to have either
a "usage" or "uuid" string. The secret object doesn't contain
that
either, so it'd need to be stored somehow.
Perhaps if the @secinfo moved from private into source that would help,
although perhaps not following the RNG. Still there'd also have to be a
way to save the string used in the original <auth> element used to look
up the secret (either by uuid or by usage). Having the password-secret
alias is OK, but really not useful.
Maybe "some future" adjustment could modify the password-secret alias to
be (for example) "virtio-disk0-%s-secret0" where the %s is either a UUID
or a Usage string of the secret used to generate the object. Perhaps
even the unsigned char UUID too so as to not have too many extra "-"'s
to parse/read. That'd be an awful alias, but serve a purpose as well.
A similar problem exists for RBD, but the RBD code in virstoragefile and
virstoragetest totally ignore the authentication pieces... That syntax
is a bit more hairly because it's only the RBD password-secret field
that needs the "file." (or not if -drive driver=rbd,... was supported).
John