Skip to content
New issue

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

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

Already on GitHub? # to your account

Add seccomp support for sandbox #274

Open
wants to merge 1 commit into
base: sandbox
Choose a base branch
from
Open

Add seccomp support for sandbox #274

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 2, 2018

No description provided.

@ghost ghost mentioned this pull request Jan 2, 2018
@mptre
Copy link
Owner

mptre commented Jan 2, 2018

Great work! Attached is a diff which reworks the macros and some cleanup:

diff --git compat-sandbox.c compat-sandbox.c
index 3f63556..2645f23 100644
--- compat-sandbox.c
+++ compat-sandbox.c
@@ -36,49 +36,45 @@ sandbox(int stage)
 #include <err.h>
 #include <seccomp.h>
 
-#define ALLOW(syscall)								\
-	if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(syscall), 0) < 0) {	\
-		err(1, "seccomp_rule_add");					\
-	}
+#define ALLOW(syscall)							\
+	(seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(syscall), 0) < 0)
 
-#define ALLOW_IOCTL(syscall, x)							\
-	if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(ioctl), x,		\
-	    SCMP_A1(SCMP_CMP_EQ, syscall)) < 0) {				\
-		err(1, "seccomp_rule_add (ioctl)");				\
-	}
+#define ALLOW_IOCTL(syscall, x)	\
+	(seccomp_rule_add(ctx, SCMP_ACT_ALLOW,SCMP_SYS(ioctl), x,	\
+	 SCMP_A1(SCMP_CMP_EQ, syscall)) < 0)
 
 void
 sandbox(int stage)
 {
-	scmp_filter_ctx		ctx;
+	scmp_filter_ctx	ctx;
 
 	switch (stage) {
 	case SANDBOX_ENTER:
-
 		if ((ctx = seccomp_init(SCMP_ACT_TRAP)) == NULL)
 			err(1, "seccomp_init");
 
-		ALLOW(access);
-		ALLOW(close);
-		ALLOW(exit_group);
-		ALLOW(fstat);
-		ALLOW(fstat64);
-		ALLOW(mmap);
-		ALLOW(mmap2);
-		ALLOW(munmap);
-		ALLOW(open);
-		ALLOW(poll);
-		ALLOW(read);
-		ALLOW(rt_sigaction);
-		ALLOW(sigaction);
-		ALLOW(sigreturn);
-		ALLOW(stat);
-		ALLOW(stat64);
-		ALLOW(time);
-		ALLOW(write);
-		ALLOW_IOCTL(TCGETS, 1);
-		ALLOW_IOCTL(TCSETS, 1);
-		ALLOW_IOCTL(TIOCGWINSZ, 1);
+		if (ALLOW(access) ||
+		    ALLOW(close) ||
+		    ALLOW(exit_group) ||
+		    ALLOW(fstat) ||
+		    ALLOW(fstat64) ||
+		    ALLOW(mmap) ||
+		    ALLOW(mmap2) ||
+		    ALLOW(munmap) ||
+		    ALLOW(open) ||
+		    ALLOW(poll) ||
+		    ALLOW(read) ||
+		    ALLOW(rt_sigaction) ||
+		    ALLOW(sigaction) ||
+		    ALLOW(sigreturn) ||
+		    ALLOW(stat) ||
+		    ALLOW(stat64) ||
+		    ALLOW(time) ||
+		    ALLOW(write) ||
+		    ALLOW_IOCTL(TCGETS, 1) ||
+		    ALLOW_IOCTL(TCSETS, 1) ||
+		    ALLOW_IOCTL(TIOCGWINSZ, 1))
+			err(1, "seccomp_rule_add");
 
 		if (seccomp_load(ctx) < 0)
 			err(1, "seccomp_load");

@mptre
Copy link
Owner

mptre commented Jan 2, 2018

I guess libseccomp-dev is required be installed on Travis in order to
trigger the autoconf check? You could try to install it globally in
.travis.yml. Caution, lines below untested:

addons:
  apt:
    packages:
      - libseccomp-dev

@ghost
Copy link
Author

ghost commented Jan 2, 2018

Thanks: applied!

@mptre mptre force-pushed the sandbox branch 12 times, most recently from cebf2cd to 321e6e7 Compare January 6, 2018 11:05
@ghost ghost closed this Jan 7, 2018
@ghost ghost reopened this Jan 7, 2018
@ghost
Copy link
Author

ghost commented Jan 14, 2018

The experimental seccomp-support can be enabled with --enable-seccomp ...

@codecov-io
Copy link

codecov-io commented Jan 14, 2018

Codecov Report

Merging #274 into sandbox will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           sandbox     #274   +/-   ##
========================================
  Coverage    90.58%   90.58%           
========================================
  Files            1        1           
  Lines          510      510           
========================================
  Hits           462      462           
  Misses          48       48

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 321e6e7...9f21d79. Read the comment docs.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants