Diving deeper into the issue, with ext4 crypto, we may lose the ability to prevent minijailed processes to see home directories:

With ext4, using -v (and -k), we could exclude the ecryptfs mounts and prevent the process from seeing user data.

With ext4 crypto, there is no mount boundary anymore, we need another method.
Given init is creating a keyring to store the key needed to decrypt the home directory, the idea was to remove that keyring from the process before launching it.

However, as proven by c/433098, it is easy for a root process to join the encryption keyring and gain access to the user directory again.

Do we have a way to prevent keyctl IOCTL to work for certain processes, an extension to no_new_perm?
 

Can we disable the IOCTL entirely for the jailed process?

If yes, can we add a seccomp filter.

We can use seccomp for that, yes.

Agreed that a seccomp filter is the right solution to this problem.  What's the right way to implement this though?  One way is to find all the daemons that run as root and also use a mount namespace and create seccomp policy files for them.  From my basic search it looks like the only things that meet this criteria are conntrackd and sslh: https://cs.corp.google.com/search/?q=exec%5C+minijail0.*%5Cn*.*%5C+-(v%7Cp)+file:.*%5C.conf+pcre:yes&type=cs

conntrackd already has a seccomp policy so it should be an easy change there but it doesn't look like sslh has one.  However it's hard to know if I've found all the callers especially when some of them might use the library interface to minijail instead of minijail0.

Is there some way to piggyback off the mount namespace option so that if a program uses a mount namespace we also block the keyctl syscalls for it?  My concern is having to create an entire seccomp policy just to block one syscall for those programs that don't already have a seccomp policy.

Actually after testing a bit more, it looks like conntrackd should be fine since it runs as the nfqueue user and the seccomp policy doesn't say anything about keyctl (which means it's blocked by default).  


So the only program that's still an issue is sslh.

What's sslh?

Minijail doesn't have a way of specifying a blacklist of syscalls, but this limitation has come up in the past so we could finally address it.

