Commit graph

22 commits

Author SHA1 Message Date
Daniel Verkamp
6a7fdb4510 seccomp: add getcwd and readlink to common policy for panic
The panic handler uses getcwd and readlink to print out the executable
name in the backtrace. Allow these for all devices so that panics
actually work instead of crashing the process.

BUG=None
TEST=intentionally panic crosvm on kevin and check /var/log/messages

Change-Id: If64a752a6f0b1f2f6bdd6663ce77078305f38171
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3309201
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2021-12-02 23:18:03 +00:00
Chirantan Ekbote
f01d4b1f16 seccomp: Add unshare to fs device policy
Needed because of CL:3263932

BUG=none
TEST=vm.Fio

Change-Id: I50502476dd9591c114094cc43dda81f2c6e2c516
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3276112
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2021-11-11 13:38:11 +00:00
Chirantan Ekbote
b56de80b07 fs: Support fs-verity
Needed by arcvm.

BUG=b:141632062
TEST=Run `fsverity enable $FILE` and `fsverity measure $FILE` inside a
     VM on a virtio-fs mountpoint

Change-Id: Ifb9e7e9c8b924a835971bf298256495d8dc81733
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2387820
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2021-08-23 16:15:03 +00:00
Jorge E. Moreira
c8cff01c36 Specify prctl's policy only once per device
The libminijail version in AOSP complains when there are multiple entries for
the same system call, which was the case for virtio-fs's policy.

BUG=b/185811304

Change-Id: I389c07c86e7d79f16e4f47a893abad598033352a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2837307
Commit-Queue: Jorge Moreira Broche <jemoreira@google.com>
Tested-by: Jorge Moreira Broche <jemoreira@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
2021-04-20 22:50:20 +00:00
Chirantan Ekbote
a00991cd84 Replace dup with fcntl(F_DUPFD_CLOEXEC)
Fds created via dup don't share file descriptor flags with the original
fd, which means that they don't have the FD_CLOEXEC flag set.  Use
fcntl(F_DUPFD_CLOEXEC) so that this flag gets set for the duplicated fds
as well.

BUG=none
TEST=unit tests

Change-Id: Ib471cf40acac1eacf72969ba45247f50b349ed58
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2809687
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
2021-04-15 10:34:04 +00:00
Chirantan Ekbote
4b47aa7105 fs: Fix device socket
validate_raw_fd assumes that the fd passed in was not created by crosvm
and returns EBADF if it sees that the fd has the FD_CLOEXEC flag set.
We can't use it for fds created by the fs device since those do have
that flag set.

We're already taking a `&dyn AsRawFd` as the parameter so just assume
it's valid and clone it directly since there's no safe way to create an
invalid one.

BUG=none
TEST=vm.Fio.virtiofs_dax* tests are no longer failing

Change-Id: I10d9752e0960143fb58a63d2b76f64d34ec464d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2809686
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2021-04-09 08:04:30 +00:00
Chirantan Ekbote
7b98502972 fs: Don't drop caps when changing UID
Set the SECBIT_NO_SETUID_FIXUP securebit so that we don't lose
capabilities when changing the thread uid/gid.  This allows us to
simplify the create and mkdir functions so that all the checks we
currently carry out are only done once by the host kernel.

To ensure that the setuid and setgid bits still get dropped when a file
is modified by a process that doesn't hold CAP_FSETID, check for
WRITE_KILL_PRIV in the write flags and temporarily drop CAP_FSETID when
it is set.

BUG=none
TEST=Check that default posix acls, setgid bits, and file/directory
     creation via membership of a supplementary group all work as
     expected.

Change-Id: I420484e357a970e997cb3e968a433278e82d8ad4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2684067
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2021-03-12 04:11:58 +00:00
Chirantan Ekbote
8e06159af3 virtiofs: Support FS_IOC_GET_ENCRYPTION_POLICY_EX
Needed by arcvm.  Also fix some style issues with the previous ioctl
definitions.

BUG=b:136127632
TEST=Call this ioctl from inside a VM and see that it is passed through
     to the host.

Change-Id: Icdeff9e2c5b15237ed49e6814b8147258a50ccab
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2560286
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
2020-12-08 00:26:39 +00:00
Chirantan Ekbote
c6810f2d4b virtiofs: Drop regular encryption ioctls
These don't work without a kernel change that we never merged.  Arcvm
doesn't need these anymore so just drop them.

