[libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'

Kashyap Chamarthy posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180601120838.15932-1-kchamart@redhat.com
Test syntax-check passed
src/qemu/qemu.conf | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Kashyap Chamarthy 5 years, 10 months ago
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
---
 src/qemu/qemu.conf | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 31738ff19c..444247cf31 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -403,13 +403,14 @@
 #
 #       user = "qemu"   # A user named "qemu"
 #       user = "+0"     # Super user (uid=0)
+#       user = 'root'   # The 'root' user
 #       user = "100"    # A user named "100" or a user with uid=100
 #
-#user = "root"
+#user = "qemu"
 
 # The group for QEMU processes run by the system instance. It can be
 # specified in a similar way to user.
-#group = "root"
+#group = "qemu"
 
 # Whether libvirt should dynamically change file ownership
 # to match the configured user/group above. Defaults to 1.
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Ján Tomko 5 years, 10 months ago
On Fri, Jun 01, 2018 at 02:08:38PM +0200, Kashyap Chamarthy wrote:
>Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
>---
> src/qemu/qemu.conf | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>

Why?

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Kashyap Chamarthy 5 years, 10 months ago
On Fri, Jun 01, 2018 at 02:24:51PM +0200, Ján Tomko wrote:
> On Fri, Jun 01, 2018 at 02:08:38PM +0200, Kashyap Chamarthy wrote:
> > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> > ---
> > src/qemu/qemu.conf | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> 
> Why?

Dan already answered it great detail:
https://www.redhat.com/archives/libvir-list/2018-June/msg00058.html

I should've written some more text in my commit message.  I just wanted
people to not mistake that the presence of "user = root" as it's safe to
use ("because, look, it's there by default as a comment").

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Daniel P. Berrangé 5 years, 10 months ago
On Fri, Jun 01, 2018 at 02:08:38PM +0200, Kashyap Chamarthy wrote:
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> ---
>  src/qemu/qemu.conf | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 31738ff19c..444247cf31 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -403,13 +403,14 @@
>  #
>  #       user = "qemu"   # A user named "qemu"
>  #       user = "+0"     # Super user (uid=0)
> +#       user = 'root'   # The 'root' user
>  #       user = "100"    # A user named "100" or a user with uid=100
>  #
> -#user = "root"
> +#user = "qemu"
>  
>  # The group for QEMU processes run by the system instance. It can be
>  # specified in a similar way to user.
> -#group = "root"
> +#group = "qemu"
>  
>  # Whether libvirt should dynamically change file ownership
>  # to match the configured user/group above. Defaults to 1.

The reason the config file documents 'root' is because that is what
configure defaults to.  If you pass --with-qemu-user to configure,
we don't update the config file example though, and perhaps we should.

Alternatively should we make configure defualt to 'qemu' instead of
'root', since it is generally considered insane to run QEMU as root.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Peter Krempa 5 years, 10 months ago
On Fri, Jun 01, 2018 at 13:32:20 +0100, Daniel Berrange wrote:
> On Fri, Jun 01, 2018 at 02:08:38PM +0200, Kashyap Chamarthy wrote:
> > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> > ---
> >  src/qemu/qemu.conf | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > index 31738ff19c..444247cf31 100644
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -403,13 +403,14 @@
> >  #
> >  #       user = "qemu"   # A user named "qemu"
> >  #       user = "+0"     # Super user (uid=0)
> > +#       user = 'root'   # The 'root' user
> >  #       user = "100"    # A user named "100" or a user with uid=100
> >  #
> > -#user = "root"
> > +#user = "qemu"
> >  
> >  # The group for QEMU processes run by the system instance. It can be
> >  # specified in a similar way to user.
> > -#group = "root"
> > +#group = "qemu"
> >  
> >  # Whether libvirt should dynamically change file ownership
> >  # to match the configured user/group above. Defaults to 1.
> 
> The reason the config file documents 'root' is because that is what
> configure defaults to.  If you pass --with-qemu-user to configure,
> we don't update the config file example though, and perhaps we should.
> 
> Alternatively should we make configure defualt to 'qemu' instead of
> 'root', since it is generally considered insane to run QEMU as root.

But user 'qemu' is not by default present on all systems. Even the
libvirt.spec file creates the account.

As a second thought, we generally use commented-out bits that are the
non-default configuration. So this fits the pattern in the extent that
any sane distro specified it's own user/group using the configure
options and if for any reason the user wants to run this as root it's
done just by uncommenting it.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Daniel P. Berrangé 5 years, 10 months ago
On Fri, Jun 01, 2018 at 02:46:40PM +0200, Peter Krempa wrote:
> On Fri, Jun 01, 2018 at 13:32:20 +0100, Daniel Berrange wrote:
> > On Fri, Jun 01, 2018 at 02:08:38PM +0200, Kashyap Chamarthy wrote:
> > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> > > ---
> > >  src/qemu/qemu.conf | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > > index 31738ff19c..444247cf31 100644
> > > --- a/src/qemu/qemu.conf
> > > +++ b/src/qemu/qemu.conf
> > > @@ -403,13 +403,14 @@
> > >  #
> > >  #       user = "qemu"   # A user named "qemu"
> > >  #       user = "+0"     # Super user (uid=0)
> > > +#       user = 'root'   # The 'root' user
> > >  #       user = "100"    # A user named "100" or a user with uid=100
> > >  #
> > > -#user = "root"
> > > +#user = "qemu"
> > >  
> > >  # The group for QEMU processes run by the system instance. It can be
> > >  # specified in a similar way to user.
> > > -#group = "root"
> > > +#group = "qemu"
> > >  
> > >  # Whether libvirt should dynamically change file ownership
> > >  # to match the configured user/group above. Defaults to 1.
> > 
> > The reason the config file documents 'root' is because that is what
> > configure defaults to.  If you pass --with-qemu-user to configure,
> > we don't update the config file example though, and perhaps we should.
> > 
> > Alternatively should we make configure defualt to 'qemu' instead of
> > 'root', since it is generally considered insane to run QEMU as root.
> 
> But user 'qemu' is not by default present on all systems. Even the
> libvirt.spec file creates the account.

Yes, that's the reason configure defaults to 'root', but I really hate
the fact that we default to a config that no one should ever run in
practice.

We could check for existance of 'qemu' in configure and complain if
it is missing, but that's painful in itself as it is valid to build
on a host without the user, as long as it exists at runtime.

I tend to think we should just blindly use qemu/qemu by default and
document that creating these accounts is a requirement. Users will
quickly see if they're missing  when they try to start a guest.

> As a second thought, we generally use commented-out bits that are the
> non-default configuration. So this fits the pattern in the extent that
> any sane distro specified it's own user/group using the configure
> options and if for any reason the user wants to run this as root it's
> done just by uncommenting it.

Most commented out bits are not a security flaw if uncommented though.
The fact that we show 'user=root' in the config file though puts across
the misleading idea that it is a reasonable thing todo, when in fact it
is a horribly insecure thing todo.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Kashyap Chamarthy 5 years, 10 months ago
On Fri, Jun 01, 2018 at 02:11:12PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 01, 2018 at 02:46:40PM +0200, Peter Krempa wrote:
> > On Fri, Jun 01, 2018 at 13:32:20 +0100, Daniel Berrange wrote:

[...]

> > > The reason the config file documents 'root' is because that is what
> > > configure defaults to.  If you pass --with-qemu-user to configure,
> > > we don't update the config file example though, and perhaps we should.

Thanks for that 'configure' context.

> > > Alternatively should we make configure defualt to 'qemu' instead of
> > > 'root', since it is generally considered insane to run QEMU as root.
> > 
> > But user 'qemu' is not by default present on all systems. Even the
> > libvirt.spec file creates the account.
> 
> Yes, that's the reason configure defaults to 'root', but I really hate
> the fact that we default to a config that no one should ever run in
> practice.
> 
> We could check for existance of 'qemu' in configure and complain if
> it is missing, but that's painful in itself as it is valid to build
> on a host without the user, as long as it exists at runtime.
> 
> I tend to think we should just blindly use qemu/qemu by default and
> document that creating these accounts is a requirement. Users will
> quickly see if they're missing  when they try to start a guest.

I'll try to audit what user all the different distributions (that
matter) use to launch QEMU.  If they are all are using 'qemu' anyway,
then probably we can just go with 'qemu:qemu', and document the
requirement as such.