sslh is an "applicative protocol multiplexer" (http://www.rutschle.net/tech/sslh.shtml).  Looks like we use it to multiplex ssh and adb connections.

Seems like it gets pulled in for cheets builds:

(cr) chirantan@endor ~/trunk/src/scripts (git-svn)-[150cdf8...] % equery-cave b /usr/sbin/sslh-fork     
 * Searching for /usr/sbin/sslh-fork ...
net-misc/sslh-1.17 (/usr/sbin/sslh-fork)
equery-cave b /usr/sbin/sslh-fork  7.77s user 0.25s system 99% cpu 8.029 total

(cr) chirantan@endor ~/trunk/src/scripts (git-svn)-[150cdf8...] % equery-cave w net-misc/sslh
/mnt/host/source/src/third_party/portage-stable/net-misc/sslh/sslh-1.17.ebuild

(cr) chirantan@endor ~/trunk/src/scripts (git-svn)-[150cdf8...] % equery-cave d -D sslh
 * These packages depend on sslh:
chromeos-base/chromeos-cheets-scripts-0.0.1-r259 (net-misc/sslh)
 chromeos-base/chromeos-cheets-0.0.1-r5 (chromeos-base/chromeos-cheets-scripts)
  virtual/target-chrome-os-1-r12 (cheets ? chromeos-base/chromeos-cheets)
   virtual/target-os-1.5-r1 (virtual/target-chrome-os)

+cernekee for sslh backstory if needed.

sslh doesn't actually need to run in a separate mount namespace.  It was a hardening measure.  I think vapier just kept tightening as many screws as possible, until something broke. ;-)

We have to run adb on 22/tcp because that is the only open port from the corp->lab networks, so we muxed it with ssh.

It wouldn't be a bad idea to create a seccomp policy for sslh, but in general it seems like it would be nice to have a way to blacklist keyctl instead of whitelisting "everything except keyctl."

cernekee@: It sounds like we're only using it to support the developer use case.  Is it used for something in release images?  Otherwise can we move it under target-os-dev instead?

AIUI, release images still support inbound adb + ssh connections:

https://developer.android.com/topic/arc/sideload.html

:-/

Ok.  So here's my plan for dealing with this short term:

- Create a new flag in minijail for dropping the session keyring.  Add this flag to the invocation for sslh.
- Do the silly thing and create a seccomp policy for sslh that whitelists _all_ syscalls except keyctl.

Longer term:
- Add blacklist support to minijail.  I can probably do this if no one else has time for it but it'll take me some time to come up to speed with seccomp and minijail to figure out how to do it properly.
- Once we have blacklist support in minijail, switch to the blacklist for sslh.

Does anyone have any concerns with this plan?

I'm OK with that, but can we restrict the syscall whitelist to what sslh actually needs?

I'm OK with that plan too.

Also filed b/34819648 to track the blacklist work.

The issue with restricting the syscall whitelist for sslh is that it forks to start sshd and adb and I think the syscall filters carry over.

So the list of syscalls that sslh might make is basically the list of syscalls that adb  could make (as well as a logged in ssh user on dev images).

I don't think sslh actually execs sshd or adbd.  It only needs to connect to the existing ssh or adb service that is already listening on a different socket.

Ah, ok.  In that case I can try to actually get a proper policy for it.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/9b70abf6877b81d97ffe62d8c19c9a288dad0968

commit 9b70abf6877b81d97ffe62d8c19c9a288dad0968
Author: Chirantan Ekbote <chirantan@chromium.org>
Date: Sat Mar 04 03:15:58 2017

Uprev chromeos-minijail

Changes:
8c13d10 Fix broken unit tests
866bb3a Add a flag to drop access to the session keyring
959f656 Fix help message, man page.
ab9eb44 allow specifying larger /tmp tmpfs mounts

BUG=chromium:682419
TEST=autotest in CL:439428

Change-Id: Ib080b64f09456172306e87a32685b019080f2730
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/440054
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[rename] https://crrev.com/9b70abf6877b81d97ffe62d8c19c9a288dad0968/chromeos-base/chromeos-minijail/chromeos-minijail-0.0.1-r1476.ebuild

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/778d6f267d85f8d9ae45c76f8770d71661eac053

commit 778d6f267d85f8d9ae45c76f8770d71661eac053
Author: Chirantan Ekbote <chirantan@chromium.org>
Date: Tue Mar 07 00:28:33 2017

autotest-tests-security: Add dependency on keyutils

The security_Minijail0 autotest has a runtime dependency on keyutils.  Add it as
an explicit dependency for the test so it's always pulled in.

BUG=chromium:682419
TEST=cbuildbot --remote lakitu-paladin

Change-Id: Iabe91e3cf36857b854cd82bc5ff814112540846f
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/442087
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/778d6f267d85f8d9ae45c76f8770d71661eac053/chromeos-base/autotest-tests-security/autotest-tests-security-9999.ebuild

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/42a2b98245faf3c17eb85047a3b25378afcb56fc

commit 42a2b98245faf3c17eb85047a3b25378afcb56fc
Author: Chirantan Ekbote <chirantan@chromium.org>
Date: Tue Mar 07 10:32:41 2017

security_Minijail0: Add test for session keyring

Test that creating a new anonymous session keyring works as expected.

BUG=chromium:682419
TEST=this CL
CQ-DEPEND=CL:440054,CL:442087

Change-Id: I721b98ea4cade30076da944199b87a24ca852c89
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/439428
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[add] https://crrev.com/42a2b98245faf3c17eb85047a3b25378afcb56fc/client/site_tests/security_Minijail0/src/test-session-keyring

Time to mark this fixed?

The autotest change ended up getting reverted.  I'll try again to get it landed this week and then I'll close this bug.

do you want to cherry pick to M-58 or can we live with it.

I think we can live with it on M-59.

I understand this is done.

We probably still want to land the test change described in c#13 (https://chromium-review.googlesource.com/c/453978/).

hmmm. what's happened to #31 ?

That test CL depends on a command-line utility. Even after an ebuild change, we were not able to get that command-line utility consistently installed on test devices, and the new test kept failing.

sslh-seccomp.policy broke ssh over IPV6, see https://chrome-internal-review.googlesource.com/c/chromeos/overlays/project-cheets-private/+/326413#message-659e8880eaf163b3f5658389d6694766bf18b060

Filed new  issue 772273  sslh seccomp policy blocks ssh to ChromeOS over IPv6