Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

container: cache the seccomp generated bpf #1035

Merged
merged 9 commits into from
Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Expand Up @@ -61,7 +61,7 @@ These dependencies are required for the build:

```console
$ sudo dnf install -y make python git gcc automake autoconf libcap-devel \
systemd-devel yajl-devel libseccomp-devel pkg-config \
systemd-devel yajl-devel libseccomp-devel pkg-config libgcrypt-devel \
go-md2man glibc-static python3-libmount libtool
```

Expand All @@ -70,7 +70,7 @@ $ sudo dnf install -y make python git gcc automake autoconf libcap-devel \
```console
$ sudo yum --enablerepo='*' --disablerepo='media-*' install -y make automake \
autoconf gettext \
libtool gcc libcap-devel systemd-devel yajl-devel \
libtool gcc libcap-devel systemd-devel yajl-devel libgcrypt-devel \
glibc-static libseccomp-devel python36 git
```

Expand All @@ -90,7 +90,7 @@ $ export PATH=$PATH:$GOPATH/bin
```console
$ sudo apt-get install -y make git gcc build-essential pkgconf libtool \
libsystemd-dev libprotobuf-c-dev libcap-dev libseccomp-dev libyajl-dev \
go-md2man autoconf python3 automake
libgcrypt20-dev go-md2man autoconf python3 automake
```

### Alpine
Expand Down
3 changes: 3 additions & 0 deletions configure.ac
Expand Up @@ -64,6 +64,9 @@ AS_IF([test "x$enable_caps" != "xno"], [
])
])

dnl libgcrypt
AC_SEARCH_LIBS([gcry_check_version], [gcrypt], [AC_DEFINE([HAVE_GCRYPT], 1, [Define if gcrypt is available])], [])

