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

BUG: Adding multiple filters does not work as expected #87

Closed
Jigsaw52 opened this issue Aug 3, 2017 · 5 comments
Closed

BUG: Adding multiple filters does not work as expected #87

Jigsaw52 opened this issue Aug 3, 2017 · 5 comments
Assignees

Comments

Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@Jigsaw52
Copy link

@Jigsaw52 Jigsaw52 commented Aug 3, 2017

When trying to add two filters, the second filter seems to have no effect, even though the call to seccomp_load returns 0.

From the man page of seccomp (http://man7.org/linux/man-pages/man2/seccomp.2.html), I expected both filters to work:

If prctl(2) or seccomp() is allowed by the attached filter,
further filters may be added. This will increase evaluation
time, but allows for further reduction of the attack surface
during execution of a thread.

If multiple filters exist, they are all executed, in reverse order of
their addition to the filter tree—that is, the most recently
installed filter is executed first. (Note that all filters will be
called even if one of the earlier filters returns SECCOMP_RET_KILL.
This is done to simplify the kernel code and to provide a tiny speed-
up in the execution of sets of filters by avoiding a check for this
uncommon case.) The return value for the evaluation of a given
system call is the first-seen SECCOMP_RET_ACTION value of highest
precedence (along with its accompanying data) returned by execution
of all of the filters.

I've created a small program to exemplify the issue:

#include <stdio.h>
#include <linux/seccomp.h>
#include <sys/prctl.h>
#include <sys/file.h>
#include <seccomp.h>
#include <unistd.h>


static int
sc_filter_prctl(scmp_filter_ctx ctx)
{
  int rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 1,
      SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_DUMPABLE));
  if (rc < 0)
    return rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 2,
      SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_NO_NEW_PRIVS),
      SCMP_CMP(1, SCMP_CMP_EQ, 1));
  if (rc < 0)
    return rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 2,
      SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_SECCOMP),
      SCMP_CMP(1, SCMP_CMP_EQ, SECCOMP_MODE_FILTER));
  if (rc < 0)
    return rc;

  return 0;
}


static int
sc_filter_seccomp(scmp_filter_ctx ctx)
{
  int rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(seccomp), 2,
      SCMP_CMP(0, SCMP_CMP_EQ, SECCOMP_SET_MODE_FILTER),
      SCMP_CMP(1, SCMP_CMP_EQ, 0));
  if (rc < 0)
    return rc;

  return 0;
}


static int
sc_filter_stuff(scmp_filter_ctx ctx)
{
  int rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(close), 0);
  if (rc < 0)
    return rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(exit_group), 0);
  if (rc < 0)
    return rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(fstat), 0);
  if (rc < 0)
    return rc;

  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(write), 0);
  if (rc < 0)
    return rc;

  return 0;
}


int
main(int argc, char *argv[])
{
  int rc, fd;
  scmp_filter_ctx ctx;

  const char *file1 = "/tmp/file1";
  const char *file2 = "/tmp/file2";

  const struct scmp_version *version = seccomp_version();
  if (version == NULL)
    goto err;

  printf("seccomp %d.%d.%d\n", version->major, version->minor, version->micro);

  ctx = seccomp_init(SCMP_ACT_TRAP);
  if (ctx == NULL)
    goto err;

  // add generic filters
  if (sc_filter_stuff(ctx))
    goto err;

  if (sc_filter_prctl(ctx))
    goto err;

  if (sc_filter_seccomp(ctx))
    goto err;

  // allow open file1
  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), 1,
          SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t)file1));
  if (rc < 0)
    goto err;

  rc = seccomp_load(ctx);
  if (rc < 0)
    goto err;
  seccomp_release(ctx);

  printf("Opening file1... ");
  fd = open(file1, O_CREAT);
  close(fd);
  printf("OK\n");

  ctx = seccomp_init(SCMP_ACT_TRAP);

  // allow open file2
  rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), 1,
          SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t)file2));
  if (rc < 0)
    goto err;

  rc = seccomp_load(ctx);
  if (rc < 0)
    goto err;
  seccomp_release(ctx);

  printf("Opening file2... ");
  fd = open(file2, O_CREAT);
  close(fd);
  printf("OK\n");

  return 0;

err:
  printf("error setting up seccomp\n");
  return -1;
}

I expected the program to terminate successfully, instead I get the following output:

seccomp 2.3.1
Opening file1... OK
Bad system call (core dumped)

The cause is the syscall to open file2 not being allowed despite the filter being added, as can be seen from the following strace output:

