Skip to content

Commit

Permalink
seccomp: cleanup
Browse files Browse the repository at this point in the history 
Simplify and cleanup some of the seccomp code. This mainly focuses on removing
the open coding of various seccomp settings all over the code place in favor of
centralized helpers.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
  • Loading branch information
Christian Brauner committed Apr 29, 2019
1 parent 41cd8a8 commit c3e3c21
Show file tree
Hide file tree
Showing 14 changed files with 375 additions and 213 deletions.
22 changes: 18 additions & 4 deletions src/lxc/af_unix.c
View file
Expand Up @@ -156,7 +156,8 @@ int lxc_abstract_unix_connect(const char *path)
int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
void *data, size_t size)
{
__do_free char *cmsgbuf;
__do_free char *cmsgbuf = NULL;
int ret;
struct msghdr msg;
struct iovec iov;
struct cmsghdr *cmsg = NULL;
Expand Down Expand Up @@ -189,13 +190,19 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
msg.msg_iov = &iov;
msg.msg_iovlen = 1;

return sendmsg(fd, &msg, MSG_NOSIGNAL);
again:
ret = sendmsg(fd, &msg, MSG_NOSIGNAL);
if (ret < 0)
if (errno == EINTR)
goto again;

return ret;
}

int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
void *data, size_t size)
{
__do_free char *cmsgbuf;
__do_free char *cmsgbuf = NULL;
int ret;
struct msghdr msg;
struct iovec iov;
Expand All @@ -221,8 +228,15 @@ int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
msg.msg_iov = &iov;
msg.msg_iovlen = 1;

again:
ret = recvmsg(fd, &msg, 0);
if (ret <= 0)
if (ret < 0) {
if (errno == EINTR)
goto again;

goto out;
}
if (ret == 0)
goto out;

/*
Expand Down
44 changes: 12 additions & 32 deletions src/lxc/attach.c
View file
Expand Up @@ -608,8 +608,8 @@ static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options

if (!(options->namespaces & CLONE_NEWNS) ||
!(options->attach_flags & LXC_ATTACH_LSM)) {
free(c->lxc_conf->seccomp);
c->lxc_conf->seccomp = NULL;
free(c->lxc_conf->seccomp.seccomp);
c->lxc_conf->seccomp.seccomp = NULL;
return true;
}

Expand Down Expand Up @@ -852,7 +852,7 @@ static int attach_child_main(struct attach_clone_payload *payload)
}

if (init_ctx->container && init_ctx->container->lxc_conf &&
init_ctx->container->lxc_conf->seccomp) {
init_ctx->container->lxc_conf->seccomp.seccomp) {
struct lxc_conf *conf = init_ctx->container->lxc_conf;

ret = lxc_seccomp_load(conf);
Expand All @@ -861,18 +861,9 @@ static int attach_child_main(struct attach_clone_payload *payload)

TRACE("Loaded seccomp profile");

#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
if (conf->has_seccomp_notify) {
ret = lxc_abstract_unix_send_fds(payload->ipc_socket,
&conf->seccomp_notify_fd,
1, NULL, 0);
close_prot_errno_disarm(conf->seccomp_notify_fd);
if (ret < 0)
goto on_error;

TRACE("Sent seccomp listener fd to parent");
}
#endif
ret = lxc_seccomp_send_notifier_fd(&conf->seccomp, payload->ipc_socket);
if (ret < 0)
goto on_error;
}

close(payload->ipc_socket);
Expand Down Expand Up @@ -1326,24 +1317,13 @@ int lxc_attach(const char *name, const char *lxcpath,
TRACE("Sent LSM label file descriptor %d to child", labelfd);
}

#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
if (conf->seccomp && conf->has_seccomp_notify) {
ret = lxc_abstract_unix_recv_fds(ipc_sockets[0],
&conf->seccomp_notify_fd,
1, NULL, 0);
if (ret < 0)
goto close_mainloop;
ret = lxc_seccomp_recv_notifier_fd(&conf->seccomp, ipc_sockets[0]);
if (ret < 0)
goto close_mainloop;

TRACE("Retrieved seccomp listener fd %d from child",
conf->seccomp_notify_fd);
ret = lxc_cmd_seccomp_notify_add_listener(name, lxcpath,
conf->seccomp_notify_fd,
-1, 0);
close_prot_errno_disarm(conf->seccomp_notify_fd);
if (ret < 0)
goto close_mainloop;
}
#endif
ret = lxc_seccomp_add_notifier(name, lxcpath, &conf->seccomp);
if (ret < 0)
goto close_mainloop;

/* We're done, the child process should now execute whatever it
* is that the user requested. The parent can now track it with
Expand Down
46 changes: 21 additions & 25 deletions src/lxc/commands.c
View file
Expand Up @@ -1053,7 +1053,7 @@ int lxc_cmd_seccomp_notify_add_listener(const char *name, const char *lxcpath,
/* unused */ unsigned int flags)
{

#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
#ifdef HAVE_SECCOMP_NOTIFY
int ret, stopped;
struct lxc_cmd_rr cmd = {
.req = {
Expand All @@ -1080,43 +1080,39 @@ static int lxc_cmd_seccomp_notify_add_listener_callback(int fd,
struct lxc_epoll_descr *descr)
{
struct lxc_cmd_rsp rsp = {0};
int ret;

#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
#ifdef HAVE_SECCOMP_NOTIFY
int ret;
__do_close_prot_errno int recv_fd = -EBADF;
int notify_fd = -EBADF;

if (!handler->conf->has_seccomp_notify ||
handler->conf->seccomp_notify_proxy_fd < 0) {
rsp.ret = -EINVAL;
goto send_error;
ret = lxc_abstract_unix_recv_fds(fd, &recv_fd, 1, NULL, 0);
if (ret <= 0) {
rsp.ret = -errno;
goto out;
}

ret = lxc_abstract_unix_recv_fds(fd, &recv_fd, 1, NULL, 0);
if (ret <= 0)
goto reap_client_fd;
if (!handler->conf->seccomp.notifier.wants_supervision ||
handler->conf->seccomp.notifier.proxy_fd < 0) {
SYSERROR("No seccomp proxy fd specified");
rsp.ret = -EINVAL;
goto out;
}

ret = lxc_mainloop_add_handler(descr, notify_fd, seccomp_notify_handler,
ret = lxc_mainloop_add_handler(descr, recv_fd, seccomp_notify_handler,
handler);
if (ret < 0) {
rsp.ret = -errno;
goto out;
}
notify_fd = move_fd(recv_fd);
if (ret < 0)
goto reap_client_fd;

send_error:
out:
#else
rsp.ret = -ENOSYS;
#endif
ret = lxc_cmd_rsp_send(fd, &rsp);
if (ret < 0)
goto reap_client_fd;

return 0;

reap_client_fd:
/* Special indicator to lxc_cmd_handler() to close the fd and do
* related cleanup.
*/
return 1;
#endif
return lxc_cmd_rsp_send(fd, &rsp);
}

static int lxc_cmd_process(int fd, struct lxc_cmd_req *req,
Expand Down
11 changes: 2 additions & 9 deletions src/lxc/conf.c
View file
Expand Up @@ -2752,14 +2752,6 @@ struct lxc_conf *lxc_conf_init(void)
new->lsm_aa_profile = NULL;
lxc_list_init(&new->lsm_aa_raw);
new->lsm_se_context = NULL;
#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
new->has_seccomp_notify = false;
new->seccomp_notify_fd = -EBADF;
new->seccomp_notify_proxy_fd = -EBADF;
memset(&new->seccomp_notify_proxy_addr, 0, sizeof(new->seccomp_notify_proxy_addr));
new->seccomp_notify_req = NULL;
new->seccomp_notify_resp = NULL;
#endif
new->tmp_umount_proc = false;
new->tmp_umount_proc = 0;
new->shmount.path_host = NULL;
Expand All @@ -2771,6 +2763,7 @@ struct lxc_conf *lxc_conf_init(void)
new->init_gid = 0;
memset(&new->cgroup_meta, 0, sizeof(struct lxc_cgroup));
memset(&new->ns_share, 0, sizeof(char *) * LXC_NS_MAX);
seccomp_conf_init(new);

return new;
}
Expand Down Expand Up @@ -4074,7 +4067,7 @@ void lxc_conf_free(struct lxc_conf *conf)
free(conf->lsm_aa_profile);
free(conf->lsm_aa_profile_computed);
free(conf->lsm_se_context);
lxc_seccomp_free(conf);
lxc_seccomp_free(&conf->seccomp);
lxc_clear_config_caps(conf);
lxc_clear_config_keepcaps(conf);
lxc_clear_cgroups(conf, "lxc.cgroup", CGROUP_SUPER_MAGIC);
Expand Down
15 changes: 2 additions & 13 deletions src/lxc/conf.h
View file
Expand Up @@ -38,6 +38,7 @@
#include "compiler.h"
#include "config.h"
#include "list.h"
#include "lxcseccomp.h"
#include "ringbuf.h"
#include "start.h"
#include "terminal.h"
Expand Down Expand Up @@ -295,19 +296,7 @@ struct lxc_conf {
struct lxc_list lsm_aa_raw;
char *lsm_se_context;
bool tmp_umount_proc;
char *seccomp; /* filename with the seccomp rules */
unsigned int seccomp_allow_nesting;
#if HAVE_SCMP_FILTER_CTX
scmp_filter_ctx seccomp_ctx;
#endif
#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
bool has_seccomp_notify;
int seccomp_notify_fd;
int seccomp_notify_proxy_fd;
struct sockaddr_un seccomp_notify_proxy_addr;
struct seccomp_notif *seccomp_notify_req;
struct seccomp_notif_resp *seccomp_notify_resp;
#endif
struct lxc_seccomp seccomp;
int maincmd_fd;
unsigned int autodev; /* if 1, mount and fill a /dev at start */
int haltsignal; /* signal used to halt container */
Expand Down
49 changes: 32 additions & 17 deletions src/lxc/confile.c
View file
Expand Up @@ -780,22 +780,27 @@ static int add_hook(struct lxc_conf *lxc_conf, int which, char *hook)
static int set_config_seccomp_allow_nesting(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
#ifdef HAVE_SECCOMP
if (lxc_config_value_empty(value))
return clr_config_seccomp_allow_nesting(key, lxc_conf, NULL);

if (lxc_safe_uint(value, &lxc_conf->seccomp_allow_nesting) < 0)
if (lxc_safe_uint(value, &lxc_conf->seccomp.allow_nesting) < 0)
return -1;

if (lxc_conf->seccomp_allow_nesting > 1)
if (lxc_conf->seccomp.allow_nesting > 1)
return minus_one_set_errno(EINVAL);

return 0;
#else
errno = ENOSYS;
return -1;
#endif
}

static int set_config_seccomp_notify_proxy(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
#ifdef HAVE_SECCOMP_NOTIFY
const char *offset;

if (lxc_config_value_empty(value))
Expand All @@ -805,7 +810,7 @@ static int set_config_seccomp_notify_proxy(const char *key, const char *value,
return minus_one_set_errno(EINVAL);

offset = value + 5;
if (lxc_unix_sockaddr(&lxc_conf->seccomp_notify_proxy_addr, offset) < 0)
if (lxc_unix_sockaddr(&lxc_conf->seccomp.notifier.proxy_addr, offset) < 0)
return -1;

return 0;
Expand All @@ -817,7 +822,7 @@ static int set_config_seccomp_notify_proxy(const char *key, const char *value,
static int set_config_seccomp_profile(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
return set_config_path_item(&lxc_conf->seccomp, value);
return set_config_path_item(&lxc_conf->seccomp.seccomp, value);
}

static int set_config_execute_cmd(const char *key, const char *value,
Expand Down Expand Up @@ -3726,17 +3731,22 @@ static int get_config_seccomp_allow_nesting(const char *key, char *retv,
int inlen, struct lxc_conf *c,
void *data)
{
return lxc_get_conf_int(c, retv, inlen, c->seccomp_allow_nesting);
#ifdef HAVE_SECCOMP
return lxc_get_conf_int(c, retv, inlen, c->seccomp.allow_nesting);
#else
errno = ENOSYS;
return -1;
#endif
}

static int get_config_seccomp_notify_proxy(const char *key, char *retv, int inlen,
struct lxc_conf *c, void *data)
{
#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
#ifdef HAVE_SECCOMP_NOTIFY
return lxc_get_conf_str(retv, inlen,
(c->seccomp_notify_proxy_addr.sun_path[0]) == '/'
? &c->seccomp_notify_proxy_addr.sun_path[0]
: &c->seccomp_notify_proxy_addr.sun_path[1]);
(c->seccomp.notifier.proxy_addr.sun_path[0]) == '/'
? &c->seccomp.notifier.proxy_addr.sun_path[0]
: &c->seccomp.notifier.proxy_addr.sun_path[1]);
#else
return minus_one_set_errno(ENOSYS);
#endif
Expand All @@ -3745,7 +3755,7 @@ static int get_config_seccomp_notify_proxy(const char *key, char *retv, int inle
static int get_config_seccomp_profile(const char *key, char *retv, int inlen,
struct lxc_conf *c, void *data)
{
return lxc_get_conf_str(retv, inlen, c->seccomp);
return lxc_get_conf_str(retv, inlen, c->seccomp.seccomp);
}

static int get_config_autodev(const char *key, char *retv, int inlen,
Expand Down Expand Up @@ -4328,16 +4338,21 @@ static inline int clr_config_console_size(const char *key, struct lxc_conf *c,
static inline int clr_config_seccomp_allow_nesting(const char *key,
struct lxc_conf *c, void *data)
{
c->seccomp_allow_nesting = 0;
#ifdef HAVE_SECCOMP
c->seccomp.allow_nesting = 0;
return 0;
#else
errno = ENOSYS;
return -1;
#endif
}

static inline int clr_config_seccomp_notify_proxy(const char *key,
struct lxc_conf *c, void *data)
{
#if HAVE_DECL_SECCOMP_NOTIF_GET_FD
memset(&c->seccomp_notify_proxy_addr, 0,
sizeof(c->seccomp_notify_proxy_addr));
#ifdef HAVE_SECCOMP_NOTIFY
memset(&c->seccomp.notifier.proxy_addr, 0,
sizeof(c->seccomp.notifier.proxy_addr));
return 0;
#else
return minus_one_set_errno(ENOSYS);
Expand All @@ -4347,8 +4362,8 @@ static inline int clr_config_seccomp_notify_proxy(const char *key,
static inline int clr_config_seccomp_profile(const char *key,
struct lxc_conf *c, void *data)
{
free(c->seccomp);
c->seccomp = NULL;
free(c->seccomp.seccomp);
c->seccomp.seccomp = NULL;
return 0;
}

Expand Down

0 comments on commit c3e3c21

Please sign in to comment.