dnl dl
AC_ARG_ENABLE([dl], AS_HELP_STRING([--disable-dl], [Disable dynamic libraries support]))
AS_IF([test "x$enable_dl" != "xno"], [
Expand Down
3 changes: 3 additions & 0 deletions src/crun.c
Expand Up @@ -30,6 +30,7 @@
#include "crun.h"
#include "libcrun/utils.h"
#include "libcrun/custom-handler.h"
#include "libcrun/status.h"

/* Commands. */
#include "run.h"
Expand Down Expand Up @@ -222,8 +223,10 @@ static struct argp_option options[] = { { "debug", OPTION_DEBUG, 0, 0, "produce
static void
print_version (FILE *stream, struct argp_state *state arg_unused)
{
cleanup_free char *rundir = libcrun_get_state_directory (arguments.root, NULL);
fprintf (stream, "%s version %s\n", PACKAGE_NAME, PACKAGE_VERSION);
fprintf (stream, "commit: %s\n", GIT_VERSION);
fprintf (stream, "rundir: %s\n", rundir);
fprintf (stream, "spec: 1.0.0\n");
#ifdef HAVE_SYSTEMD
fprintf (stream, "+SYSTEMD ");
Expand Down
30 changes: 17 additions & 13 deletions src/libcrun/container.c
Expand Up @@ -2174,6 +2174,8 @@ libcrun_container_run_internal (libcrun_container_t *container, libcrun_context_
.custom_handler = NULL,
.handler_cookie = NULL,
};
const char *seccomp_bpf_data = find_annotation (container, "run.oci.seccomp_bpf_data");
unsigned int seccomp_gen_options = 0;

if (def->hooks
&& (def->hooks->prestart_len || def->hooks->poststart_len || def->hooks->create_runtime_len
Expand Down Expand Up @@ -2222,8 +2224,14 @@ libcrun_container_run_internal (libcrun_container_t *container, libcrun_context_
if (UNLIKELY (ret < 0))
return ret;

if (def->linux && (def->linux->seccomp || find_annotation (container, "run.oci.seccomp_bpf_data")))
if (def->linux && (def->linux->seccomp || seccomp_bpf_data))
{
const char *annotation;

annotation = find_annotation (container, "run.oci.seccomp_fail_unknown_syscall");
if (annotation && strcmp (annotation, "0") != 0)
seccomp_gen_options = LIBCRUN_SECCOMP_FAIL_UNKNOWN_SYSCALL;

ret = open_seccomp_output (context->id, &seccomp_fd, false, context->state_root, err);
if (UNLIKELY (ret < 0))
return ret;
Expand Down Expand Up @@ -2338,24 +2346,17 @@ libcrun_container_run_internal (libcrun_container_t *container, libcrun_context_

if (seccomp_fd >= 0)
{
unsigned int seccomp_gen_options = 0;
const char *annotation;

annotation = find_annotation (container, "run.oci.seccomp_fail_unknown_syscall");
if (annotation && strcmp (annotation, "0") != 0)
seccomp_gen_options = LIBCRUN_SECCOMP_FAIL_UNKNOWN_SYSCALL;

if ((annotation = find_annotation (container, "run.oci.seccomp_bpf_data")) != NULL)
if (seccomp_bpf_data != NULL)
{
cleanup_free char *bpf_data = NULL;
size_t size = 0;
size_t in_size;
int consumed;

in_size = strlen (annotation);
in_size = strlen (seccomp_bpf_data);
bpf_data = xmalloc (in_size + 1);

consumed = base64_decode (annotation, in_size, bpf_data, in_size, &size);
consumed = base64_decode (seccomp_bpf_data, in_size, bpf_data, in_size, &size);
if (UNLIKELY (consumed != (int) in_size))
{
ret = crun_make_error (err, 0, "invalid seccomp BPF data");
Expand Down Expand Up @@ -3275,6 +3276,8 @@ libcrun_container_exec_with_options (libcrun_context_t *context, const char *id,
if (container == NULL)
return crun_make_error (err, 0, "error loading config.json");

container->context = context;

if (container_status == 0)
return crun_make_error (err, 0, "the container `%s` is not running", id);

Expand All @@ -3301,8 +3304,9 @@ libcrun_container_exec_with_options (libcrun_context_t *context, const char *id,

if (seccomp_fd >= 0)
{
ret = get_seccomp_receiver_fd (container, &seccomp_receiver_fd, &own_seccomp_receiver_fd, &seccomp_notify_plugins,
err);
ret = get_seccomp_receiver_fd (container, &seccomp_receiver_fd,
&own_seccomp_receiver_fd,
&seccomp_notify_plugins, err);
if (UNLIKELY (ret < 0))
return ret;
}
Expand Down
99 changes: 99 additions & 0 deletions src/libcrun/seccomp.c
Expand Up @@ -36,6 +36,10 @@
#include <sys/stat.h>
#include <sys/mman.h>

#if HAVE_GCRYPT
# include <gcrypt.h>
#endif

#if HAVE_STDATOMIC_H
# include <stdatomic.h>
#else
Expand Down Expand Up @@ -340,6 +344,101 @@ seccomp_action_supports_errno (const char *action)
|| strcmp (action, "SCMP_ACT_TRACE") == 0;
}

static int
calculate_seccomp_checksum (libcrun_container_t *container, unsigned int seccomp_gen_options, seccomp_checksum_t out, libcrun_error_t *err)
{
#if HAVE_GCRYPT
runtime_spec_schema_config_linux_seccomp *seccomp;
gcry_error_t gcrypt_err;
unsigned char *res;
gcry_md_hd_t hd;
size_t i;

# define PROCESS_STRING(X) \
do \
{ \
if (X) \
{ \
gcry_md_write (hd, (X), strlen (X)); \
} \
} while (0)
# define PROCESS_DATA(X) \
do \
{ \
gcry_md_write (hd, &(X), sizeof (X)); \
} while (0)

out[0] = 0;

if (container == NULL || container->container_def == NULL || container->container_def->linux == NULL)
return 0;

seccomp = container->container_def->linux->seccomp;
if (seccomp == NULL)
return 0;

gcrypt_err = gcry_md_open (&hd, GCRY_MD_SHA256, 0);
if (gcrypt_err)
return crun_make_error (err, EINVAL, "internal libgcrypt error: %s", gcry_strerror (gcrypt_err));

PROCESS_STRING (PACKAGE_VERSION);

# ifdef HAVE_SECCOMP
{
const struct scmp_version *version = seccomp_version ();

PROCESS_DATA (version->major);
PROCESS_DATA (version->minor);
PROCESS_DATA (version->micro);
}
# endif

PROCESS_DATA (seccomp_gen_options);

PROCESS_STRING (seccomp->default_action);
for (i = 0; i < seccomp->flags_len; i++)
PROCESS_STRING (seccomp->flags[i]);
for (i = 0; i < seccomp->architectures_len; i++)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is enough, or if we also want to hash the current architecture? Say we seccomp->archtetures is ["i386"]. Will this give the same result when run on a i386 arch and a x86_64 arch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, better be safe. Would hashing the result of uname(2) be enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed a new version where we also hash uname(2)

PROCESS_STRING (seccomp->architectures[i]);
for (i = 0; i < seccomp->syscalls_len; i++)
{
size_t j;

if (seccomp->syscalls[i]->action)
PROCESS_STRING (seccomp->syscalls[i]->action);
for (j = 0; j < seccomp->syscalls[i]->names_len; j++)
PROCESS_STRING (seccomp->syscalls[i]->names[j]);
for (j = 0; j < seccomp->syscalls[i]->args_len; j++)
{
if (seccomp->syscalls[i]->args[j]->index_present)
PROCESS_DATA (seccomp->syscalls[i]->args[j]->index);
if (seccomp->syscalls[i]->args[j]->value_present)
PROCESS_DATA (seccomp->syscalls[i]->args[j]->value);
if (seccomp->syscalls[i]->args[j]->value_two_present)
PROCESS_DATA (seccomp->syscalls[i]->args[j]->value_two);
PROCESS_STRING (seccomp->syscalls[i]->args[j]->op);
}
}

res = gcry_md_read (hd, GCRY_MD_SHA256);
for (i = 0; i < 32; i++)
sprintf (&out[i * 2], "%02x", res[i]);
out[64] = 0;

gcry_md_close (hd);

# undef PROCESS_STRING
# undef PROCESS_DATA
#else
(void) container;
(void) seccomp_gen_options;
(void) out;
(void) err;
out[0] = 0;
#endif
return 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't fully analyzed this, but this not returning an error here, or in the early NULL checks for container->container_def->linux->seccomp scares me. It feels like we may accidentally rely on the output digest even though it is not written to.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only caller of this function is find_in_cache that will return early when the checksum is an empty string:

  ret = calculate_seccomp_checksum (ctx->container, ctx->options, ctx->checksum, err);
  if (UNLIKELY (ret < 0))
    return ret;

  if (is_empty_string (ctx->checksum))
    return 0;

I'll make it clearer and use a different argument instead of relying on the checksum return value.

}

int
libcrun_generate_seccomp (libcrun_container_t *container, int outfd, unsigned int options, libcrun_error_t *err)
{
Expand Down
2 changes: 2 additions & 0 deletions src/libcrun/seccomp.h
Expand Up @@ -31,6 +31,8 @@ enum
LIBCRUN_SECCOMP_FAIL_UNKNOWN_SYSCALL = 1 << 0,
};

typedef char seccomp_checksum_t[65];

int libcrun_generate_seccomp (libcrun_container_t *container, int outfd, unsigned int options, libcrun_error_t *err);
int libcrun_apply_seccomp (int infd, int listener_receiver_fd, const char *receiver_fd_payload,
size_t receiver_fd_payload_len, char **flags, size_t flags_len, libcrun_error_t *err);
Expand Down
2 changes: 1 addition & 1 deletion tests/alpine-build/Dockerfile
@@ -1,7 +1,7 @@
FROM alpine

RUN apk add gcc automake autoconf libtool gettext pkgconf git make musl-dev \
python3 libcap-dev libseccomp-dev yajl-dev argp-standalone go-md2man
python3 libcap-dev libgcrypt-dev libseccomp-dev yajl-dev argp-standalone go-md2man

COPY run-tests.sh /usr/local/bin

Expand Down
2 changes: 1 addition & 1 deletion tests/centos8-build/Dockerfile
Expand Up @@ -2,7 +2,7 @@ FROM quay.io/centos/centos:stream8

RUN yum --enablerepo='powertools' install -y make automake autoconf gettext \
criu-devel libtool gcc libcap-devel systemd-devel yajl-devel \
libseccomp-devel python36 libtool git
libgcrypt-devel libseccomp-devel python36 libtool git

COPY run-tests.sh /usr/local/bin

Expand Down
2 changes: 1 addition & 1 deletion tests/centos9-build/Dockerfile
Expand Up @@ -2,7 +2,7 @@ FROM quay.io/centos/centos:stream9

RUN yum --enablerepo='appstream' --enablerepo='baseos' --enablerepo='crb' install -y make \
automake autoconf gettext criu-devel libtool gcc libcap-devel systemd-devel yajl-devel \
libseccomp-devel python3 libtool git protobuf-c protobuf-c-devel
libgcrypt-devel libseccomp-devel python3 libtool git protobuf-c protobuf-c-devel

COPY run-tests.sh /usr/local/bin

Expand Down
2 changes: 1 addition & 1 deletion tests/containerd/Dockerfile
Expand Up @@ -6,7 +6,7 @@ ENV PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/root/go/b
RUN apt-get update \
&& apt-get -y upgrade \
&& apt-get install -y bash golang-1.18 libbtrfs-dev libnl-3-dev libnet1-dev \
protobuf-c-compiler libcap-dev libaio-dev \
protobuf-c-compiler libgcrypt20-dev libcap-dev libaio-dev \
curl libprotobuf-c-dev libprotobuf-dev socat libseccomp-dev \
pigz lsof make git gcc build-essential pkgconf libtool \
libsystemd-dev libcap-dev libyajl-dev \
Expand Down
2 changes: 1 addition & 1 deletion tests/cri-o/Dockerfile
Expand Up @@ -6,7 +6,7 @@ ENV PATH=/usr/bin:/usr/sbin:/root/go/bin:/usr/local/bin::/usr/local/sbin
RUN yum install -y python git gcc automake autoconf libcap-devel \
systemd-devel yajl-devel libseccomp-devel go-md2man conntrack-tools which \
glibc-static python3-libmount libtool make podman xz nmap-ncat jq bats \
iproute openssl iputils socat && \
libgcrypt-devel iproute openssl iputils socat && \
dnf install -y 'dnf-command(builddep)' && dnf builddep -y podman && \
dnf remove -y golang && \
sudo dnf update -y && \
Expand Down
2 changes: 1 addition & 1 deletion tests/fuzzing/Dockerfile
@@ -1,7 +1,7 @@
FROM fedora:latest

RUN yum install -y golang python git automake autoconf libcap-devel \
systemd-devel yajl-devel libseccomp-devel go-md2man \
libgcrypt-devel systemd-devel yajl-devel libseccomp-devel go-md2man \
glibc-static python3-libmount libtool make honggfuzz git

RUN git clone https://github.com/giuseppe/containers-fuzzing-corpus /testcases
Expand Down
2 changes: 1 addition & 1 deletion tests/podman/Dockerfile
Expand Up @@ -4,7 +4,7 @@ ENV GOPATH=/root/go
ENV PATH=/usr/bin:/usr/sbin:/root/go/bin:/usr/local/bin::/usr/local/sbin

RUN dnf install -y golang python git gcc automake autoconf libcap-devel \
systemd-devel yajl-devel libseccomp-devel go-md2man \
systemd-devel yajl-devel libseccomp-devel go-md2man libgcrypt-devel \
glibc-static python3-libmount libtool make podman xz nmap-ncat \
containernetworking-plugins 'dnf-command(builddep)' && \
dnf builddep -y podman && \
Expand Down
1 change: 1 addition & 0 deletions tests/wasmedge-build/Dockerfile
Expand Up @@ -4,6 +4,7 @@ ARG WASM_EDGE_VERSION="0.11.0"
# Install the deps for building crun
RUN dnf update -y && dnf install -y make python git gcc automake autoconf libcap-devel \
systemd-devel yajl-devel libseccomp-devel pkg-config diffutils \
libgcrypt-devel systemd-devel yajl-devel libseccomp-devel pkg-config \
go-md2man glibc-static python3-libmount libtool buildah podman

# Install WasmEdge
Expand Down