execve("./a.out", ["./a.out"], [/* 53 vars */]) = 0
brk(NULL)                               = 0x561790abc000
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fd9eab5c000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=86874, ...}) = 0
mmap(NULL, 86874, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fd9eab46000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libseccomp.so.2", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\220\30\2\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=280848, ...}) = 0
mmap(NULL, 2375912, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7fd9ea6f5000
mprotect(0x7fd9ea723000, 2097152, PROT_NONE) = 0
mmap(0x7fd9ea923000, 94208, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2e000) = 0x7fd9ea923000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20\5\2\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1856752, ...}) = 0
mmap(NULL, 3959200, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7fd9ea32e000
mprotect(0x7fd9ea4ec000, 2093056, PROT_NONE) = 0
mmap(0x7fd9ea6eb000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1bd000) = 0x7fd9ea6eb000
mmap(0x7fd9ea6f1000, 14752, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fd9ea6f1000
close(3)                                = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fd9eab44000
arch_prctl(ARCH_SET_FS, 0x7fd9eab44700) = 0
mprotect(0x7fd9ea6eb000, 16384, PROT_READ) = 0
mprotect(0x7fd9ea923000, 90112, PROT_READ) = 0
mprotect(0x5617904c3000, 4096, PROT_READ) = 0
mprotect(0x7fd9eab5f000, 4096, PROT_READ) = 0
munmap(0x7fd9eab46000, 86874)           = 0
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 3), ...}) = 0
brk(NULL)                               = 0x561790abc000
brk(0x561790add000)                     = 0x561790add000
write(1, "seccomp 2.3.1\n", 14seccomp 2.3.1
)         = 14
prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)  = 0
seccomp(SECCOMP_SET_MODE_STRICT, 1, NULL) = -1 EINVAL (Invalid argument)
seccomp(SECCOMP_SET_MODE_FILTER, 0, {len=42, filter=0x561790abfba0}) = 0
open("/tmp/file1", O_RDONLY|O_CREAT, 03775475233613540) = 3
close(3)                                = 0
write(1, "Opening file1... OK\n", 20Opening file1... OK
)   = 20
prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)  = 0
seccomp(SECCOMP_SET_MODE_FILTER, 0, {len=12, filter=0x561790abd5f0}) = 0
open("/tmp/file2", O_RDONLY|O_CREAT, 03775475233613540) = 2
--- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x7fd9ea426650, si_syscall=__NR_open, si_arch=AUDIT_ARCH_X86_64} ---
+++ killed by SIGSYS (core dumped) +++
Bad system call (core dumped)
@pcmoore
Copy link
Member

@pcmoore pcmoore commented Aug 3, 2017

Hello.

Are you convinced that the intptr_t cast is not mangling the file2 pointer? Have you tried it without the cast, or using void * as the cast?

// allow open file2
rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open), 1,
                      SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t)file2));

@Jigsaw52
Copy link
Author

@Jigsaw52 Jigsaw52 commented Aug 4, 2017

I haven't tried without the cast or with cast to void* but I'm convinced that is not the cause of the issue. Notice that I use the same cast when adding the filter for file1 and that filter works as expected.

@Yawning
Copy link

@Yawning Yawning commented Aug 4, 2017

I commented on the tor bug, but maybe you didn't see it, or don't believe me.

There is no libseccomp bug. You are trying to do something that does not work. The second filter is getting installed and evaluated. When multiple filters are present, all of them are evaluated, and the highest precedence value is returned. You can tighten restrictions by installing additional filters, but can not relax them.

This is trivially observable, for example by setting the default action to return 2 different errno values (eg: seccomp_init(SCMP_ACT_ERRNO(ENOSYS)) and seccomp_init(SCMP_ACT_ERRNO(EIO)), and stracing the binary.

open("/tmp/file1", O_RDONLY|O_CREAT, 000) = -1 EACCES (Permission denied)
close(-1)                               = -1 EBADF (Bad file descriptor)
write(1, "Opening file1... OK\n", 20Opening file1... OK
)   = 20
prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)  = 0
seccomp(SECCOMP_SET_MODE_FILTER, 0, {len=13, filter=0xf92deee550}) = 0
open("/tmp/file2", O_RDONLY|O_CREAT, 000) = -1 ENOSYS (Function not implemented)
close(-1)                               = -1 EIO (Input/output error)
write(1, "Opening file2... OK\n", 20)   = -1 EIO (Input/output error)
exit_group(0)                           = -1 EIO (Input/output error)
exit(0)                                 = -1 EIO (Input/output error)

The ENOSYS result from open() is from the first filter, as ACT_ERRNO takes precedence over ACT_ALLOW. The EIO results are from the second filter.

@Jigsaw52
Copy link
Author

@Jigsaw52 Jigsaw52 commented Aug 4, 2017

I did not see your comment.

I see my mistake now. I misunderstood the seccomp documentation. I was thinking that the rules of all filters were evaluated and then the highest precedence value of those was returned. But the default value is evaluated too. You're correct. This means that no further allow rules can be added after having a filter with the default other than allow.

@pcmoore
Copy link
Member

@pcmoore pcmoore commented Aug 4, 2017

Thanks @Yawning, I missed the first seccomp_load()/seccomp_init() and thought this was a single filter instance, despite the subject line. I'm going to close this issue, thanks for your help!

@pcmoore pcmoore closed this Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment