From 4f9f5c74791fd13f6930a48232b3327066259b8e Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Thu, 5 Dec 2019 19:20:48 +0900 Subject: 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 Tested-by: Chirantan Ekbote Tested-by: kokoro Commit-Queue: Chirantan Ekbote --- devices/src/virtio/fs/filesystem.rs | 64 ++++++++++++++++++--- devices/src/virtio/fs/fuse.rs | 19 +++--- devices/src/virtio/fs/mod.rs | 7 +++ devices/src/virtio/fs/passthrough.rs | 108 ++++++++++++++++++++++++++++++++++- devices/src/virtio/fs/server.rs | 99 +++++++++++++++++++++++++++++--- seccomp/arm/fs_device.policy | 1 + seccomp/x86_64/fs_device.policy | 1 + tests/vfs_crypto.c | 58 +++++++++++++++++++ 8 files changed, 330 insertions(+), 27 deletions(-) create mode 100644 tests/vfs_crypto.c diff --git a/devices/src/virtio/fs/filesystem.rs b/devices/src/virtio/fs/filesystem.rs index 53a3f1e..232ff99 100644 --- a/devices/src/virtio/fs/filesystem.rs +++ b/devices/src/virtio/fs/filesystem.rs @@ -13,10 +13,7 @@ use libc; use crate::virtio::fs::fuse; -pub use fuse::FsOptions; -pub use fuse::OpenOptions; -pub use fuse::SetattrValid; -pub use fuse::ROOT_ID; +pub use fuse::{FsOptions, IoctlFlags, IoctlIovec, OpenOptions, SetattrValid, ROOT_ID}; /// Information about a path in the filesystem. pub struct Entry { @@ -111,6 +108,26 @@ pub enum ListxattrReply { Count(u32), } +/// A reply to an `ioctl` method call. +pub enum IoctlReply { + /// Indicates that the ioctl should be retried. This is only a valid reply when the `flags` + /// field of the ioctl request contains `IoctlFlags::UNRESTRICTED`. The kernel will read in data + /// and prepare output buffers as specified in the `input` and `output` fields before re-sending + /// the ioctl message. + Retry { + /// Data that should be read by the kernel module and sent to the server when the ioctl is + /// retried. + input: Vec, + + /// Buffer space that should be prepared so that the server can send back the response to + /// the ioctl. + output: Vec, + }, + + /// Indicates that the ioctl was processed. + Done(io::Result>), +} + /// A trait for directly copying data from the fuse transport into a `File` without first storing it /// in an intermediate buffer. pub trait ZeroCopyReader { @@ -1054,23 +1071,52 @@ pub trait FileSystem { Err(io::Error::from_raw_os_error(libc::ENOSYS)) } - /// TODO: support this - fn getlk(&self) -> io::Result<()> { + /// Perform an ioctl on a file or directory. + /// + /// `handle` is the `Handle` returned by the file system from the `open` or `opendir` methods, + /// if any. If the file system did not return a `Handle` from then the contents of `handle` are + /// undefined. + /// + /// If `flags` contains `IoctlFlags::UNRESTRICTED` then the file system may retry the ioctl + /// after informing the kernel about the input and output areas. If `flags` does not contain + /// `IoctlFlags::UNRESTRICTED` then the kernel will prepare the input and output areas according + /// to the encoding in the ioctl command. In that case the ioctl cannot be retried. + /// + /// `cmd` is the ioctl request made by the calling process, truncated to 32 bits. + /// + /// `arg` is the argument provided by the calling process. + /// + /// `in_size` is the length of the additional data that accompanies the request. The file system + /// may fetch this data from `reader`. + /// + /// `out_size` is the length of the output area prepared by the kernel to hold the response to + /// the ioctl. + fn ioctl( + &self, + ctx: Context, + handle: Self::Handle, + flags: IoctlFlags, + cmd: u32, + arg: u64, + in_size: u32, + out_size: u32, + reader: R, + ) -> io::Result { Err(io::Error::from_raw_os_error(libc::ENOSYS)) } /// TODO: support this - fn setlk(&self) -> io::Result<()> { + fn getlk(&self) -> io::Result<()> { Err(io::Error::from_raw_os_error(libc::ENOSYS)) } /// TODO: support this - fn setlkw(&self) -> io::Result<()> { + fn setlk(&self) -> io::Result<()> { Err(io::Error::from_raw_os_error(libc::ENOSYS)) } /// TODO: support this - fn ioctl(&self) -> io::Result<()> { + fn setlkw(&self) -> io::Result<()> { Err(io::Error::from_raw_os_error(libc::ENOSYS)) } diff --git a/devices/src/virtio/fs/fuse.rs b/devices/src/virtio/fs/fuse.rs index f9f97d8..612202b 100644 --- a/devices/src/virtio/fs/fuse.rs +++ b/devices/src/virtio/fs/fuse.rs @@ -339,27 +339,24 @@ const IOCTL_32BIT: u32 = 8; const IOCTL_DIR: u32 = 16; /// Maximum of in_iovecs + out_iovecs -const IOCTL_MAX_IOV: u32 = 256; +pub const IOCTL_MAX_IOV: usize = 256; bitflags! { pub struct IoctlFlags: u32 { /// 32bit compat ioctl on 64bit machine - const IOCTL_COMPAT = IOCTL_COMPAT; + const COMPAT = IOCTL_COMPAT; /// Not restricted to well-formed ioctls, retry allowed - const IOCTL_UNRESTRICTED = IOCTL_UNRESTRICTED; + const UNRESTRICTED = IOCTL_UNRESTRICTED; /// Retry with new iovecs - const IOCTL_RETRY = IOCTL_RETRY; + const RETRY = IOCTL_RETRY; /// 32bit ioctl const IOCTL_32BIT = IOCTL_32BIT; /// Is a directory - const IOCTL_DIR = IOCTL_DIR; - - /// Maximum of in_iovecs + out_iovecs - const IOCTL_MAX_IOV = IOCTL_MAX_IOV; + const DIR = IOCTL_DIR; } } @@ -872,10 +869,16 @@ pub struct IoctlIn { } unsafe impl DataInit for IoctlIn {} +/// Describes a region of memory in the address space of the process that made the ioctl syscall. +/// Similar to `libc::iovec` but uses `u64`s for the address and the length. #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct IoctlIovec { + /// The start address of the memory region. This must be in the address space of the process + /// that made the ioctl syscall. pub base: u64, + + /// The length of the memory region. pub len: u64, } unsafe impl DataInit for IoctlIovec {} diff --git a/devices/src/virtio/fs/mod.rs b/devices/src/virtio/fs/mod.rs index a1f8868..9cac98b 100644 --- a/devices/src/virtio/fs/mod.rs +++ b/devices/src/virtio/fs/mod.rs @@ -84,6 +84,8 @@ pub enum Error { /// The `size` field of the `SetxattrIn` message does not match the length /// of the decoded value. InvalidXattrSize((u32, usize)), + /// Requested too many `iovec`s for an `ioctl` retry. + TooManyIovecs((usize, usize)), } impl ::std::error::Error for Error {} @@ -116,6 +118,11 @@ impl fmt::Display for Error { decoded value: size = {}, value.len() = {}", size, len ), + TooManyIovecs((count, max)) => write!( + f, + "requested too many `iovec`s for an `ioctl` retry reply: requested {}, max: {}", + count, max + ), } } } diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index 7f6e3bb..bdc1c6f 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -17,11 +17,11 @@ use std::time::Duration; use data_model::DataInit; use libc; use sync::Mutex; -use sys_util::error; +use sys_util::{error, ioctl_ior_nr, ioctl_iow_nr, ioctl_with_mut_ptr, ioctl_with_ptr}; use crate::virtio::fs::filesystem::{ - Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, ListxattrReply, OpenOptions, - SetattrValid, ZeroCopyReader, ZeroCopyWriter, + Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, IoctlFlags, IoctlIovec, + IoctlReply, ListxattrReply, OpenOptions, SetattrValid, ZeroCopyReader, ZeroCopyWriter, }; use crate::virtio::fs::fuse; use crate::virtio::fs::multikey::MultikeyBTreeMap; @@ -32,6 +32,22 @@ const EMPTY_CSTR: &[u8] = b"\0"; const ROOT_CSTR: &[u8] = b"/\0"; const PROC_CSTR: &[u8] = b"/proc\0"; +const FSCRYPT_KEY_DESCRIPTOR_SIZE: usize = 8; + +#[repr(C)] +#[derive(Debug, Clone, Copy)] +struct fscrypt_policy_v1 { + _version: u8, + _contents_encryption_mode: u8, + _filenames_encryption_mode: u8, + _flags: u8, + _master_key_descriptor: [u8; FSCRYPT_KEY_DESCRIPTOR_SIZE], +} +unsafe impl DataInit for fscrypt_policy_v1 {} + +ioctl_ior_nr!(FS_IOC_SET_ENCRYPTION_POLICY, 0x66, 19, fscrypt_policy_v1); +ioctl_iow_nr!(FS_IOC_GET_ENCRYPTION_POLICY, 0x66, 21, fscrypt_policy_v1); + type Inode = u64; type Handle = u64; @@ -629,6 +645,50 @@ impl PassthroughFs { Err(io::Error::last_os_error()) } } + + fn get_encryption_policy(&self, handle: Handle) -> io::Result { + let data = self + .handles + .read() + .unwrap() + .get(&handle) + .map(Arc::clone) + .ok_or_else(ebadf)?; + + let mut buf = MaybeUninit::::zeroed(); + let file = data.file.lock(); + + // Safe because the kernel will only write to `buf` and we check the return value. + let res = + unsafe { ioctl_with_mut_ptr(&*file, FS_IOC_GET_ENCRYPTION_POLICY(), buf.as_mut_ptr()) }; + if res < 0 { + Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) + } else { + // Safe because the kernel guarantees that the policy is now initialized. + let policy = unsafe { buf.assume_init() }; + Ok(IoctlReply::Done(Ok(policy.as_slice().to_vec()))) + } + } + + fn set_encryption_policy(&self, handle: Handle, r: R) -> io::Result { + let data = self + .handles + .read() + .unwrap() + .get(&handle) + .map(Arc::clone) + .ok_or_else(ebadf)?; + + let policy = fscrypt_policy_v1::from_reader(r)?; + let file = data.file.lock(); + // Safe because the kernel will only read from `policy` and we check the return value. + let res = unsafe { ioctl_with_ptr(&*file, FS_IOC_SET_ENCRYPTION_POLICY(), &policy) }; + if res < 0 { + Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) + } else { + Ok(IoctlReply::Done(Ok(Vec::new()))) + } + } } fn forget_one( @@ -1587,4 +1647,46 @@ impl FileSystem for PassthroughFs { Err(io::Error::last_os_error()) } } + + fn ioctl( + &self, + _ctx: Context, + handle: Handle, + _flags: IoctlFlags, + cmd: u32, + arg: u64, + in_size: u32, + out_size: u32, + r: R, + ) -> io::Result { + // Normally, we wouldn't need to retry the FS_IOC_GET_ENCRYPTION_POLICY and + // FS_IOC_SET_ENCRYPTION_POLICY ioctls. Unfortunately, the I/O directions for both of them + // are encoded backwards so they can only be handled as unrestricted fuse ioctls. + if cmd == FS_IOC_GET_ENCRYPTION_POLICY() as u32 { + if out_size < size_of::() as u32 { + let input = Vec::new(); + let output = vec![IoctlIovec { + base: arg, + len: size_of::() as u64, + }]; + Ok(IoctlReply::Retry { input, output }) + } else { + self.get_encryption_policy(handle) + } + } else if cmd == FS_IOC_SET_ENCRYPTION_POLICY() as u32 { + if in_size < size_of::() as u32 { + let input = vec![IoctlIovec { + base: arg, + len: size_of::() as u64, + }]; + let output = Vec::new(); + Ok(IoctlReply::Retry { input, output }) + } else { + self.set_encryption_policy(handle, r) + } + } else { + // Did you know that a file/directory is not a TTY? + Err(io::Error::from_raw_os_error(libc::ENOTTY)) + } + } } diff --git a/devices/src/virtio/fs/server.rs b/devices/src/virtio/fs/server.rs index 9c3136d..32b5008 100644 --- a/devices/src/virtio/fs/server.rs +++ b/devices/src/virtio/fs/server.rs @@ -12,8 +12,8 @@ use libc; use sys_util::error; use crate::virtio::fs::filesystem::{ - Context, DirEntry, Entry, FileSystem, GetxattrReply, ListxattrReply, ZeroCopyReader, - ZeroCopyWriter, + Context, DirEntry, Entry, FileSystem, GetxattrReply, IoctlReply, ListxattrReply, + ZeroCopyReader, ZeroCopyWriter, }; use crate::virtio::fs::fuse::*; use crate::virtio::fs::{Error, Result}; @@ -1097,11 +1097,35 @@ impl Server { Ok(0) } - fn ioctl(&self, in_header: InHeader, _r: Reader, w: Writer) -> Result { - if let Err(e) = self.fs.ioctl() { - reply_error(e, in_header.unique, w) - } else { - Ok(0) + fn ioctl(&self, in_header: InHeader, mut r: Reader, w: Writer) -> Result { + let IoctlIn { + fh, + flags, + cmd, + arg, + in_size, + out_size, + } = r.read_obj().map_err(Error::DecodeMessage)?; + + let res = self.fs.ioctl( + in_header.into(), + fh.into(), + IoctlFlags::from_bits_truncate(flags), + cmd, + arg, + in_size, + out_size, + r, + ); + + match res { + Ok(reply) => match reply { + IoctlReply::Retry { input, output } => { + retry_ioctl(in_header.unique, input, output, w) + } + IoctlReply::Done(res) => finish_ioctl(in_header.unique, res, w), + }, + Err(e) => reply_error(e, in_header.unique, w), } } @@ -1186,6 +1210,67 @@ impl Server { } } +fn retry_ioctl( + unique: u64, + input: Vec, + output: Vec, + mut w: Writer, +) -> Result { + // We don't need to check for overflow here because if adding these 2 values caused an overflow + // we would have run out of memory before reaching this point. + if input.len() + output.len() > IOCTL_MAX_IOV { + return Err(Error::TooManyIovecs(( + input.len() + output.len(), + IOCTL_MAX_IOV, + ))); + } + + let len = size_of::() + + size_of::() + + (input.len() * size_of::()) + + (output.len() * size_of::()); + let header = OutHeader { + len: len as u32, + error: 0, + unique, + }; + let out = IoctlOut { + result: 0, + flags: IoctlFlags::RETRY.bits(), + in_iovs: input.len() as u32, + out_iovs: output.len() as u32, + }; + + w.write_obj(header).map_err(Error::EncodeMessage)?; + w.write_obj(out).map_err(Error::EncodeMessage)?; + for i in input.into_iter().chain(output.into_iter()) { + w.write_obj(i).map_err(Error::EncodeMessage)?; + } + + debug_assert_eq!(len, w.bytes_written()); + Ok(w.bytes_written()) +} + +fn finish_ioctl(unique: u64, res: io::Result>, w: Writer) -> Result { + let (out, data) = match res { + Ok(data) => { + let out = IoctlOut { + result: 0, + ..Default::default() + }; + (out, Some(data)) + } + Err(e) => { + let out = IoctlOut { + result: -e.raw_os_error().unwrap_or(libc::EIO), + ..Default::default() + }; + (out, None) + } + }; + reply_ok(Some(out), data.as_ref().map(|d| &d[..]), unique, w) +} + fn reply_ok( out: Option, data: Option<&[u8]>, diff --git a/seccomp/arm/fs_device.policy b/seccomp/arm/fs_device.policy index 0ea7fe0..0708ec8 100644 --- a/seccomp/arm/fs_device.policy +++ b/seccomp/arm/fs_device.policy @@ -16,6 +16,7 @@ ftruncate64: 1 getdents64: 1 getegid32: 1 geteuid32: 1 +ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY linkat: 1 _llseek: 1 mkdirat: 1 diff --git a/seccomp/x86_64/fs_device.policy b/seccomp/x86_64/fs_device.policy index cbf0288..20db0bf 100644 --- a/seccomp/x86_64/fs_device.policy +++ b/seccomp/x86_64/fs_device.policy @@ -15,6 +15,7 @@ ftruncate: 1 getdents64: 1 getegid: 1 geteuid: 1 +ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY linkat: 1 lseek: 1 mkdirat: 1 diff --git a/tests/vfs_crypto.c b/tests/vfs_crypto.c new file mode 100644 index 0000000..3672b31 --- /dev/null +++ b/tests/vfs_crypto.c @@ -0,0 +1,58 @@ +// Copyright 2019 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + +#include +#include +#include +#include +#include +#include +#include +#include + +extern char* program_invocation_short_name; + +int main(int argc, char** argv) { + if (argc != 2) { + printf("Usage: %s \n", program_invocation_short_name); + return 1; + } + + int dir = open(argv[1], O_DIRECTORY | O_CLOEXEC); + if (dir < 0) { + perror("Failed to open directory"); + return 1; + } + + struct fscrypt_policy policy; + int ret = ioctl(dir, FS_IOC_GET_ENCRYPTION_POLICY, &policy); + if (ret < 0) { + perror("FS_IOC_GET_ENCRYPTION_POLICY failed"); + return 1; + } + + printf("File system encryption policy:\n"); + printf("\tversion = %#x\n", policy.version); + printf("\tcontents_encryption_mode = %#x\n", policy.contents_encryption_mode); + printf("\tfilenames_encryption_mode = %#x\n", + policy.filenames_encryption_mode); + printf("\tflags = %#x\n", policy.flags); + printf("\tmaster_key_descriptor = 0x"); + for (int i = 0; i < FS_KEY_DESCRIPTOR_SIZE; ++i) { + printf("%x", policy.master_key_descriptor[i]); + } + printf("\n"); + + ret = ioctl(dir, FS_IOC_SET_ENCRYPTION_POLICY, &policy); + if (ret < 0) { + perror("FS_IOC_SET_ENCRYPTION_POLICY failed"); + return 1; + } + + return 0; +} -- cgit 1.4.1