[libvirt PATCH 0/5] Formalize the deprecation of arguments in virsh

[Tim Wiederhake]

On Tue, 2021-03-23 at 15:28 +0100, Peter Krempa wrote:
> On Tue, Mar 23, 2021 at 15:19:44 +0100, Michal Privoznik wrote:
> > On 3/23/21 3:04 PM, Peter Krempa wrote:
> > > On Tue, Mar 23, 2021 at 14:50:09 +0100, Michal Privoznik wrote:
> > > > On 3/23/21 2:42 PM, Daniel P. Berrangé wrote:
> > > > > On Tue, Mar 23, 2021 at 02:36:19PM +0100, Michal Privoznik
> > > > > wrote:
> 
> [...]
> 
> > > The only thing that IMO should be removed but I didn't for
> > > compatibility
> > > is the 'secret-set-value's 'base64' parameter as that is
> > > insecure. There
> > > isn't a compatible replacement though.
> > > 
> > 
> > That's debatable. Its not much worse than reading from a file. I
> > mean, who
> > has access to my $HISTFILE? Only me and root and in both cases the
> > secret
> 
> It's not about HISTFILE, but about the process listing. On a default
> linux box, all users can list all other user's processes. If your
> password is an argument for a command, it will be readable for other
> users without the access to your directory.
> 
> Arguably, the lifetime of virsh is very short, so it's extremely
> unlikely for anyone to notice, but it's insecure regardless.
> 
> > can be changed or read from the file (if the file is not deleted
> > right away,
> > and even then it could be recovered). Many tools accept passwords
> > in clear
> > text on cmd line (e.g. curl, wget). If anything, we could document
> > why
> 
> You should avoid use of those arguments if you are on a multi-user
> box.
> 
> > --base64 is dangerous (if we haven't done so yet).
> 
> It is documented as such and also prints a warning as pointed out in
> the
> other reply.
> 

Hi all,

thank you for your feedback!

My motivation for starting this patch series was the desire to change
the behavior of the virsh commands "create", "define", "snapshot-
create", "cpu-compare", and "hypervisor-cpu-compare": Currently, those
commands accept an "--validate" argument that enables RNG schema
validation for the provided XML. In my opinion, this should be the
default behavior. This series was supposed to lay the ground work, a
formal deprecation system for commands and arguments.

I understand that suddenly rejecting invalid XML where it was accepted
previously and seemingly did the right thing breaks backwards
compatibility, even if the only affected users are those with invalid
XML which should be fixed anyway. To that end, I wrote a small set of
patches that introduce a  "--no-validate" argument to these commands,
mark that flag as deprecated, and only then make the switch for
enabling validation by default.

For users with valid XML, this change is invisible. Users with invalid
XML are being made aware that their XML is broken, but given the
opportunity to continue operations by specifying "--no-validate" until
their XML is fixed.

Once a certain amount of time has passed, e.g. "two major releases" or
"all officially supported distributions carry libvirt versions that
have XML validation on by default", the deprecated "--no-validate"
flags can be deleted.

But alas, I was unaware of https://libvirt.org/support.html#virsh,
(thanks Daniel!) I did not find this when grepping for
"deprecat(ed|ion)": "Existing commands and arguments will not be
removed or renamed". This guarantee prevents any change of this series
or the unpublished one described above.

("Infinite" stability guarantees place an undue maintenance burden on
libvirt as it prevents developers from fixing / removing outdated or
insecure commands and arguments, in my opinion; but that is besides the
point here.)

I will abandon this series and retry with a version that simply outputs
a warning if the user provides XML data without also using "
--validate", similar to the one for "--base64".

Cheers,
Tim