Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/09/2013 05:36 PM, Paul Moore wrote: On Tuesday, October 08, 2013 09:42:24 PM Eduardo Otubo wrote: v3: The -netdev tap option is checked in the vl.c file during the process of the command line argument list. It sets tap_enabled to true or false according to the configuration found. Later at the seccomp filter installation, this value is checked wheter to install or not this feature. I like the idea of slowly making the QEMU syscall filter dependent on the runtime configuration. With that in mind, I wonder if we should have a more general purpose API in include/sysemu/seccomp.h that allows QEMU to indicate to the the QEMU/seccomp code that a particular feature is enabled. Maybe something like this: #define SCMP_FEAT_TAP ... int seccomp_feature_enable(int feature); This is a good approach, and then the blacklist can vary based on what features are enabled. -- Regards, Corey Bryant One more comment below. Adding a system call blacklist right before the vcpus starts. This filter is composed by the system calls that can't be executed after the guests are up. This list should be refined as whitelist is, with as much testing as we can do using virt-test. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- include/sysemu/seccomp.h | 6 - qemu-seccomp.c | 64 +++- vl.c | 21 +++- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h index 1189fa2..9dc7e52 100644 --- a/include/sysemu/seccomp.h +++ b/include/sysemu/seccomp.h @@ -15,8 +15,12 @@ #ifndef QEMU_SECCOMP_H #define QEMU_SECCOMP_H +#define WHITELIST 0 +#define BLACKLIST 1 Should these #defines be namespaced in some way, e.g. SCMP_LIST_BLACKLIST? #include seccomp.h #include qemu/osdep.h -int seccomp_start(void); +int seccomp_start(int list_type); + #endif
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/08/2013 11:05 PM, Eric Blake wrote: On 10/08/2013 06:42 PM, Eduardo Otubo wrote: v3: The -netdev tap option is checked in the vl.c file during the process of the command line argument list. It sets tap_enabled to true or false according to the configuration found. Later at the seccomp filter installation, this value is checked wheter to install or not this s/wheter/whether/ Thank you. feature. Adding a system call blacklist right before the vcpus starts. This filter is composed by the system calls that can't be executed after the guests are up. This list should be refined as whitelist is, with as much testing as we can do using virt-test. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- include/sysemu/seccomp.h | 6 - qemu-seccomp.c | 64 +++- vl.c | 21 +++- 3 files changed, 77 insertions(+), 14 deletions(-) No review on the actual patch, just spotting a typo. -- Eduardo Otubo IBM Linux Technology Center
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/08/2013 08:42 PM, Eduardo Otubo wrote:
v3: The -netdev tap option is checked in the vl.c file during the
process of the command line argument list. It sets tap_enabled to true
or false according to the configuration found. Later at the seccomp
filter installation, this value is checked wheter to install or not this
feature.
Adding a system call blacklist right before the vcpus starts. This
filter is composed by the system calls that can't be executed after the
guests are up. This list should be refined as whitelist is, with as much
testing as we can do using virt-test.
Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
include/sysemu/seccomp.h | 6 -
qemu-seccomp.c | 64 +++-
vl.c | 21 +++-
3 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 1189fa2..9dc7e52 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -15,8 +15,12 @@
#ifndef QEMU_SECCOMP_H
#define QEMU_SECCOMP_H
+#define WHITELIST 0
+#define BLACKLIST 1
+
#include seccomp.h
#include qemu/osdep.h
-int seccomp_start(void);
+int seccomp_start(int list_type);
+
#endif
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 37d38f8..84a42bc 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -21,7 +21,7 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall whitelist[] = {
{ SCMP_SYS(timer_settime), 255 },
{ SCMP_SYS(timer_gettime), 254 },
{ SCMP_SYS(futex), 253 },
@@ -221,32 +221,72 @@ static const struct QemuSeccompSyscall
seccomp_whitelist[] = {
{ SCMP_SYS(arch_prctl), 240 }
};
-int seccomp_start(void)
+/*
+ * The second list, called blacklist, basically reduces previously installed
+ * whitelist. All the syscalls configured by the previous whitelist are still
+ * allowed, except for the ones in the blacklist.
+ * */
+
+static const struct QemuSeccompSyscall blacklist[] = {
+{ SCMP_SYS(execve), 255 }
+};
+
+static int process_list(scmp_filter_ctx *ctx,
+const struct QemuSeccompSyscall *list,
+unsigned int list_size, uint32_t action)
{
int rc = 0;
unsigned int i = 0;
-scmp_filter_ctx ctx;
-ctx = seccomp_init(SCMP_ACT_KILL);
-if (ctx == NULL) {
-goto seccomp_return;
-}
+for (i = 0; i list_size; i++) {
+rc = seccomp_rule_add(ctx, action, list[i].num, 0);
+if (rc 0) {
+goto seccomp_return;
+}
-for (i = 0; i ARRAY_SIZE(seccomp_whitelist); i++) {
-rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num,
0);
+rc = seccomp_syscall_priority(ctx, list[i].num,
+ list[i].priority);
if (rc 0) {
goto seccomp_return;
}
-rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
- seccomp_whitelist[i].priority);
+}
+
+seccomp_return:
+return rc;
+}
+
+int seccomp_start(int list_type)
+{
+int rc = 0;
+scmp_filter_ctx ctx;
+
+switch (list_type) {
+case WHITELIST:
+ctx = seccomp_init(SCMP_ACT_KILL);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, whitelist, ARRAY_SIZE(whitelist),
SCMP_ACT_ALLOW);
if (rc 0) {
goto seccomp_return;
}
+break;
+case BLACKLIST:
+ctx = seccomp_init(SCMP_ACT_ALLOW);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, blacklist, ARRAY_SIZE(blacklist),
SCMP_ACT_KILL);
+break;
+default:
+rc = -1;
+goto seccomp_return;
}
rc = seccomp_load(ctx);
seccomp_return:
-seccomp_release(ctx);
+if (ctx)
+seccomp_release(ctx);
return rc;
}
diff --git a/vl.c b/vl.c
index b4b119a..ee95674 100644
--- a/vl.c
+++ b/vl.c
@@ -179,6 +179,8 @@ int main(int argc, char **argv)
#define MAX_VIRTIO_CONSOLES 1
#define MAX_SCLP_CONSOLES 1
+static bool enable_blacklist = false;
+static bool tap_enabled = false;
static const char *data_dir[16];
static int data_dir_idx;
const char *bios_name = NULL;
@@ -1033,7 +1035,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
/* FIXME: change this to true for 1.3 */
if (qemu_opt_get_bool(opts, enable, false)) {
#ifdef CONFIG_SECCOMP
-if (seccomp_start() 0) {
+if (seccomp_start(WHITELIST) 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
failed to install seccomp syscall filter in the
kernel);
return -1;
@@ -1765,12 +1767,24 @@ void vm_state_notify(int running, RunState state)
}
}
+static void install_seccomp_blacklist(void)
+{
+if
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On Tuesday, October 08, 2013 09:42:24 PM Eduardo Otubo wrote: v3: The -netdev tap option is checked in the vl.c file during the process of the command line argument list. It sets tap_enabled to true or false according to the configuration found. Later at the seccomp filter installation, this value is checked wheter to install or not this feature. I like the idea of slowly making the QEMU syscall filter dependent on the runtime configuration. With that in mind, I wonder if we should have a more general purpose API in include/sysemu/seccomp.h that allows QEMU to indicate to the the QEMU/seccomp code that a particular feature is enabled. Maybe something like this: #define SCMP_FEAT_TAP ... int seccomp_feature_enable(int feature); One more comment below. Adding a system call blacklist right before the vcpus starts. This filter is composed by the system calls that can't be executed after the guests are up. This list should be refined as whitelist is, with as much testing as we can do using virt-test. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- include/sysemu/seccomp.h | 6 - qemu-seccomp.c | 64 +++- vl.c | 21 +++- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h index 1189fa2..9dc7e52 100644 --- a/include/sysemu/seccomp.h +++ b/include/sysemu/seccomp.h @@ -15,8 +15,12 @@ #ifndef QEMU_SECCOMP_H #define QEMU_SECCOMP_H +#define WHITELIST 0 +#define BLACKLIST 1 Should these #defines be namespaced in some way, e.g. SCMP_LIST_BLACKLIST? #include seccomp.h #include qemu/osdep.h -int seccomp_start(void); +int seccomp_start(int list_type); + #endif -- paul moore security and virtualization @ redhat
[Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
v3: The -netdev tap option is checked in the vl.c file during the
process of the command line argument list. It sets tap_enabled to true
or false according to the configuration found. Later at the seccomp
filter installation, this value is checked wheter to install or not this
feature.
Adding a system call blacklist right before the vcpus starts. This
filter is composed by the system calls that can't be executed after the
guests are up. This list should be refined as whitelist is, with as much
testing as we can do using virt-test.
Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
include/sysemu/seccomp.h | 6 -
qemu-seccomp.c | 64 +++-
vl.c | 21 +++-
3 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 1189fa2..9dc7e52 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -15,8 +15,12 @@
#ifndef QEMU_SECCOMP_H
#define QEMU_SECCOMP_H
+#define WHITELIST 0
+#define BLACKLIST 1
+
#include seccomp.h
#include qemu/osdep.h
-int seccomp_start(void);
+int seccomp_start(int list_type);
+
#endif
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 37d38f8..84a42bc 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -21,7 +21,7 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall whitelist[] = {
{ SCMP_SYS(timer_settime), 255 },
{ SCMP_SYS(timer_gettime), 254 },
{ SCMP_SYS(futex), 253 },
@@ -221,32 +221,72 @@ static const struct QemuSeccompSyscall
seccomp_whitelist[] = {
{ SCMP_SYS(arch_prctl), 240 }
};
-int seccomp_start(void)
+/*
+ * The second list, called blacklist, basically reduces previously installed
+ * whitelist. All the syscalls configured by the previous whitelist are still
+ * allowed, except for the ones in the blacklist.
+ * */
+
+static const struct QemuSeccompSyscall blacklist[] = {
+{ SCMP_SYS(execve), 255 }
+};
+
+static int process_list(scmp_filter_ctx *ctx,
+const struct QemuSeccompSyscall *list,
+unsigned int list_size, uint32_t action)
{
int rc = 0;
unsigned int i = 0;
-scmp_filter_ctx ctx;
-ctx = seccomp_init(SCMP_ACT_KILL);
-if (ctx == NULL) {
-goto seccomp_return;
-}
+for (i = 0; i list_size; i++) {
+rc = seccomp_rule_add(ctx, action, list[i].num, 0);
+if (rc 0) {
+goto seccomp_return;
+}
-for (i = 0; i ARRAY_SIZE(seccomp_whitelist); i++) {
-rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num,
0);
+rc = seccomp_syscall_priority(ctx, list[i].num,
+ list[i].priority);
if (rc 0) {
goto seccomp_return;
}
-rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
- seccomp_whitelist[i].priority);
+}
+
+seccomp_return:
+return rc;
+}
+
+int seccomp_start(int list_type)
+{
+int rc = 0;
+scmp_filter_ctx ctx;
+
+switch (list_type) {
+case WHITELIST:
+ctx = seccomp_init(SCMP_ACT_KILL);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, whitelist, ARRAY_SIZE(whitelist),
SCMP_ACT_ALLOW);
if (rc 0) {
goto seccomp_return;
}
+break;
+case BLACKLIST:
+ctx = seccomp_init(SCMP_ACT_ALLOW);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, blacklist, ARRAY_SIZE(blacklist),
SCMP_ACT_KILL);
+break;
+default:
+rc = -1;
+goto seccomp_return;
}
rc = seccomp_load(ctx);
seccomp_return:
-seccomp_release(ctx);
+if (ctx)
+seccomp_release(ctx);
return rc;
}
diff --git a/vl.c b/vl.c
index b4b119a..ee95674 100644
--- a/vl.c
+++ b/vl.c
@@ -179,6 +179,8 @@ int main(int argc, char **argv)
#define MAX_VIRTIO_CONSOLES 1
#define MAX_SCLP_CONSOLES 1
+static bool enable_blacklist = false;
+static bool tap_enabled = false;
static const char *data_dir[16];
static int data_dir_idx;
const char *bios_name = NULL;
@@ -1033,7 +1035,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
/* FIXME: change this to true for 1.3 */
if (qemu_opt_get_bool(opts, enable, false)) {
#ifdef CONFIG_SECCOMP
-if (seccomp_start() 0) {
+if (seccomp_start(WHITELIST) 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
failed to install seccomp syscall filter in the
kernel);
return -1;
@@ -1765,12 +1767,24 @@ void vm_state_notify(int running, RunState state)
}
}
+static void install_seccomp_blacklist(void)
+{
+if (enable_blacklist !tap_enabled) {
+if (seccomp_start(BLACKLIST) 0) {
+
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/08/2013 06:42 PM, Eduardo Otubo wrote: v3: The -netdev tap option is checked in the vl.c file during the process of the command line argument list. It sets tap_enabled to true or false according to the configuration found. Later at the seccomp filter installation, this value is checked wheter to install or not this s/wheter/whether/ feature. Adding a system call blacklist right before the vcpus starts. This filter is composed by the system calls that can't be executed after the guests are up. This list should be refined as whitelist is, with as much testing as we can do using virt-test. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- include/sysemu/seccomp.h | 6 - qemu-seccomp.c | 64 +++- vl.c | 21 +++- 3 files changed, 77 insertions(+), 14 deletions(-) No review on the actual patch, just spotting a typo. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
