woah I thought that bind-mounts cloned the flags of the original directory: https://elixir.bootlin.com/linux/v4.4/source/fs/namespace.c#L988

Apparently that's not the case at all, even when the original source path is a mountpoint itself :S

# mount | grep /run | head -n 1
run on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,seclabel,mode=755)
# minijail0 --profile=minimalistic-mountns -b /run -- /bin/bash -c mount | grep /run
run on /run type tmpfs (ro,relatime,seclabel,mode=755)

Fortunately amending the flags after the original mount seems to work as expected, and we already have a bit of code that does that for the bind+ro case[1]:

# minijail0 --profile=minimalistic-mountns -k tmpfs,/run,tmpfs -b /run/dbus -k 'bind,/run/dbus,bind,MS_REMOUNT|MS_NOEXEC|MS_NOSUID|MS_NODEV' -- /bin/bash -c mount | grep /run/dbus
run on /run/dbus type tmpfs (rw,nosuid,nodev,noexec,relatime,seclabel,mode=755)
# minijail0 --profile=minimalistic-mountns -k tmpfs,/run,tmpfs -b /run/dbus -k 'bind,/run/dbus,bind,MS_REMOUNT|MS_NOEXEC|MS_NOSUID|MS_NODEV' -- /bin/bash -c /run/dbus/test.sh     
/bin/bash: /run/dbus/test.sh: Permission denied

But note that this is not limited to minijail, since the kernel just plain ignores any other flags that are passed together with MS_BIND (except MS_REC): https://elixir.bootlin.com/linux/v4.4/source/fs/namespace.c#L2719

1: https://android.googlesource.com/platform/external/minijail/+/master/libminijail.c#1404

Wait, my understanding was correct:

# minijail0 --profile=minimalistic-mountns -k 'tmpfs,/run,tmpfs,MS_NODEV|MS_NOEXEC|MS_NOSUID' -b /run/dbus,,1 -- /bin/bash -c mount | grep /run/dbus
run on /run/dbus type tmpfs (rw,nosuid,nodev,noexec,relatime,seclabel,mode=755)
# minijail0 --profile=minimalistic-mountns -k 'tmpfs,/run,tmpfs,MS_NODEV|MS_NOEXEC|MS_NOSUID' -b /run/dbus -- /bin/bash -c mount | grep /run/dbus
run on /run/dbus type tmpfs (ro,relatime,seclabel,mode=755)

The difference here being that the former just does a plain bind-mount:

[pid 31965] mount("/run/dbus", "/var/empty/run/dbus", 0x5942f2879040, MS_BIND, NULL) = 0

whereas the second one issues a remount afterwards:

[pid 31992] mount("/run/dbus", "/var/empty/run/dbus", 0x57c0c7f12040, MS_BIND, NULL) = 0
[pid 31992] mount("/run/dbus", "/var/empty/run/dbus", NULL, MS_RDONLY|MS_REMOUNT|MS_BIND, NULL) = 0

BUT I just realized that another this is happening when issuing mounts with MS_REMOUNT|MS_BIND: once it adds/removes the MNT_READONLY flag to the mountpoint, it also clears all other flags >< https://elixir.bootlin.com/linux/v4.4/source/fs/namespace.c#L2214 that means that this comment is wrong: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/security/chromiumos/lsm.c#92 so the mitigations that were set in place for https://bugs.chromium.org/p/chromium/issues/detail?id=810235 can be bypassed trivially by issuing a bind+remount. welp :(

minijail patches are out for review: https://android-review.googlesource.com/c/platform/external/minijail/+/715442/

the LSM patch is almost ready, but I need to chase some more denials. If everything goes well, we should have everything ready before EOW.

LSM WIP change here: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1132688

There's at least one Android change that's needed to quiesce one failure, although the system still boots.

Let me open a separate bug for the LSM changes to avoid conflating the minijail change with that.

All the changes are in the CQ.

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

commit 5de15bbdb0d07752484007c95194c41f33ea7db7
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Fri Jul 13 09:22:07 2018

Marking 9999 ebuild for chromeos-base/minijail as stable.

Pulled 2 new change from platform/external/minijail: (ebuild: chromeos-base/minijail)

  c5f4f47 More RAII for system_unittests
  0bacbf8 minijail: Copy the mount flags from source when bind-mounting
  6bdebb0 minijail: Use std::string for the system_unittests

BUG= chromium:862171 
TEST=precq passes
TEST=# minijail0 --profile=minimalistic-mountns \
       -k 'tmpfs,/run,tmpfs,MS_NOEXEC|MS_NODEV|MS_NOSUID' \
       -b /run/dbus -- /bin/sh -c 'mount | grep dbus'
     run on /run/dbus type tmpfs (ro,nosuid,nodev,noexec,relatime,seclabel,mode=755)
CQ-DEPEND=CL:1134600,CL:1134601

Change-Id: Id86ad5bcd9a9abb313c5eb01b6c7e64aacda5ff4
Reviewed-on: https://chromium-review.googlesource.com/1134400
Commit-Ready: Luis Hector Chavez <lhchavez@chromium.org>
Tested-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/5de15bbdb0d07752484007c95194c41f33ea7db7/chromeos-base/minijail/minijail-1-r17.ebuild

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/81cfb1cb6e133e5ae9c7c0432fc4fd948babc208

commit 81cfb1cb6e133e5ae9c7c0432fc4fd948babc208
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Fri Jul 13 09:22:06 2018

run_oci: Improve the flag propagation for bind-mounts

This change makes it possible to propagate the MS_RDONLY flag (or lack
thereof) to the bind-mount flags. This is because minijail now can
add/remove that flag correctly.

BUG= chromium:862171 
TEST=Android can still boot
TEST=cheets_ContainerSmokeTest
CQ-DEPEND=CL:1134400

Change-Id: I1a552dedf7d242236659d23a3e11688e4d2d11fb
Reviewed-on: https://chromium-review.googlesource.com/1134600
Commit-Ready: Luis Hector Chavez <lhchavez@chromium.org>
Tested-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>

[modify] https://crrev.com/81cfb1cb6e133e5ae9c7c0432fc4fd948babc208/run_oci/run_oci.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a6c219e940ff82daf5c835c6fc5492aa5e691bfc

commit a6c219e940ff82daf5c835c6fc5492aa5e691bfc
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Fri Jul 13 09:22:07 2018

arc: Add the rw flag to the ptmx mount

This mount is not visible to run_oci when it starts building the
mountpoint list, so it does not know what the original flags for it will
be. This change helps run_oci by providing an explicit "rw" for the
/dev/ptmx mount, so that it ends up with the right set of flags.

BUG= chromium:862171 
TEST=Android still boots
TEST=cheets_ContainerSmokeTest
CQ-DEPEND=CL:1134400

Change-Id: I723753dc5bd72defea71dc69274676e451d27d12
Reviewed-on: https://chromium-review.googlesource.com/1134601
Commit-Ready: Luis Hector Chavez <lhchavez@chromium.org>
Tested-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a6c219e940ff82daf5c835c6fc5492aa5e691bfc/arc/container-bundle/pi/config.json
[modify] https://crrev.com/a6c219e940ff82daf5c835c6fc5492aa5e691bfc/arc/container-bundle/nyc/config.json

And the minijail part is done!

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot