openssh bugs - May 2012 - [Bug 2011] New: sandbox selection needs some kind of fallback mechanism

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

             Bug #: 2011
           Summary: sandbox selection needs some kind of fallback
                    mechanism
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 6.0p1
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Build system
        AssignedTo: unassigned-bugs at mindrot.org
        ReportedBy: cjwatson at debian.org


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?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

--- Comment #1 from Colin Watson <cjwatson at debian.org> 2012-05-19
00:45:42 EST ---
Created attachment 2154
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2154
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.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

--- Comment #2 from Colin Watson <cjwatson at debian.org> 2012-05-19
03:40:45 EST ---
Created attachment 2155
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2155
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.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

Kees Cook <kees at outflux.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kees at outflux.net

--- Comment #3 from Kees Cook <kees at outflux.net> 2012-05-19 05:52:01
EST ---
FWIW, this looks good to me. I prefer the idea of this being runtime
detected over configure-time detected.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

--- Comment #4 from Damien Miller <djm at mindrot.org> 2012-06-01 10:40:30
EST ---
Created attachment 2160
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2160
seccomp-fallback.diff

fallback to rlimit in seccomp sandbox

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #5 from Damien Miller <djm at mindrot.org> 2012-06-01 10:42:05
EST ---
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
implements this.

I'm happy to revisit this if we ever have a deeper stack of candidate
sandboxes for a platform.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2011

--- Comment #6 from Darren Tucker <dtucker at zip.com.au> 2012-06-01
10:52:04 EST ---
Comment on attachment 2160
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2160
seccomp-fallback.diff

seems reasonable, although I'd make the verbose() calls into debug3s,
since otherwise it'll spam logs.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.