BUG=b:136127632
TEST=vm.Virtiofs

Change-Id: I427dee41720b0db1d14f4d03c95ff678ec3fd347
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2573709
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2020-12-08 00:26:39 +00:00
Chirantan Ekbote
b696d4feb2 fs: Don't use path-based operations
There are no *at variants for the xattr syscalls so we were using the
path-based variants by reading the /proc/self/fd symlink.  While this
symlink can track renames it cannot track if the user hard links the
file to some other location and then unlinks the original location.  In
this case the symlink simply shows that the path was deleted even though
the fd is still valid.

To deal with this, stop using the path-based method.  For regular files
and directories, we can simply open them and then make the appropriate
f{get,set,list,remove}xattr syscall.  For non-regular files and
directories we can emulate an *at syscall by changing the CWD to /proc
and then making the path-based syscall via the self/fd path.  Even
though the symlink is no longer valid, manipulating the fd via this path
still works.

BUG=b:168689429
TEST=Create a file, set an xattr, hard link it to a new location,
     unlink the original, and then verify that {get,set}fattr still
     works on the new link

Change-Id: Ic291f432b96f18e3e7ccbcf27737d5f8fd240a65
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2437684
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2020-10-12 15:45:19 +00:00
Chirantan Ekbote
2135787b74 devices: fs: Support FS_IOC_{GET,SET}FLAGS
Needed by arcvm.

BUG=b:159870893
TEST=`lsattr foo` and `chattr +S foo` inside a VM.  Check with strace
     that it's making the ioctl.

Cq-Depend: chromium:2297090
Change-Id: I11b490062b3c777c0cf4543f3d09f94e3790453b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2297004
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Lepton Wu <lepton@chromium.org>
2020-07-17 22:30:32 +00:00
Chirantan Ekbote
44336b9131 devices: fs: Fix posix acl handling
Posix acls are a truly incredible example of API design.  The presence
of a default posix acl in a directory completely changes the meaning of
the `mode` parameter for all system call that create inodes.  However,
this new behavior only applies when the inode is first created and not
for any subsequent operations that use the mode, like fchmod.

When a directory has a default posix acl, all inodes created in that
directory get the permissions specified in the default acl.  The mode
parameter is treated like a umask where any permissions allowed by the
default acl that are not allowed by the mode parameter are blocked.  The
actual umask is ignored in this case.

So to handle this properly we need to set FUSE_DONT_MASK to prevent the
kernel driver from preemptively applying the umask.  Then we have to
check if the parent directory has a default posix acl and only apply the
umask to the mode if it does not.  This also means that we cannot use
`mkdtemp` because that always creates directories with a mode of 0o700
and since the default posix acl calculation only applies on creation and
not on later operations, we need to apply the proper mode in the very
beginning.

BUG=b:159285544,b:152806644
TEST=vm.Virtiofs.  Use a test program to create files/directories in
     directories that have a default acl and ones that don't, and verify
     that the mode is correctly set after creation

Change-Id: Ieca8ac9db391feebe5719630c5f3b57b04b71533
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2260253
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
2020-06-29 10:14:42 +00:00
Chirantan Ekbote
dc82c819f6 devices: fs: Implement FS_IOC_FSSETXATTR
Needed by arcvm.

BUG=b:159297591
TEST=run a test program that calls the ioctl

Change-Id: I1f8c17fa2b2457f5a9e73045c3dbee3440eb943d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2265932
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2020-06-26 09:10:32 +00:00
Chirantan Ekbote
df71719d7f devices: fs: Support FS_IOC_FSGETXATTR
Arcvm needs this ioctl for looking up the project id of an inode.

BUG=b:157189438
TEST=Call the ioctl inside a vm and verify that it succeeds

Change-Id: Ib178cf32b09056f9b1e9acedb49de068d5525a66
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2214964
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Matthias Springer <springerm@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
2020-06-16 11:13:14 +00:00
Chirantan Ekbote
814a8da0ed devices: fs: Use 2 stage create and mkdir
When creating a file or directory the virtio-fs server changes its
effective uid and gid to the uid and gid of the process that made the
call.  This ensures that the file or directory has the correct owner and
group when it is created and also serves as an access check to ensure
that the process that made the call has permission to modify the parent
directory.

However, this causes an EACCES error when the following conditions are
met:

  * The parent directory has g+rw permissions with gid A
  * The process has gid B but has A in its list of supplementary groups

