BUG: ensure normal processing of syscall -1 on x86_64/x32

[hallyn]

When libseccomp generates bpf, it adds a check to make sure that X86_SYSCALL_BIT is not set, bypassing compat checks. It does this by simply checking whether nr >= X86_SYSCALL_BIT. Unfortunately, this catches nr = (u32)-1. -1 is a nop, and as far as I know cannot be abused.

As the seccomp manpage points out, after seeing a SECCOMP_RET_TRACE, a tracer can set nr to -1 to skip the syscall. Similarly, one task could be debugging another seccomp'd task, simply doing PTRACE_SYSCALL without using SECCOMP_PTRACE, and want to make the tracee skip a syscall by setting nr to -1.

Since libseccomp is used by default by all lxc and lxd containers, any such tracing application do not work in containers right now.

Please update the bpf generator to allow syscall_nr == -1.

[pcmoore]

The relevant portion of the seccomp(2) man-page:

   SECCOMP_RET_TRACE
          When returned, this value will cause the kernel to attempt to
          notify a ptrace(2)-based tracer prior to executing the system
          call.  If there is no tracer present, the system call is not
          executed and returns a failure status with errno set to
          ENOSYS.

          A tracer will be notified if it requests PTRACE_O_TRACESECCOMP
          using ptrace(PTRACE_SETOPTIONS).  The tracer will be notified
          of a PTRACE_EVENT_SECCOMP and the SECCOMP_RET_DATA portion of
          the filter's return value will be available to the tracer via
          PTRACE_GETEVENTMSG.

          The tracer can skip the system call by changing the system
          call number to -1.  Alternatively, the tracer can change the
          system call requested by changing the system call to a valid
          system call number.  If the tracer asks to skip the system
          call, then the system call will appear to return the value
          that the tracer puts in the return value register.

          Before kernel 4.8, the seccomp check will not be run again
          after the tracer is notified.  (This means that, on older
          kernels, seccomp-based sandboxes must not allow use of
          ptrace(2)—even of other sandboxed processes—without extreme
          care; ptracers can use this mechanism to escape from the
          seccomp sandbox.)

[pcmoore]

It turns out that libseccomp will not let you create a filter rule using a syscall value of -1 at the moment so the risk of someone accidentally running into this problem should be zero. However, the greater problem of not handling syscall -1 correctly still stands.

[pcmoore]

The more time I spend with this, the messier it gets. Perhaps the largest problem is that __NR_SCMP_ERROR has a value of -1 in the libseccomp v2.x series which means that we need to allow a syscall value that has always been used as an error sentinel. Allowing rules with a -1 syscall value is likely going to require some additional work (think attribute setting) on the caller's end so we don't break the API for everyone.

[pcmoore]

@hallyn Any chance you can take a look at the master branch of https://github.com/pcmoore/misc-libseccomp and see if it is acceptable?

[hallyn]

I'll take a look this weekend, thanks.

[pcmoore]

@hallyn I'm mainly looking for feedback on the API, specifically the SCMP_FLTATR_CTL_TSKIP attribute requirement. You can ignore the guts of the change if you want, although any review is always welcome ;)

[hallyn]

Tried out the patch and it did work for me, thanks.

My imagination is failing me - what does SCMP_FLTATR_CTL_TSKIP stand for? :-)

What is the threat against which you are defending by making this off by default? (again, failing imagination)

[pcmoore]

Tried out the patch and it did work for me, thanks.

Thanks.

My imagination is failing me - what does SCMP_FLTATR_CTL_TSKIP stand for? :-)

The thinking was that TSKIP was a shortened version of "Tracer SKIP"; I also thought about "Negative ONE" but that didn't work for obvious reasons. If you've got a suggestion, I'd love to hear it. I'm definitely not loving TSKIP, but I'm failing to think of anything better.

What is the threat against which you are defending by making this off by default? (again, failing imagination)

The problem is that historically -1 has been a protected value due to __NR_SCMP_ERROR and seccomp_rule_add(...) specifically checks for this value (see the "RETURN VALUE" section in the seccomp_syscall_resolve_name(3) man-page). If I changed seccomp_rule_add(...) to allow -1 as a syscall value I would potentially break a number of applications which make use of this functionality.

[hallyn]

Quoting Paul Moore (notifications@github.com):

> Tried out the patch and it did work for me, thanks. Thanks. > My imagination is failing me - what does SCMP_FLTATR_CTL_TSKIP stand for? :-) The thinking was that TSKIP was a shortened version of "Tracer SKIP"; I also thought about "Negative ONE" but that didn't work for obvious reasons. If you've got a suggestion, I'd love to hear it. I'm definitely not loving TSKIP, but I'm failing to think of anything better.

Nah, TSKIP makes sense :)

> What is the threat against which you are defending by making this off by default? (again, failing imagination) The problem is that historically -1 has been a protected value due to __NR_SCMP_ERROR and seccomp_rule_add(...) specifically checks for this value (see the "RETURN VALUE" section in the seccomp_syscall_resolve_name(3) man-page). If I changed seccomp_rule_add(...) to allow -1 as a syscall value I would potentially break a number of applications which make use of this functionality.

Hm. But the -1 is ambiguous only for passing to rules right? If it is deemed "always ok" to allow syscall -1, then we can unambiguously state that a. -1 at seccomp_rule_add() means __NR_SCMP_ERROR b. -1 as a syscall is ignored (allowed) Basically my possibly false assertion is that syscall -1 can never put the system at risk and can safely be always allowed, so admins would never try to say "don't allow syscall -1" in a policy.

[pcmoore]

But the -1 is ambiguous only for passing to rules right? If it is deemed "always ok" to allow syscall -1, then we can unambiguously state that a. -1 at seccomp_rule_add() means __NR_SCMP_ERROR b. -1 as a syscall is ignored (allowed) Basically my possibly false assertion is that syscall -1 can never put the system at risk and can safely be always allowed, so admins would never try to say "don't allow syscall -1" in a policy.

My concern is that we have never implicitly added a rule (other than the ABI checks) to the libseccomp filters and I'm not sure this is something I want to start doing at this moment. While small, I don't think the risk is close enough to zero to warrant adding a default -1 rule, and there is always the chance that some application will want to do something clever with a seccomp filter and the -1 syscall and I don't want to prevent that.

However, this did get me thinking about changing the name of the CTL_TSKIP attribute to API_TSKIP to leave the door open for a potential future attribute which would create an implicit -1 allow rule. I think I'll do that now, just in case (arguably it makes more sense anyway).

Thoughts @hallyn ?

[pcmoore]

I just committed the fixes in dc87999 11e2109 ba73ee4 and I'm going to mark this as resolved for now.

[hallyn]

Great, thanks!