Skip to content

L
lxc
Unverified Commit 3e9671a1 by Christian Brauner
Options

Revert "seccomp: add rules for specified architecture only"

This reverts commit f1bcfc79. The reverted branch breaks starting all seccomp confined containers. Not even a containers with our standard seccomp profile starts correctly. This is strong evidence that these changes have never been tested even with a standard workload. That is unacceptable! We are still happy to merge that feature but going forward we want tests that verify that standard workloads and new features work correctly. seccomp is a crucial part of our security story and I will not let the be compromised by missing tests! Signed-off-by: 's avatarChristian Brauner <christian.brauner@ubuntu.com>
parent b6825c4b
Showing with 50 additions and 83 deletions
src/lxc/seccomp.c
...@@ -291,7 +291,7 @@ on_error: ...@@ -291,7 +291,7 @@ on_error:
#endif #endif
#if HAVE_DECL_SECCOMP_SYSCALL_RESOLVE_NAME_ARCH #if HAVE_DECL_SECCOMP_SYSCALL_RESOLVE_NAME_ARCH
enum lxc_arch_t { enum lxc_hostarch_t {
lxc_seccomp_arch_all = 0, lxc_seccomp_arch_all = 0,
lxc_seccomp_arch_native, lxc_seccomp_arch_native,
lxc_seccomp_arch_i386, lxc_seccomp_arch_i386,
...@@ -345,8 +345,8 @@ int get_hostarch(void) ...@@ -345,8 +345,8 @@ int get_hostarch(void)
return lxc_seccomp_arch_unknown; return lxc_seccomp_arch_unknown;
} }
scmp_filter_ctx get_new_ctx(enum lxc_arch_t n_arch, scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t n_arch,
uint32_t default_policy_action) uint32_t default_policy_action, bool *needs_merge)
{ {
int ret; int ret;
uint32_t arch; uint32_t arch;
...@@ -464,7 +464,10 @@ scmp_filter_ctx get_new_ctx(enum lxc_arch_t n_arch, ...@@ -464,7 +464,10 @@ scmp_filter_ctx get_new_ctx(enum lxc_arch_t n_arch,
return NULL; return NULL;
} }
TRACE("Removed native arch from main seccomp context"); TRACE("Removed native arch from main seccomp context");
*needs_merge = true;
} else { } else {
*needs_merge = false;
TRACE("Arch %d already present in main seccomp context", (int)n_arch); TRACE("Arch %d already present in main seccomp context", (int)n_arch);
} }
...@@ -547,27 +550,6 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx, ...@@ -547,27 +550,6 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
return true; return true;
} }
#define SCMP_ARCH_INDEX_MAX 3
struct scmp_ctx_info {
uint32_t architectures[SCMP_ARCH_INDEX_MAX];
enum lxc_arch_t lxc_arch[SCMP_ARCH_INDEX_MAX];
scmp_filter_ctx contexts[SCMP_ARCH_INDEX_MAX];
bool needs_merge[SCMP_ARCH_INDEX_MAX];
};
static int get_arch_index(enum lxc_arch_t arch, struct scmp_ctx_info *ctx)
{
int i;
for (i = 0; i < SCMP_ARCH_INDEX_MAX; i++) {
if (ctx->lxc_arch[i] == arch)
return i;
}
return -1;
}
/* /*
* v2 consists of * v2 consists of
* [x86] * [x86]
...@@ -586,11 +568,15 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -586,11 +568,15 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
{ {
int ret; int ret;
char *p; char *p;
enum lxc_arch_t cur_rule_arch, native_arch; enum lxc_hostarch_t cur_rule_arch, native_arch;
bool blacklist = false; bool blacklist = false;
uint32_t default_policy_action = -1, default_rule_action = -1; uint32_t default_policy_action = -1, default_rule_action = -1;
struct seccomp_v2_rule rule; struct seccomp_v2_rule rule;
struct scmp_ctx_info ctx; struct scmp_ctx_info {
uint32_t architectures[3];
scmp_filter_ctx contexts[3];
bool needs_merge[3];
} ctx;
if (strncmp(line, "blacklist", 9) == 0) if (strncmp(line, "blacklist", 9) == 0)
blacklist = true; blacklist = true;
...@@ -631,23 +617,23 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -631,23 +617,23 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
ctx.architectures[0] = SCMP_ARCH_X86; ctx.architectures[0] = SCMP_ARCH_X86;
ctx.lxc_arch[0] = lxc_seccomp_arch_i386;
ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_i386, ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_i386,
default_policy_action); default_policy_action,
&ctx.needs_merge[0]);
if (!ctx.contexts[0]) if (!ctx.contexts[0])
goto bad; goto bad;
ctx.architectures[1] = SCMP_ARCH_X32; ctx.architectures[1] = SCMP_ARCH_X32;
ctx.lxc_arch[1] = lxc_seccomp_arch_x32;
ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_x32, ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_x32,
default_policy_action); default_policy_action,
&ctx.needs_merge[1]);
if (!ctx.contexts[1]) if (!ctx.contexts[1])
goto bad; goto bad;
ctx.architectures[2] = SCMP_ARCH_X86_64; ctx.architectures[2] = SCMP_ARCH_X86_64;
ctx.lxc_arch[2] = lxc_seccomp_arch_amd64;
ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_amd64, ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_amd64,
default_policy_action); default_policy_action,
&ctx.needs_merge[2]);
if (!ctx.contexts[2]) if (!ctx.contexts[2])
goto bad; goto bad;
#ifdef SCMP_ARCH_PPC #ifdef SCMP_ARCH_PPC
...@@ -655,17 +641,17 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -655,17 +641,17 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
ctx.architectures[0] = SCMP_ARCH_PPC; ctx.architectures[0] = SCMP_ARCH_PPC;
ctx.lxc_arch[0] = lxc_seccomp_arch_ppc;
ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_ppc, ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_ppc,
default_policy_action); default_policy_action,
&ctx.needs_merge[0]);
if (!ctx.contexts[0]) if (!ctx.contexts[0])
goto bad; goto bad;
ctx.architectures[1] = SCMP_ARCH_PPC64; ctx.architectures[2] = SCMP_ARCH_PPC64;
ctx.lxc_arch[1] = lxc_seccomp_arch_ppc64; ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_ppc64,
ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_ppc64, default_policy_action,
default_policy_action); &ctx.needs_merge[2]);
if (!ctx.contexts[1]) if (!ctx.contexts[2])
goto bad; goto bad;
#endif #endif
#ifdef SCMP_ARCH_ARM #ifdef SCMP_ARCH_ARM
...@@ -673,18 +659,18 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -673,18 +659,18 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
ctx.architectures[0] = SCMP_ARCH_ARM; ctx.architectures[0] = SCMP_ARCH_ARM;
ctx.lxc_arch[0] = lxc_seccomp_arch_arm;
ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_arm, ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_arm,
default_policy_action); default_policy_action,
&ctx.needs_merge[0]);
if (!ctx.contexts[0]) if (!ctx.contexts[0])
goto bad; goto bad;
#ifdef SCMP_ARCH_AARCH64 #ifdef SCMP_ARCH_AARCH64
ctx.architectures[1] = SCMP_ARCH_AARCH64; ctx.architectures[2] = SCMP_ARCH_AARCH64;
ctx.lxc_arch[1] = lxc_seccomp_arch_arm64; ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_arm64,
ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_arm64, default_policy_action,
default_policy_action); &ctx.needs_merge[2]);
if (!ctx.contexts[1]) if (!ctx.contexts[2])
goto bad; goto bad;
#endif #endif
#endif #endif
...@@ -693,46 +679,46 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -693,46 +679,46 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
ctx.architectures[0] = SCMP_ARCH_MIPS; ctx.architectures[0] = SCMP_ARCH_MIPS;
ctx.lxc_arch[0] = lxc_seccomp_arch_mips;
ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_mips, ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_mips,
default_policy_action); default_policy_action,
&ctx.needs_merge[0]);
if (!ctx.contexts[0]) if (!ctx.contexts[0])
goto bad; goto bad;
ctx.architectures[1] = SCMP_ARCH_MIPS64N32; ctx.architectures[1] = SCMP_ARCH_MIPS64N32;
ctx.lxc_arch[1] = lxc_seccomp_arch_mips64n32;
ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_mips64n32, ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_mips64n32,
default_policy_action); default_policy_action,
&ctx.needs_merge[1]);
if (!ctx.contexts[1]) if (!ctx.contexts[1])
goto bad; goto bad;
ctx.architectures[2] = SCMP_ARCH_MIPS64; ctx.architectures[2] = SCMP_ARCH_MIPS64;
ctx.lxc_arch[2] = lxc_seccomp_arch_mips64;
ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_mips64, ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_mips64,
default_policy_action); default_policy_action,
&ctx.needs_merge[2]);
if (!ctx.contexts[2]) if (!ctx.contexts[2])
goto bad; goto bad;
} else if (native_arch == lxc_seccomp_arch_mipsel64) { } else if (native_arch == lxc_seccomp_arch_mipsel64) {
cur_rule_arch = lxc_seccomp_arch_all; cur_rule_arch = lxc_seccomp_arch_all;
ctx.architectures[0] = SCMP_ARCH_MIPSEL; ctx.architectures[0] = SCMP_ARCH_MIPSEL;
ctx.lxc_arch[0] = lxc_seccomp_arch_mipsel;
ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_mipsel, ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_mipsel,
default_policy_action); default_policy_action,
&ctx.needs_merge[0]);
if (!ctx.contexts[0]) if (!ctx.contexts[0])
goto bad; goto bad;
ctx.architectures[1] = SCMP_ARCH_MIPSEL64N32; ctx.architectures[1] = SCMP_ARCH_MIPSEL64N32;
ctx.lxc_arch[1] = lxc_seccomp_arch_mipsel64n32;
ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_mipsel64n32, ctx.contexts[1] = get_new_ctx(lxc_seccomp_arch_mipsel64n32,
default_policy_action); default_policy_action,
&ctx.needs_merge[1]);
if (!ctx.contexts[1]) if (!ctx.contexts[1])
goto bad; goto bad;
ctx.architectures[2] = SCMP_ARCH_MIPSEL64; ctx.architectures[2] = SCMP_ARCH_MIPSEL64;
ctx.lxc_arch[2] = lxc_seccomp_arch_mipsel64;
ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_mipsel64, ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_mipsel64,
default_policy_action); default_policy_action,
&ctx.needs_merge[2]);
if (!ctx.contexts[2]) if (!ctx.contexts[2])
goto bad; goto bad;
#endif #endif
...@@ -942,7 +928,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -942,7 +928,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
goto bad_rule; goto bad_rule;
} }
if (cur_rule_arch == native_arch) {
if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line, if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line,
conf->seccomp_ctx, &rule)) conf->seccomp_ctx, &rule))
goto bad_rule; goto bad_rule;
...@@ -950,20 +935,7 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -950,20 +935,7 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
INFO("Added native rule for arch %d for %s action %d(%s)", INFO("Added native rule for arch %d for %s action %d(%s)",
SCMP_ARCH_NATIVE, line, rule.action, SCMP_ARCH_NATIVE, line, rule.action,
get_action_name(rule.action)); get_action_name(rule.action));
} else if (cur_rule_arch != lxc_seccomp_arch_all) {
int arch_index = get_arch_index(cur_rule_arch, &ctx);
if (arch_index < 0)
goto bad_arch;
if (!do_resolve_add_rule(ctx.architectures[arch_index], line,
ctx.contexts[arch_index], &rule))
goto bad_rule;
INFO("Added compat rule for arch %d for %s action %d(%s)",
ctx.architectures[arch_index], line, rule.action,
get_action_name(rule.action));
ctx.needs_merge[arch_index] = true;
} else {
if (ctx.architectures[0] != SCMP_ARCH_NATIVE) { if (ctx.architectures[0] != SCMP_ARCH_NATIVE) {
if (!do_resolve_add_rule(ctx.architectures[0], line, if (!do_resolve_add_rule(ctx.architectures[0], line,
ctx.contexts[0], &rule)) ctx.contexts[0], &rule))
...@@ -972,7 +944,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -972,7 +944,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
INFO("Added compat rule for arch %d for %s action %d(%s)", INFO("Added compat rule for arch %d for %s action %d(%s)",
ctx.architectures[0], line, rule.action, ctx.architectures[0], line, rule.action,
get_action_name(rule.action)); get_action_name(rule.action));
ctx.needs_merge[0] = true;
} }
if (ctx.architectures[1] != SCMP_ARCH_NATIVE) { if (ctx.architectures[1] != SCMP_ARCH_NATIVE) {
...@@ -983,7 +954,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -983,7 +954,6 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
INFO("Added compat rule for arch %d for %s action %d(%s)", INFO("Added compat rule for arch %d for %s action %d(%s)",
ctx.architectures[1], line, rule.action, ctx.architectures[1], line, rule.action,
get_action_name(rule.action)); get_action_name(rule.action));
ctx.needs_merge[1] = true;
} }
if (ctx.architectures[2] != SCMP_ARCH_NATIVE) { if (ctx.architectures[2] != SCMP_ARCH_NATIVE) {
...@@ -994,19 +964,16 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -994,19 +964,16 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
INFO("Added native rule for arch %d for %s action %d(%s)", INFO("Added native rule for arch %d for %s action %d(%s)",
ctx.architectures[2], line, rule.action, ctx.architectures[2], line, rule.action,
get_action_name(rule.action)); get_action_name(rule.action));
ctx.needs_merge[2] = true;
} }
} }
}
INFO("Merging compat seccomp contexts into main context"); INFO("Merging compat seccomp contexts into main context");
if (ctx.contexts[0]) { if (ctx.contexts[0]) {
if (ctx.needs_merge[0]) { if (ctx.needs_merge[0]) {
ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[0]); ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[0]);
if (ret < 0) { if (ret < 0) {
ERROR("%s - Failed to merge first compat seccomp " ERROR("Failed to merge first compat seccomp "
"context into main context", strerror(-ret)); "context into main context");
goto bad; goto bad;
} }
...@@ -1021,8 +988,8 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -1021,8 +988,8 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
if (ctx.needs_merge[1]) { if (ctx.needs_merge[1]) {
ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[1]); ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[1]);
if (ret < 0) { if (ret < 0) {
ERROR("%s - Failed to merge second compat seccomp " ERROR("Failed to merge first compat seccomp "
"context into main context", strerror(-ret)); "context into main context");
goto bad; goto bad;
} }
...@@ -1037,8 +1004,8 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c ...@@ -1037,8 +1004,8 @@ static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_c
if (ctx.needs_merge[2]) { if (ctx.needs_merge[2]) {
ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[2]); ret = seccomp_merge(conf->seccomp_ctx, ctx.contexts[2]);
if (ret < 0) { if (ret < 0) {
ERROR("%s - Failed to merge third compat seccomp " ERROR("Failed to merge third compat seccomp "
"context into main context", strerror(-ret)); "context into main context");
goto bad; goto bad;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment