Skip to content

Commit

Permalink
configure: Add a flag to disable seccomp.
Browse files Browse the repository at this point in the history 
This is needed for new arches where libseccomp support doesn't exist
yet.

Fixes #1878.
  • Loading branch information
@shlevy
shlevy committed Feb 18, 2018
1 parent 3a5a241 commit 690ac7c
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
1 change: 1 addition & 0 deletions Makefile.config.in
View file
Expand Up @@ -7,6 +7,7 @@ ENABLE_S3 = @ENABLE_S3@
HAVE_SODIUM = @HAVE_SODIUM@
HAVE_READLINE = @HAVE_READLINE@
HAVE_BROTLI = @HAVE_BROTLI@
HAVE_SECCOMP = @HAVE_SECCOMP@
LIBCURL_LIBS = @LIBCURL_LIBS@
OPENSSL_LIBS = @OPENSSL_LIBS@
PACKAGE_NAME = @PACKAGE_NAME@
Expand Down
16 changes: 14 additions & 2 deletions configure.ac
View file
Expand Up @@ -186,9 +186,21 @@ AC_SUBST(HAVE_BROTLI, [$have_brotli])

# Look for libseccomp, required for Linux sandboxing.
if test "$sys_name" = linux; then
PKG_CHECK_MODULES([LIBSECCOMP], [libseccomp],
[CXXFLAGS="$LIBSECCOMP_CFLAGS $CXXFLAGS"])
AC_ARG_ENABLE([seccomp-sandboxing],
AC_HELP_STRING([--disable-seccomp-sandboxing],
[Don't build support for seccomp sandboxing (only recommended if your arch doesn't support libseccomp yet!)]
))
if test "x$enable_seccomp_sandboxing" != "xno"; then
PKG_CHECK_MODULES([LIBSECCOMP], [libseccomp],
[CXXFLAGS="$LIBSECCOMP_CFLAGS $CXXFLAGS"])
have_seccomp=1
else
have_seccomp=
fi
else
have_seccomp=
fi
AC_SUBST(HAVE_SECCOMP, [$have_seccomp])


# Look for aws-cpp-sdk-s3.
Expand Down
4 changes: 3 additions & 1 deletion src/libstore/build.cc
View file
Expand Up @@ -49,7 +49,9 @@
#include <sys/param.h>
#include <sys/mount.h>
#include <sys/syscall.h>
#if HAVE_SECCOMP
#include <seccomp.h>
#endif
#define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old))
#endif

Expand Down Expand Up @@ -2469,7 +2471,7 @@ void DerivationGoal::chownToBuilder(const Path & path)

void setupSeccomp()
{
#if __linux__
#if __linux__ && HAVE_SECCOMP
if (!settings.filterSyscalls) return;

scmp_filter_ctx ctx;
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/local.mk
View file
Expand Up @@ -25,7 +25,7 @@ ifeq ($(OS), SunOS)
libstore_LDFLAGS += -lsocket
endif

ifeq ($(OS), Linux)
ifeq ($(HAVE_SECCOMP), 1)
libstore_LDFLAGS += -lseccomp
endif

Expand Down

7 comments on commit 690ac7c

@edolstra
Copy link
Member

Choose a reason for hiding this comment

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

Disabling seccomp has significant security implications. It's really not optional on Linux.

@edolstra
Copy link
Member

Choose a reason for hiding this comment

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

This broke the setuid tests: https://hydra.nixos.org/eval/1434204

@shlevy
Copy link
Member Author

@shlevy shlevy commented on 690ac7c Feb 19, 2018

Choose a reason for hiding this comment

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

@edolstra I'll take a look at the tests in a bit, but re your first comment what do you recommend people on arches without seccomp do? And if it's not optional why is there a setting to disable it at runtime, and why do we allow nix to build on systems that don't have seccomp at all or something similar?

@edolstra
Copy link
Member

Choose a reason for hiding this comment

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

The motivation for the setting is here: 1dd29d7

Multi-user Nix is insecure on any platform where we can't prevent the creation of setuid/setgid binaries. There is a runtime check for that in build.cc.

@shlevy
Copy link
Member Author

@shlevy shlevy commented on 690ac7c Feb 19, 2018

Choose a reason for hiding this comment

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

@edolstra Should we just extend that runtime check to fail with seccomp is disabled then? Where is the check?

@edolstra
Copy link
Member

Choose a reason for hiding this comment

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

The check is

        /* Don't know how to block the creation of setuid/setgid
           binaries on this platform. */
        throw Error("build users are not supported on this platform for security reasons");

However, it seems better to change setupSeccomp() to:

#if __linux__
    if (!settings.filterSyscalls) return;

#if HAVE_SECCOMP
    ....
#else
    throw Error("seccomp is not supported on this platform");
#endif
#endif

So multi-user builds on RISC-V would fail unless filterSyscalls is explicitly set to false.

@shlevy
Copy link
Member Author

@shlevy shlevy commented on 690ac7c Feb 19, 2018

Choose a reason for hiding this comment

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

#1882

Please sign in to comment.