In this case the fuse context only contains gid B, which doesn't have
permission to modify the parent directory.

Unfortunately there's no way for us to detect this on the server side so
instead we just have to rely on the permission checks carried out by the
kernel driver. If the server receives a create call, then assume that
the kernel has verified that the process is allowed to create that
file/directory and just create it without changing the server thread's
uid and gid.

Additionally, in order to ensure that a newly created file appears
atomically in the parent directory with the proper owner and group,
change the create implementation to use `O_TMPFILE` and `linkat` as
described in the open(2) manpage.  There is no `O_TMPFILE` equivalent
for directories so create a "hidden" directory with a randomly generated
name, modify the uid/gid and mode, and then rename it into place.

BUG=b:156696212
TEST=tast run $DUT vm.Virtiofs
TEST=Create a test directory with group wayland and permissions g+rw.
     Then run `su -s /bin/bash -c 'touch ${dir}/foo' - crosvm` and
     `su -s /bin/bash -c 'mkdir ${dir}/bar' - crosvm`.

Change-Id: If5fbcb1b011664c7c1ac29542a2f90d129c34962
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2217534
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
2020-06-04 12:49:49 +00:00
Chirantan Ekbote
d74bb77a3e devices: fs: Use l{get,set,list,remove}xattr
Using the `open_inode` method on an fd for a symlink results in the
kernel returning -ELOOP.  Since there are no `*at` methods for extended
attributes, manually read the path for the file and then use the
l{get,set,list,remove}xattr method on the returned path.

BUG=b:136128512
TEST=boot arcvm with virtio-fs and selinux enabled

Change-Id: I2fde57db8a075838a3a877309f6cf89059f19258
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2120763
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
2020-03-27 00:44:06 +00:00
Chirantan Ekbote
6dfa1e4ce5 devices: fs: Implement copy_file_range
BUG=none
TEST=vm.Virtiofs

Change-Id: I2ed7137a901e6e506e6b1562b77fdb042bdc58ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2105822
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2020-03-19 08:13:31 +00:00
Chirantan Ekbote
2977573365 seccomp: Add missing syscalls to fs device policy
BUG=none
TEST=`tast run <dut> vm.Fio.virtiofs`

Change-Id: I937df0be738e0aa302a4ad3e87ed33ff97afb4fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2089174
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2020-03-06 03:48:12 +00:00
Matt Delco
4389dab579 seccomp: remove redundant unconditional rules
Minijail's policy compiler complains when there's multiple
unconditional rules for a syscall.  In most cases the rules
are redundant to common_device.policy.  I don't know what
to do about the intentionally contradictory rules for open
and openat, other than to remove then from the common device
policy and add it to all the others.

BUG=None
TEST=Ran compile_seccomp_policy.py until it stopped
complaining.

Change-Id: I6813dd1e0b39e975415662bd7de74c25a1be9eb3
Signed-off-by: Matt Delco <delco@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1918607
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
2020-01-22 17:36:36 +00:00
Daniel Verkamp
5de0604f29 seccomp: allow statx syscall where stat/fstat was allowed
This is used in Rust 1.40.0's libstd in place of stat/fstat; update the
whitelists to allow the new syscall as well.

BUG=chromium:1042461
TEST=`crosvm disk resize` does not trigger seccomp failure

Change-Id: Ia3f0e49ee009547295c7af7412dfb5eb3ac1efcb
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2003685
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2020-01-17 23:04:03 +00:00
Chirantan Ekbote
4f9f5c7479 devices: fs: Support fs crypto ioctls
Add support for FS_IOC_{GET,SET}_ENCRYPTION_POLICY.  Unfortunately,
since the I/O direction is encoded backwards in the ioctl definitions,
these will only work with on a kernel that's compiled with a patch to
mark them as unrestricted FUSE ioctls.

BUG=b:136127632
TEST=Compile and run the vfs_crypto.c program on a virtio-fs mount
     inside a VM

Change-Id: I124c5a943111b453dd44921a079a2baa1036dfd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1952570
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2019-12-10 03:10:57 +00:00
Chirantan Ekbote
b0ac00745a Enable seccomp for virtio-fs
Add x86_64 and arm seccomp policy files for the virtio-fs device.

BUG=b:136128319
TEST=Run a vm with a sandboxed virtio-fs device

Change-Id: I8ea7c5d3e90696077f0c2b1f942cb286a0b36cf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1916559
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
2019-11-19 17:59:49 +00:00