> > As a second thought, we generally use commented-out bits that are the
> > non-default configuration. So this fits the pattern in the extent that
> > any sane distro specified it's own user/group using the configure
> > options and if for any reason the user wants to run this as root it's
> > done just by uncommenting it.
> 
> Most commented out bits are not a security flaw if uncommented though.
> The fact that we show 'user=root' in the config file though puts across
> the misleading idea that it is a reasonable thing todo, when in fact it
> is a horribly insecure thing todo.

Yeah, indeed.

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Ján Tomko 5 years, 10 months ago
On Fri, Jun 01, 2018 at 05:05:30PM +0200, Kashyap Chamarthy wrote:
>On Fri, Jun 01, 2018 at 02:11:12PM +0100, Daniel P. Berrangé wrote:
>> On Fri, Jun 01, 2018 at 02:46:40PM +0200, Peter Krempa wrote:
>> > On Fri, Jun 01, 2018 at 13:32:20 +0100, Daniel Berrange wrote:
>
>[...]
>
>> > > The reason the config file documents 'root' is because that is what
>> > > configure defaults to.  If you pass --with-qemu-user to configure,
>> > > we don't update the config file example though, and perhaps we should.
>
>Thanks for that 'configure' context.
>
>> > > Alternatively should we make configure defualt to 'qemu' instead of
>> > > 'root', since it is generally considered insane to run QEMU as root.
>> >
>> > But user 'qemu' is not by default present on all systems. Even the
>> > libvirt.spec file creates the account.
>>
>> Yes, that's the reason configure defaults to 'root', but I really hate
>> the fact that we default to a config that no one should ever run in
>> practice.
>>

