| Message ID | 1458004129-29004-1-git-send-email-jfehlig@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Opps, forgot to cc the tools maintainers, sorry. I can resend if needed. Regards, Jim On 03/14/2016 07:08 PM, Jim Fehlig wrote: > Commit 6ef823fd added '-nodefaults' to the qemu args created by > libxl, which is a good step in restricting qemu's default > configuration. This change takes another step by adding > -no-user-config, which ignores any user-provided config files in > sysconfdir. Together, -nodefaults and -no-user-config allow Xen > to avoid unkown and uncontrolled qemu configuration. > > Both options are also added to the qemu invocation in the > xen-qemu-dom0-disk-backend systemd service file. > > Signed-off-by: Jim Fehlig <jfehlig@suse.com> > --- > tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 1 + > tools/libxl/libxl_dm.c | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > index acf61a8..f56775b 100644 > --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > @@ -14,6 +14,7 @@ ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@ > ExecStart=@qemu_xen_systemd@ -xen-domid 0 \ > -xen-attach -name dom0 -nographic -M xenpv -daemonize \ > -monitor /dev/null -serial /dev/null -parallel /dev/null \ > + -nodefaults -no-user-config \ > -pidfile @XEN_RUN_DIR@/qemu-dom0.pid > > [Install] > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 4aca38e..cfda24c 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -828,6 +828,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, > */ > flexarray_append(dm_args, "-nodefaults"); > > + /* > + * Do not use any of the user-provided config files in sysconfdir, > + * avoiding unkown and uncontrolled configuration. > + */ > + flexarray_append(dm_args, "-no-user-config"); > + > if (b_info->type == LIBXL_DOMAIN_TYPE_PV) { > flexarray_append(dm_args, "-xen-attach"); > }
On Mon, Mar 14, 2016 at 07:14:12PM -0600, Jim Fehlig wrote: > Opps, forgot to cc the tools maintainers, sorry. I can resend if needed. > > Regards, > Jim > > On 03/14/2016 07:08 PM, Jim Fehlig wrote: > > Commit 6ef823fd added '-nodefaults' to the qemu args created by > > libxl, which is a good step in restricting qemu's default > > configuration. This change takes another step by adding > > -no-user-config, which ignores any user-provided config files in > > sysconfdir. Together, -nodefaults and -no-user-config allow Xen > > to avoid unkown and uncontrolled qemu configuration. > > > > Both options are also added to the qemu invocation in the > > xen-qemu-dom0-disk-backend systemd service file. > > > > Signed-off-by: Jim Fehlig <jfehlig@suse.com> Acked-by: Wei Liu <wei.liu2@citrix.com> > > --- > > tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 1 + > > tools/libxl/libxl_dm.c | 6 ++++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > > index acf61a8..f56775b 100644 > > --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > > +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in > > @@ -14,6 +14,7 @@ ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@ > > ExecStart=@qemu_xen_systemd@ -xen-domid 0 \ > > -xen-attach -name dom0 -nographic -M xenpv -daemonize \ > > -monitor /dev/null -serial /dev/null -parallel /dev/null \ > > + -nodefaults -no-user-config \ > > -pidfile @XEN_RUN_DIR@/qemu-dom0.pid > > > > [Install] > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > index 4aca38e..cfda24c 100644 > > --- a/tools/libxl/libxl_dm.c > > +++ b/tools/libxl/libxl_dm.c > > @@ -828,6 +828,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, > > */ > > flexarray_append(dm_args, "-nodefaults"); > > > > + /* > > + * Do not use any of the user-provided config files in sysconfdir, > > + * avoiding unkown and uncontrolled configuration. > > + */ > > + flexarray_append(dm_args, "-no-user-config"); > > + > > if (b_info->type == LIBXL_DOMAIN_TYPE_PV) { > > flexarray_append(dm_args, "-xen-attach"); > > } >
On 03/15/2016 06:28 AM, Wei Liu wrote: > On Mon, Mar 14, 2016 at 07:14:12PM -0600, Jim Fehlig wrote: >> Opps, forgot to cc the tools maintainers, sorry. I can resend if needed. >> >> Regards, >> Jim >> >> On 03/14/2016 07:08 PM, Jim Fehlig wrote: >>> Commit 6ef823fd added '-nodefaults' to the qemu args created by >>> libxl, which is a good step in restricting qemu's default >>> configuration. This change takes another step by adding >>> -no-user-config, which ignores any user-provided config files in >>> sysconfdir. Together, -nodefaults and -no-user-config allow Xen >>> to avoid unkown and uncontrolled qemu configuration. >>> >>> Both options are also added to the qemu invocation in the >>> xen-qemu-dom0-disk-backend systemd service file. >>> >>> Signed-off-by: Jim Fehlig <jfehlig@suse.com> > Acked-by: Wei Liu <wei.liu2@citrix.com> If there are no remaining issues, can this patch be applied? Thanks! Regards, Jim > >>> --- >>> tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 1 + >>> tools/libxl/libxl_dm.c | 6 ++++++ >>> 2 files changed, 7 insertions(+) >>> >>> diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in >>> index acf61a8..f56775b 100644 >>> --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in >>> +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in >>> @@ -14,6 +14,7 @@ ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@ >>> ExecStart=@qemu_xen_systemd@ -xen-domid 0 \ >>> -xen-attach -name dom0 -nographic -M xenpv -daemonize \ >>> -monitor /dev/null -serial /dev/null -parallel /dev/null \ >>> + -nodefaults -no-user-config \ >>> -pidfile @XEN_RUN_DIR@/qemu-dom0.pid >>> >>> [Install] >>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >>> index 4aca38e..cfda24c 100644 >>> --- a/tools/libxl/libxl_dm.c >>> +++ b/tools/libxl/libxl_dm.c >>> @@ -828,6 +828,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, >>> */ >>> flexarray_append(dm_args, "-nodefaults"); >>> >>> + /* >>> + * Do not use any of the user-provided config files in sysconfdir, >>> + * avoiding unkown and uncontrolled configuration. >>> + */ >>> + flexarray_append(dm_args, "-no-user-config"); >>> + >>> if (b_info->type == LIBXL_DOMAIN_TYPE_PV) { >>> flexarray_append(dm_args, "-xen-attach"); >>> } > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jim Fehlig writes ("[Xen-devel] [PATCH] tools: Restrict configuration of qemu processes"):
> Commit 6ef823fd added '-nodefaults' to the qemu args created by
> libxl, which is a good step in restricting qemu's default
> configuration. This change takes another step by adding
> -no-user-config, which ignores any user-provided config files in
> sysconfdir. Together, -nodefaults and -no-user-config allow Xen
> to avoid unkown and uncontrolled qemu configuration.
>
> Both options are also added to the qemu invocation in the
> xen-qemu-dom0-disk-backend systemd service file.
Queued, thanks. Also listed for backport.
Ian.
Ian Jackson writes ("Re: [Xen-devel] [PATCH] tools: Restrict configuration of qemu processes"):
> Jim Fehlig writes ("[Xen-devel] [PATCH] tools: Restrict configuration of qemu processes"):
> > Commit 6ef823fd added '-nodefaults' to the qemu args created by
> > libxl, which is a good step in restricting qemu's default
> > configuration. This change takes another step by adding
> > -no-user-config, which ignores any user-provided config files in
> > sysconfdir. Together, -nodefaults and -no-user-config allow Xen
> > to avoid unkown and uncontrolled qemu configuration.
> >
> > Both options are also added to the qemu invocation in the
> > xen-qemu-dom0-disk-backend systemd service file.
>
> Queued, thanks. Also listed for backport.
I found this on my backport todo list. Thinking about it, I have had
second thoughts.
I worry that existing users of stable branches might be relying on the
user config feature (for example by dropping qemu configuration in
~root). If they are, then applying this would break things for them.
It's not a security problem because in xen the configuration in
question would have to come from ~root.
So I think, probably, that we should leave this be (ie, not backport
the patch). Does anyone want to try to change my mind ?
Ian.
On 05/09/2016 10:35 AM, Ian Jackson wrote: > Ian Jackson writes ("Re: [Xen-devel] [PATCH] tools: Restrict configuration of qemu processes"): >> Jim Fehlig writes ("[Xen-devel] [PATCH] tools: Restrict configuration of qemu processes"): >>> Commit 6ef823fd added '-nodefaults' to the qemu args created by >>> libxl, which is a good step in restricting qemu's default >>> configuration. This change takes another step by adding >>> -no-user-config, which ignores any user-provided config files in >>> sysconfdir. Together, -nodefaults and -no-user-config allow Xen >>> to avoid unkown and uncontrolled qemu configuration. >>> >>> Both options are also added to the qemu invocation in the >>> xen-qemu-dom0-disk-backend systemd service file. >> Queued, thanks. Also listed for backport. > I found this on my backport todo list. Thinking about it, I have had > second thoughts. > > I worry that existing users of stable branches might be relying on the > user config feature (for example by dropping qemu configuration in > ~root). If they are, then applying this would break things for them. > > It's not a security problem because in xen the configuration in > question would have to come from ~root. Good point. > So I think, probably, that we should leave this be (ie, not backport > the patch). Does anyone want to try to change my mind ? I never asked for a backport, so have no incentive to change your mind. Plus, I agree with your comment. Regards, Jim
diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in index acf61a8..f56775b 100644 --- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in +++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in @@ -14,6 +14,7 @@ ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@ ExecStart=@qemu_xen_systemd@ -xen-domid 0 \ -xen-attach -name dom0 -nographic -M xenpv -daemonize \ -monitor /dev/null -serial /dev/null -parallel /dev/null \ + -nodefaults -no-user-config \ -pidfile @XEN_RUN_DIR@/qemu-dom0.pid [Install] diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 4aca38e..cfda24c 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -828,6 +828,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, */ flexarray_append(dm_args, "-nodefaults"); + /* + * Do not use any of the user-provided config files in sysconfdir, + * avoiding unkown and uncontrolled configuration. + */ + flexarray_append(dm_args, "-no-user-config"); + if (b_info->type == LIBXL_DOMAIN_TYPE_PV) { flexarray_append(dm_args, "-xen-attach"); }
Commit 6ef823fd added '-nodefaults' to the qemu args created by libxl, which is a good step in restricting qemu's default configuration. This change takes another step by adding -no-user-config, which ignores any user-provided config files in sysconfdir. Together, -nodefaults and -no-user-config allow Xen to avoid unkown and uncontrolled qemu configuration. Both options are also added to the qemu invocation in the xen-qemu-dom0-disk-backend systemd service file. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 1 + tools/libxl/libxl_dm.c | 6 ++++++ 2 files changed, 7 insertions(+)