Bug 2011 - sandbox selection needs some kind of fallback mechanism
Summary: sandbox selection needs some kind of fallback mechanism
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Build system (show other bugs)
Version: 6.0p1
Hardware: All All
: P2 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_6_1
  Show dependency treegraph
 
Reported: 2012-05-18 21:53 AEST by Colin Watson
Modified: 2015-08-11 23:02 AEST (History)
2 users (show)

See Also:


Attachments
strawman patch for sandbox fallback (19.77 KB, patch)
2012-05-19 00:45 AEST, Colin Watson
no flags Details | Diff
fixed strawman patch (19.66 KB, patch)
2012-05-19 03:40 AEST, Colin Watson
no flags Details | Diff
seccomp-fallback.diff (1.33 KB, patch)
2012-06-01 10:40 AEST, Damien Miller
no flags Details | Diff
don't probe seccomp capability of running kernel in configure (832 bytes, patch)
2013-02-05 22:35 AEDT, Petr Lautrbach
no flags Details | Diff
don't probe seccomp capability of running kernel in configure (446 bytes, patch)
2013-02-06 20:51 AEDT, Petr Lautrbach
no flags Details | Diff
don't probe seccomp capability of running kernel in configure (659 bytes, patch)
2013-02-06 23:35 AEDT, Petr Lautrbach
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Watson 2012-05-18 21:53:51 AEST
At the moment, sandbox selection is done entirely at configure time, and you get to pick exactly one.  The seccomp_filter sandbox, at least, probes the capabilities of the running kernel in configure.

This sort of approach is straightforward, but doesn't work well for those of us distributing OpenSSH binaries that are likely to be run on a different kernel version than is running on our build systems.  For example, my current setup for uploading to Debian is such that 32-bit x86 binaries are built in a Debian unstable chroot on my laptop, which runs a 64-bit Ubuntu kernel.  Since Ubuntu's kernel packagers have pulled in the seccomp patches from Will Drewry, but as far as I know the Debian kernel hasn't, and the patches haven't quite landed in an upstream Linux release yet, this means that by default if I build OpenSSH 6.0p1 I'll end up with binaries that will fatal() on Debian kernels, which would probably not make my users terribly happy.

Ideally, what I'd like to be able to do is build binaries that will try to use seccomp_filter if the running kernel (checked at run-time, *not* at build-time) supports it.  That way I could build binaries that will, say, use the rlimit sandbox on Linux <3.5 and the seccomp_filter sandbox on Linux >=3.5 (or whatever version it actually lands in).  Otherwise I won't be able to enable seccomp_filter until I can be absolutely sure that the binaries I'm building will never be used on kernels that predate support for it; bearing in mind that sshd is typically restarted on system upgrades long before rebooting into the upgraded kernel, that's going to be years away.

I realise this is a problem specific to distributors, and even at that I'm sure some distributors have the luxury of being able to assume that kernel and userspace are in sync; so I'd be happy to try to put together a patch for this, given a bit of direction.  Would you prefer a simple approach that just explicitly hands off from seccomp_filter to rlimit until something more complex is needed, or would you prefer something like:

  struct Sandbox *sandboxes = {
#ifdef SANDBOX_SYSTRACE
    sandbox_systrace,
#endif
...
    sandbox_null
  }

and then something that iterates over all the compiled-in sandboxes and picks the first one whose init succeeds?
Comment 1 Colin Watson 2012-05-19 00:45:42 AEST
Created attachment 2154 [details]
strawman patch for sandbox fallback

Perhaps something along these general lines?  I haven't quite got seccomp_filter working for me with this patch yet; the probing subprocess gets SIGSYS rather than doing anything more useful.  However, that might be something to do with running 32-bit userspace on a 64-bit kernel, and it does at least manage to fall back to the rlimit sandbox.

I'd welcome comments on the general approach, anyway.
Comment 2 Colin Watson 2012-05-19 03:40:45 AEST
Created attachment 2155 [details]
fixed strawman patch

Kees Cook set me straight; I was configuring with the wrong --build so it was getting killed by the architecture check (due to my 32-on-64 setup).  This version actually works for me.
Comment 3 Kees Cook 2012-05-19 05:52:01 AEST
FWIW, this looks good to me. I prefer the idea of this being runtime detected over configure-time detected.
Comment 4 Damien Miller 2012-06-01 10:40:30 AEST
Created attachment 2160 [details]
seccomp-fallback.diff

fallback to rlimit in seccomp sandbox
Comment 5 Damien Miller 2012-06-01 10:42:05 AEST
I think the proposed patch is a little over-complicated. The only viable fallback path at the moment is to the rlimit pseudo-sandbox, so let's allow that without fatal() for the seccomp case. Attachment #2160 [details] implements this.

I'm happy to revisit this if we ever have a deeper stack of candidate sandboxes for a platform.
Comment 6 Darren Tucker 2012-06-01 10:52:04 AEST
Comment on attachment 2160 [details]
seccomp-fallback.diff

seems reasonable, although I'd make the verbose() calls into debug3s, since otherwise it'll spam logs.
Comment 7 Damien Miller 2012-07-06 10:32:11 AEST
I just committed my specific fallback for seccomp. I'll be happy to revisit this and adopt a more general fallback mechanism once we have more than one consumer for it.
Comment 8 Petr Lautrbach 2013-02-05 22:35:55 AEDT
Created attachment 2214 [details]
don't probe seccomp capability of running kernel in configure

I'd like to add also possibility to build seccomp_filter sandbox on system with older kernel, E.g. Fedora build systems use buildroots with needed sources and headers, but system is run with older kernels.
Comment 9 Damien Miller 2013-02-06 07:14:03 AEDT
Comment on attachment 2214 [details]
don't probe seccomp capability of running kernel in configure

Maybe it should just s/AC_LINK_IFELSE/AC_RUN_IFELSE/ here?
Comment 10 Petr Lautrbach 2013-02-06 20:51:55 AEDT
Created attachment 2215 [details]
don't probe seccomp capability of running kernel in configure

You are right, s/AC_RUN_IFELSE/AC_LINK_IFELSE/ is sufficient.
Comment 11 Colin Watson 2013-02-06 21:24:41 AEDT
Comment on attachment 2215 [details]
don't probe seccomp capability of running kernel in configure

If you're switching to AC_LINK_IFELSE, you should remove the fourth argument to the macro which controls behaviour when cross-compiling.
Comment 12 Petr Lautrbach 2013-02-06 23:35:32 AEDT
Created attachment 2216 [details]
don't probe seccomp capability of running kernel in configure
Comment 13 Damien Miller 2013-02-07 10:11:48 AEDT
applied - thanks. This will be in openssh-6.2, hopefully due around the end of the month.
Comment 14 Damien Miller 2015-08-11 23:02:30 AEST
Set all RESOLVED bugs to CLOSED with release of OpenSSH 7.1