Nobody should be using ./configure with the defaults in practice anyway.
For real world usage, people should go through their distribution's
packages which should have specified --with-qemu-user.

>> We could check for existance of 'qemu' in configure and complain if
>> it is missing, but that's painful in itself as it is valid to build
>> on a host without the user, as long as it exists at runtime.
>>

We probe for many things that are only relevant at runtime (paths to
various binaries come to mind). For many things we try to be smart and
try to guess what the user wants and turn off configure options
when dependencies for drivers and features are missing, instead of
building everything by default and letting the user disable stuff
manually if they don't want the dependencies.

Which brings me to the question: who are the ./configure defaults for?
Distributions usually specify all possible configure options when
building the packages and end users IMO should not be building from
the source. Should we try to build everything and error out when
it's not present? Or try to build as much as we can with the libaries
and dependencies currently present in the system?

>> I tend to think we should just blindly use qemu/qemu by default and
>> document that creating these accounts is a requirement. Users will
>> quickly see if they're missing  when they try to start a guest.
>
>I'll try to audit what user all the different distributions (that
>matter) use to launch QEMU.  If they are all are using 'qemu' anyway,
>then probably we can just go with 'qemu:qemu', and document the
>requirement as such.

Just by defaulting to qemu:qemu if they are present in the system
you can eliminate many setups that would default to root.

But if we start requiring things we consider sensible at configure time,
I'd happily clean up more of our automagic and let the users frugal
on system libraries supply --without- arguments for stuff they don't
want.

>
>> > As a second thought, we generally use commented-out bits that are the
>> > non-default configuration. So this fits the pattern in the extent that
>> > any sane distro specified it's own user/group using the configure
>> > options and if for any reason the user wants to run this as root it's
>> > done just by uncommenting it.
>>
>> Most commented out bits are not a security flaw if uncommented though.
>> The fact that we show 'user=root' in the config file though puts across
>> the misleading idea that it is a reasonable thing todo, when in fact it
>> is a horribly insecure thing todo.
>

In that case,
#allow_disk_format_probing = 1
should also probably go. Maybe {vnc,tls}_listen = "0.0.0.0" as well.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Daniel P. Berrangé 5 years, 10 months ago
On Fri, Jun 01, 2018 at 05:49:24PM +0200, Ján Tomko wrote:
> On Fri, Jun 01, 2018 at 05:05:30PM +0200, Kashyap Chamarthy wrote:
> > On Fri, Jun 01, 2018 at 02:11:12PM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Jun 01, 2018 at 02:46:40PM +0200, Peter Krempa wrote:
> > > > On Fri, Jun 01, 2018 at 13:32:20 +0100, Daniel Berrange wrote:
> > 
> > [...]
> > 
> > > > > The reason the config file documents 'root' is because that is what
> > > > > configure defaults to.  If you pass --with-qemu-user to configure,
> > > > > we don't update the config file example though, and perhaps we should.
> > 
> > Thanks for that 'configure' context.
> > 
> > > > > Alternatively should we make configure defualt to 'qemu' instead of
> > > > > 'root', since it is generally considered insane to run QEMU as root.
> > > >
> > > > But user 'qemu' is not by default present on all systems. Even the
> > > > libvirt.spec file creates the account.
> > > 
> > > Yes, that's the reason configure defaults to 'root', but I really hate
> > > the fact that we default to a config that no one should ever run in
> > > practice.
> > > 
> 
> Nobody should be using ./configure with the defaults in practice anyway.
> For real world usage, people should go through their distribution's
> packages which should have specified --with-qemu-user.
>
> > > We could check for existance of 'qemu' in configure and complain if
> > > it is missing, but that's painful in itself as it is valid to build
> > > on a host without the user, as long as it exists at runtime.
> > > 
> 
> We probe for many things that are only relevant at runtime (paths to
> various binaries come to mind). For many things we try to be smart and
> try to guess what the user wants and turn off configure options
> when dependencies for drivers and features are missing, instead of
> building everything by default and letting the user disable stuff
> manually if they don't want the dependencies.
> 
> Which brings me to the question: who are the ./configure defaults for?
> Distributions usually specify all possible configure options when
> building the packages and end users IMO should not be building from
> the source. Should we try to build everything and error out when
> it's not present? Or try to build as much as we can with the libaries
> and dependencies currently present in the system?

