Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/10/2013 05:31 PM, Paul Moore wrote: On Tuesday, December 10, 2013 04:48:54 PM Lucas Meneghel Rodrigues wrote: On 12/10/2013 01:20 AM, Corey Bryant wrote: IMHO the test suite should probe to see if sandbox is working or not, and just not use the "-sandbox on" arg if the host doesn't support it. But I think this could be done on virt-test as well :) This would make sense. Although it sounds like Lucas was looking for an error message when seccomp kills qemu. Maybe virt-test could grep the audit log for the existence of a "type=SECCOMP" record within the test's time of execution, and issue a message based on that. It's a valid idea. The problem I see with it is that not every distro out there uses SELinux. Not getting into the merits of whether they should, ideally it'd be nice to have this working on distros that won't use SELinux. Minor point of clarification, but audit and SELinux and independent subsystems in the kernel. Also, and I don't have a non-audit kernel built at the moment to verify this, but on non-audit kernels the audit messages should be sent to syslog so you *should* still be able to search for SECCOMP records regardless. Ok, my bad, thanks for the clarification! We'll look into checking the audit log.
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/10/2013 04:48 PM, Lucas Meneghel Rodrigues wrote: On 12/10/2013 01:20 AM, Corey Bryant wrote: IMHO the test suite should probe to see if sandbox is working or not, and just not use the "-sandbox on" arg if the host doesn't support it. But I think this could be done on virt-test as well :) This would make sense. Although it sounds like Lucas was looking for an error message when seccomp kills qemu. Maybe virt-test could grep the audit log for the existence of a "type=SECCOMP" record within the test's time of execution, and issue a message based on that. It's a valid idea. The problem I see with it is that not every distro out there uses SELinux. Not getting into the merits of whether they should, ideally it'd be nice to have this working on distros that won't use SELinux. Completely misunderstanding, I feel sorry for that. While we can't rely on the fact that every distro will have audit log working properly, I can start working on some support for virt-test to detect if the host machine has support for seccomp or if the Qemu binary has this feature built in. Again, sorry for the mess. Please disconsider this patch. -- Eduardo Otubo IBM Linux Technology Center
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On Tuesday, December 10, 2013 04:48:54 PM Lucas Meneghel Rodrigues wrote: > On 12/10/2013 01:20 AM, Corey Bryant wrote: > >>> IMHO the test suite should probe to see if sandbox is working or not, > >>> and > >>> just not use the "-sandbox on" arg if the host doesn't support it. > >> > >> But I think this could be done on virt-test as well :) > > > > This would make sense. > > > > Although it sounds like Lucas was looking for an error message when > > seccomp kills qemu. Maybe virt-test could grep the audit log for the > > existence of a "type=SECCOMP" record within the test's time of > > execution, and issue a message based on that. > > It's a valid idea. The problem I see with it is that not every distro > out there uses SELinux. Not getting into the merits of whether they > should, ideally it'd be nice to have this working on distros that won't > use SELinux. Minor point of clarification, but audit and SELinux and independent subsystems in the kernel. Also, and I don't have a non-audit kernel built at the moment to verify this, but on non-audit kernels the audit messages should be sent to syslog so you *should* still be able to search for SECCOMP records regardless. -- paul moore security and virtualization @ redhat
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/10/2013 01:20 AM, Corey Bryant wrote: IMHO the test suite should probe to see if sandbox is working or not, and just not use the "-sandbox on" arg if the host doesn't support it. But I think this could be done on virt-test as well :) This would make sense. Although it sounds like Lucas was looking for an error message when seccomp kills qemu. Maybe virt-test could grep the audit log for the existence of a "type=SECCOMP" record within the test's time of execution, and issue a message based on that. It's a valid idea. The problem I see with it is that not every distro out there uses SELinux. Not getting into the merits of whether they should, ideally it'd be nice to have this working on distros that won't use SELinux.
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/09/2013 12:51 PM, Eduardo Otubo wrote:
On 12/09/2013 03:33 PM, Daniel P. Berrange wrote:
On Mon, Dec 09, 2013 at 03:20:52PM -0200, Eduardo Otubo wrote:
This option was requested by virt-test team so they can run tests with
Qemu and "-sandbox on" set without breaking whole test if host doesn't
have support for seccomp in kernel. It covers two possibilities:
1) Host kernel support does not support seccomp, but user installed
Qemu
package with sandbox support: Libseccomp will fail -> qemu will
fail
nicely and won't stop execution.
2) Host kernel has support but Qemu package wasn't built with sandbox
feature. Qemu will fail nicely and won't stop execution.
Signed-off-by: Eduardo Otubo
---
vl.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index b0399de..a0806dc 100644
--- a/vl.c
+++ b/vl.c
@@ -967,13 +967,11 @@ static int parse_sandbox(QemuOpts *opts, void
*opaque)
#ifdef CONFIG_SECCOMP
if (seccomp_start() < 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "failed to install seccomp syscall filter
in the kernel");
-return -1;
+ "failed to install seccomp syscall filter
in the kernel, disabling it");
}
#else
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "sandboxing request but seccomp is not
compiled into this build");
-return -1;
+ "sandboxing request but seccomp is not
compiled into this build, disabling it");
#endif
}
@@ -3808,9 +3806,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
-if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox,
NULL, 0)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox,
NULL, 0);
#ifndef _WIN32
if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd,
NULL, 1)) {
This change is really dubious from a security POV. If the admin requested
sandboxing and the host or QEMU build cannot support it, then QEMU really
*must* exit.
I think an admin must know what he's doing. If he requested sandbox but
without kernel support he need to step back a little and understand what
he's doing. This patch won't decrease the security level, IMHO.
Preventing qemu from exiting is definitely not the right approach.
Please feel free to run code by me ahead of time in the future.
IMHO the test suite should probe to see if sandbox is working or not, and
just not use the "-sandbox on" arg if the host doesn't support it.
But I think this could be done on virt-test as well :)
This would make sense.
Although it sounds like Lucas was looking for an error message when
seccomp kills qemu. Maybe virt-test could grep the audit log for the
existence of a "type=SECCOMP" record within the test's time of
execution, and issue a message based on that.
--
Regards,
Corey Bryant
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/09/2013 03:20 PM, Eduardo Otubo wrote: This option was requested by virt-test team so they can run tests with Qemu and "-sandbox on" set without breaking whole test if host doesn't have support for seccomp in kernel. It covers two possibilities: 1) Host kernel support does not support seccomp, but user installed Qemu package with sandbox support: Libseccomp will fail -> qemu will fail nicely and won't stop execution. 2) Host kernel has support but Qemu package wasn't built with sandbox feature. Qemu will fail nicely and won't stop execution. It seems there was a misunderstanding of what we wanted here. The problem we had there happened on a -sandbox bug on Fedora 19 that got one of our team members confused, since qemu did not give any sort of useful output that would allow him to identify the bug was related to -sandbox (qemu was accessing a syscall outside of the whitelist on Fedora 19). He took a while until he figured out -sandbox was the problem, due to the lack of any clues of what was going on. I was not affected due to the fact I was already on Fedora 20 by that time. I assume Eduardo thought that we somehow wanted qemu to just carry on when seccomp requirements could not be fulfilled, but that was not our point. What I thought I'd commented with hum was that some more useful message could be printed to stderr, and perhaps make the qemu process to exit with rc != 0 on such errors, instead of just going dead silently. I still couldn't quite grasp why that could not be done, but if it can't, so be it. No big deal. Lucas
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On Monday, December 09, 2013 03:51:36 PM Eduardo Otubo wrote:
> On 12/09/2013 03:33 PM, Daniel P. Berrange wrote:
> > On Mon, Dec 09, 2013 at 03:20:52PM -0200, Eduardo Otubo wrote:
> >> This option was requested by virt-test team so they can run tests with
> >> Qemu and "-sandbox on" set without breaking whole test if host doesn't
> >>
> >> have support for seccomp in kernel. It covers two possibilities:
> >> 1) Host kernel support does not support seccomp, but user installed
> >> Qemu
> >>
> >> package with sandbox support: Libseccomp will fail -> qemu will fail
> >> nicely and won't stop execution.
> >>
> >> 2) Host kernel has support but Qemu package wasn't built with sandbox
> >>
> >> feature. Qemu will fail nicely and won't stop execution.
> >>
> >> Signed-off-by: Eduardo Otubo
> >> ---
> >>
> >> vl.c | 10 +++---
> >> 1 file changed, 3 insertions(+), 7 deletions(-)
{snip}
> > This change is really dubious from a security POV. If the admin requested
> > sandboxing and the host or QEMU build cannot support it, then QEMU really
> > *must* exit.
>
> I think an admin must know what he's doing. If he requested sandbox but
> without kernel support he need to step back a little and understand what
> he's doing. This patch won't decrease the security level, IMHO.
NACK
For the reasons Daniel already mentioned. Mistakes happen, a lot, and if the
user explicitly requests security functionality and we can't provide it we
need to fail in a manner that doesn't increase the user's risk.
> > IMHO the test suite should probe to see if sandbox is working or not, and
> > just not use the "-sandbox on" arg if the host doesn't support it.
>
> But I think this could be done on virt-test as well :)
This would be ideal, but if you must have a fallback mechanism in QEMU proper,
make it separate from '-sandbox on' so that it doesn't break with the current
behavior and also makes it is obvious that the functionality is not
guaranteed, e.g. '-sandbox try' or similar.
--
paul moore
security and virtualization @ redhat
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On 12/09/2013 03:33 PM, Daniel P. Berrange wrote:
On Mon, Dec 09, 2013 at 03:20:52PM -0200, Eduardo Otubo wrote:
This option was requested by virt-test team so they can run tests with
Qemu and "-sandbox on" set without breaking whole test if host doesn't
have support for seccomp in kernel. It covers two possibilities:
1) Host kernel support does not support seccomp, but user installed Qemu
package with sandbox support: Libseccomp will fail -> qemu will fail
nicely and won't stop execution.
2) Host kernel has support but Qemu package wasn't built with sandbox
feature. Qemu will fail nicely and won't stop execution.
Signed-off-by: Eduardo Otubo
---
vl.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index b0399de..a0806dc 100644
--- a/vl.c
+++ b/vl.c
@@ -967,13 +967,11 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
#ifdef CONFIG_SECCOMP
if (seccomp_start() < 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "failed to install seccomp syscall filter in the
kernel");
-return -1;
+ "failed to install seccomp syscall filter in the kernel,
disabling it");
}
#else
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "sandboxing request but seccomp is not compiled into this
build");
-return -1;
+ "sandboxing request but seccomp is not compiled into this
build, disabling it");
#endif
}
@@ -3808,9 +3806,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
-if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0);
#ifndef _WIN32
if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
This change is really dubious from a security POV. If the admin requested
sandboxing and the host or QEMU build cannot support it, then QEMU really
*must* exit.
I think an admin must know what he's doing. If he requested sandbox but
without kernel support he need to step back a little and understand what
he's doing. This patch won't decrease the security level, IMHO.
IMHO the test suite should probe to see if sandbox is working or not, and
just not use the "-sandbox on" arg if the host doesn't support it.
But I think this could be done on virt-test as well :)
--
Eduardo Otubo
IBM Linux Technology Center
Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
On Mon, Dec 09, 2013 at 03:20:52PM -0200, Eduardo Otubo wrote:
> This option was requested by virt-test team so they can run tests with
> Qemu and "-sandbox on" set without breaking whole test if host doesn't
> have support for seccomp in kernel. It covers two possibilities:
>
> 1) Host kernel support does not support seccomp, but user installed Qemu
> package with sandbox support: Libseccomp will fail -> qemu will fail
> nicely and won't stop execution.
>
> 2) Host kernel has support but Qemu package wasn't built with sandbox
> feature. Qemu will fail nicely and won't stop execution.
>
> Signed-off-by: Eduardo Otubo
> ---
> vl.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index b0399de..a0806dc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -967,13 +967,11 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
> #ifdef CONFIG_SECCOMP
> if (seccomp_start() < 0) {
> qerror_report(ERROR_CLASS_GENERIC_ERROR,
> - "failed to install seccomp syscall filter in the
> kernel");
> -return -1;
> + "failed to install seccomp syscall filter in the
> kernel, disabling it");
> }
> #else
> qerror_report(ERROR_CLASS_GENERIC_ERROR,
> - "sandboxing request but seccomp is not compiled into
> this build");
> -return -1;
> + "sandboxing request but seccomp is not compiled into
> this build, disabling it");
> #endif
> }
>
> @@ -3808,9 +3806,7 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
>
> -if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL,
> 0)) {
> -exit(1);
> -}
> +qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0);
>
> #ifndef _WIN32
> if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
This change is really dubious from a security POV. If the admin requested
sandboxing and the host or QEMU build cannot support it, then QEMU really
*must* exit.
IMHO the test suite should probe to see if sandbox is working or not, and
just not use the "-sandbox on" arg if the host doesn't support it.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in
This option was requested by virt-test team so they can run tests with
Qemu and "-sandbox on" set without breaking whole test if host doesn't
have support for seccomp in kernel. It covers two possibilities:
1) Host kernel support does not support seccomp, but user installed Qemu
package with sandbox support: Libseccomp will fail -> qemu will fail
nicely and won't stop execution.
2) Host kernel has support but Qemu package wasn't built with sandbox
feature. Qemu will fail nicely and won't stop execution.
Signed-off-by: Eduardo Otubo
---
vl.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index b0399de..a0806dc 100644
--- a/vl.c
+++ b/vl.c
@@ -967,13 +967,11 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
#ifdef CONFIG_SECCOMP
if (seccomp_start() < 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "failed to install seccomp syscall filter in the
kernel");
-return -1;
+ "failed to install seccomp syscall filter in the
kernel, disabling it");
}
#else
qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "sandboxing request but seccomp is not compiled into
this build");
-return -1;
+ "sandboxing request but seccomp is not compiled into
this build, disabling it");
#endif
}
@@ -3808,9 +3806,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
-if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0);
#ifndef _WIN32
if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
--
1.8.3.1
