Created attachment 2328 [details] libseccomp patch Hi I've been playing with libseccomp and i think that it abstracts a lot of the low-level BPF stuff by providing a simpler, easier & portable API. The patch is based off will drewry's seccomp patch. I tested it on my ubuntu box. It's still a WiP.
Comment on attachment 2328 [details] libseccomp patch I don't think this is an improvement: the code isn't much shorter, only a little more readable and we have to use an external library.
So libseccomp would be "untrusted", similar to kerberos ? libseccomp has seen steady progress, and I think that it would be nice if openssh takes advantage of it if it is deployed on a fairly recent linux system. http://www.paul-moore.com/files/lj/libseccomp-pmoore-lss2012-r1.pdf Please see page 3 :-) Capsicum is also working towards a similar approach with libcapsicum & libangel. (https://code.google.com/p/capsicum-core/) I'm not suggesting replacing will's seccomp patch, but rather provide it as an additional build time option that package maintainers can take advantage of if libseccomp is present.
Sure, but I don't see the point - what's the advantage to using libseccomp? It looks like it might have some advantages if we were doing argument inspection, were scared of writing BPF or running a complex policy but we aren't. The existing seccomp sandbox will work on any system that has libseccomp and will do the same thing with fewer dependencies and less code. Adding another sandbox that does exactly the same thing just means we need to maintain two sets of code instead of one.
(In reply to Damien Miller from comment #3) > Sure, but I don't see the point - what's the advantage to using > libseccomp? It looks like it might have some advantages if we were > doing argument inspection, were scared of writing BPF or running a > complex policy but we aren't. Agreed. > The existing seccomp sandbox will work on any system that has > libseccomp and will do the same thing with fewer dependencies and > less code. Adding another sandbox that does exactly the same thing > just means we need to maintain two sets of code instead of one. I see your point ("Reduced attack surface") :-) In that case, it's probably better that i don't spend more time further on this. Thanks.
It's not so much a question of attack surface, just the amount of code that needs to be maintained. I think if the sandbox were ever to grow more complicated then libseccomp might be worth investigating, but we'll pass for now.
I'd like to reopen this because there's now a reason to implement this change. A build of portable OpenSSH with the x32 ABI (gcc -mx32) on x86_64 doesn't work correctly with the seccomp_filter sandbox. With libseccomp I'm able to do seccomp_arch_add for SCMP_ARCH_X86_64 and SCMP_ARCH_X32 -- which is sufficient to unbreak things. I'm attaching an updated patch which is a bit smaller and cleaner than the previous version, and contains an array of syscall rules similar to the one in sandbox-seccomp-filter.c. This reduces code size by a fair amount.
Created attachment 2563 [details] libseccomp patch v2
Comment on attachment 2563 [details] libseccomp patch v2 >+static int >+seccomp_add_secondary_archs(scmp_filter_ctx *c) >+{ >+#if defined(__i386__) || defined(__x86_64__) >+ int r; >+ r = seccomp_arch_add(c, SCMP_ARCH_X86); >+ if (r < 0 && r != -EEXIST) >+ return r; >+ r = seccomp_arch_add(c, SCMP_ARCH_X86_64); >+ if (r < 0 && r != -EEXIST) >+ return r; >+ r = seccomp_arch_add(c, SCMP_ARCH_X32); >+ if (r < 0 && r != -EEXIST) >+ return r; >+#endif >+ return 0; >+} i don't think this is correct. there's no reason to permit alternative ABIs from the one you're currently executing as. x86/32bit should only permit the X86 ABI, x86_64/64bit should only permit the X86_64 ABI, and x86_64/32bit should only permit the X32 ABI.
In principle, I agree. But I wasn't able to get it to work without that. :(
Created attachment 2927 [details] Work around clock_gettime kernel bug on Linux x32 Here's an alternative patch that fixes the seccomp sandbox on Linux x32. It's working around what I consider to be a kernel bug (reported in Debian and we'll see where it goes), so I don't know what you'll make of this, but it's reasonably unobtrusive as workarounds go.
Created attachment 2962 [details] updated diff I've refactored that file a bit to make the manual expansion of SC_ALLOW() unnecessary here. Here's an updated and simpler diff that just allows the clock_gettime syscall with the X32 bit masked off.
Patch is applied and the refactoring of that file will make it easier to permit other syscalls with the X32 bit masked off in future if necessary. This will be in the OpenSSH 7.5 release, due very soon.
closing resolved bugs as of 8.6p1 release