The general view of "configure" defaults is that they should
"do the right thing".

Distros could build with the defaults - we just use the explicit
--with/--without args as a safety net, so features don't accidentally
go missing/appear if the build root contents changes in surprising
ways.


> > > I tend to think we should just blindly use qemu/qemu by default and
> > > document that creating these accounts is a requirement. Users will
> > > quickly see if they're missing  when they try to start a guest.
> > 
> > I'll try to audit what user all the different distributions (that
> > matter) use to launch QEMU.  If they are all are using 'qemu' anyway,
> > then probably we can just go with 'qemu:qemu', and document the
> > requirement as such.
> 
> Just by defaulting to qemu:qemu if they are present in the system
> you can eliminate many setups that would default to root.

Yeah, that would be a decent step forward if we didn't want to
unconditionally use qemu:qemu.

> But if we start requiring things we consider sensible at configure time,
> I'd happily clean up more of our automagic and let the users frugal
> on system libraries supply --without- arguments for stuff they don't
> want.

If libvirt code considers it optional, then configure should always
automatically probe for it, requiring --without- args would be a
step backwards to how things worked in old days, which was a constant
source of pain for people to figure out how to which arg needed
disabling next.

> > > > As a second thought, we generally use commented-out bits that are the
> > > > non-default configuration. So this fits the pattern in the extent that
> > > > any sane distro specified it's own user/group using the configure
> > > > options and if for any reason the user wants to run this as root it's
> > > > done just by uncommenting it.
> > > 
> > > Most commented out bits are not a security flaw if uncommented though.
> > > The fact that we show 'user=root' in the config file though puts across
> > > the misleading idea that it is a reasonable thing todo, when in fact it
> > > is a horribly insecure thing todo.
> > 
> 
> In that case,
> #allow_disk_format_probing = 1

Yes, I think there's actually a good case to be made for that to
go. We had to have it as a get out of jail free card when we disabled
format probing by default, so we had some compat with existing legacy
tools/deployments. After all this time, I think we could reasonably
justify dropping this though.

> should also probably go. Maybe {vnc,tls}_listen = "0.0.0.0" as well.

listening on 0.0.0.0 is only bad if you've not enabled authentication,
otherwise it is an entirely normal/sensible config choice.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu.conf: Change the example user from 'root' to 'qemu'
Posted by Peter Krempa 5 years, 10 months ago
On Fri, Jun 01, 2018 at 17:01:06 +0100, Daniel Berrange wrote:
> On Fri, Jun 01, 2018 at 05:49:24PM +0200, Ján Tomko wrote:
> > On Fri, Jun 01, 2018 at 05:05:30PM +0200, Kashyap Chamarthy wrote:
> > > On Fri, Jun 01, 2018 at 02:11:12PM +0100, Daniel P. Berrangé wrote:

[...]

> > > > Most commented out bits are not a security flaw if uncommented though.
> > > > The fact that we show 'user=root' in the config file though puts across
> > > > the misleading idea that it is a reasonable thing todo, when in fact it
> > > > is a horribly insecure thing todo.
> > > 
> > 
> > In that case,
> > #allow_disk_format_probing = 1
> 
> Yes, I think there's actually a good case to be made for that to
> go. We had to have it as a get out of jail free card when we disabled
> format probing by default, so we had some compat with existing legacy
> tools/deployments. After all this time, I think we could reasonably
> justify dropping this though.

I volunteer for this. I will most probably make my life of adding
blockdev-ized blockjobs way easier.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list