Harden SEFS with extra MAC and permission checks

This commit is contained in:
Shuang Liu 2019-10-12 09:03:06 +00:00 committed by Tate, Hongliang Tian
parent 70fd9dc210
commit 68e02962d5
15 changed files with 258 additions and 30 deletions

2
deps/sefs vendored

@ -1 +1 @@
Subproject commit f095460eefa473eefaddf8723170289e16d4648e
Subproject commit 96fbb3ed48ec949dd3aa0d34b8a54205bd8e09d3

@ -99,19 +99,23 @@ impl INode for HNode {
}
fn set_metadata(&self, metadata: &Metadata) -> Result<()> {
unimplemented!()
warn!("HostFS: set_metadata() is unimplemented");
Ok(())
}
fn sync_all(&self) -> Result<()> {
unimplemented!()
warn!("HostFS: sync_all() is unimplemented");
Ok(())
}
fn sync_data(&self) -> Result<()> {
unimplemented!()
warn!("HostFS: sync_data() is unimplemented");
Ok(())
}
fn resize(&self, len: usize) -> Result<()> {
unimplemented!()
warn!("HostFS: resize() is unimplemented");
Ok(())
}
fn create(&self, name: &str, type_: FileType, mode: u32) -> Result<Arc<dyn INode>> {
@ -185,7 +189,8 @@ impl INode for HNode {
}
fn io_control(&self, cmd: u32, data: usize) -> Result<()> {
unimplemented!()
warn!("HostFS: io_control is unimplemented");
Ok(())
}
fn fs(&self) -> Arc<FileSystem> {

@ -1,4 +1,6 @@
use super::*;
use rcore_fs_sefs::dev::SefsMac;
use sgx_trts::libc::S_IRUSR;
use std::fmt;
pub struct INodeFile {
@ -148,6 +150,13 @@ impl File for INodeFile {
impl INodeFile {
pub fn open(inode: Arc<INode>, options: OpenOptions) -> Result<Self, Error> {
if (options.read && !inode.allow_read()?) {
return errno!(EBADF, "File not readable");
}
if (options.write && !inode.allow_write()?) {
return errno!(EBADF, "File not writable");
}
Ok(INodeFile {
inode,
offset: SgxMutex::new(0),
@ -178,6 +187,9 @@ impl From<FsError> for Error {
FsError::IOCTLError => EINVAL,
FsError::Again => EAGAIN,
FsError::Busy => EBUSY,
FsError::WrProtected => EROFS,
FsError::NoIntegrity => EROFS,
FsError::PermError => EPERM,
};
Error::new(errno, "")
}
@ -196,6 +208,8 @@ impl Debug for INodeFile {
pub trait INodeExt {
fn read_as_vec(&self) -> Result<Vec<u8>, Error>;
fn allow_write(&self) -> Result<bool, Error>;
fn allow_read(&self) -> Result<bool, Error>;
}
impl INodeExt for INode {
@ -208,4 +222,18 @@ impl INodeExt for INode {
self.read_at(0, buf.as_mut_slice())?;
Ok(buf)
}
fn allow_write(&self) -> Result<bool, Error> {
let info = self.metadata()?;
let perms = info.mode as u32;
let writable = (perms & S_IWUSR) == S_IWUSR;
Ok(writable)
}
fn allow_read(&self) -> Result<bool, Error> {
let info = self.metadata()?;
let perms = info.mode as u32;
let readable = (perms & S_IRUSR) == S_IRUSR;
Ok(readable)
}
}

@ -19,6 +19,7 @@ pub use self::pipe::Pipe;
pub use self::root_inode::ROOT_INODE;
pub use self::socket_file::{AsSocket, SocketFile};
pub use self::unix_socket::{AsUnixSocket, UnixSocketFile};
use sgx_trts::libc::S_IWUSR;
use std::any::Any;
use std::mem::uninitialized;
@ -311,6 +312,9 @@ pub fn do_mkdir(path: &str, mode: usize) -> Result<(), Error> {
if inode.find(file_name).is_ok() {
return errno!(EEXIST, "");
}
if !inode.allow_write()? {
return errno!(EPERM, "dir cannot be written");
}
inode.create(file_name, FileType::Dir, mode as u32)?;
Ok(())
}
@ -434,7 +438,12 @@ impl Process {
}
file_inode
}
Err(FsError::EntryNotFound) => dir_inode.create(file_name, FileType::File, mode)?,
Err(FsError::EntryNotFound) => {
if !dir_inode.allow_write()? {
return errno!(EPERM, "file cannot be created");
}
dir_inode.create(file_name, FileType::File, mode)?
}
Err(e) => return Err(Error::from(e)),
}
} else {

@ -27,7 +27,7 @@ lazy_static! {
}
fn open_root_fs_according_to(mount_config: &Vec<ConfigMount>) -> Result<Arc<MountFS>, Error> {
let root_sefs_source = {
let (root_sefs_mac, root_sefs_source) = {
let root_mount_config = mount_config
.iter()
.find(|m| m.target == Path::new("/"))
@ -45,11 +45,14 @@ fn open_root_fs_according_to(mount_config: &Vec<ConfigMount>) -> Result<Arc<Moun
"The root SEFS must be given a source path (on host)"
);
}
root_mount_config.source.as_ref().unwrap()
(
root_mount_config.options.mac,
root_mount_config.source.as_ref().unwrap(),
)
};
let root_sefs = SEFS::open(
Box::new(SgxStorage::new(root_sefs_source, true)),
Box::new(SgxStorage::new(root_sefs_source, true, root_sefs_mac)),
&time::OcclumTimeProvider,
&SgxUuidProvider,
)?;
@ -86,14 +89,14 @@ fn mount_nonroot_fs_according_to(
let source_path = mc.source.as_ref().unwrap();
let sefs = {
SEFS::open(
Box::new(SgxStorage::new(source_path, false)),
Box::new(SgxStorage::new(source_path, false, None)),
&time::OcclumTimeProvider,
&SgxUuidProvider,
)
}
.or_else(|_| {
SEFS::create(
Box::new(SgxStorage::new(source_path, false)),
Box::new(SgxStorage::new(source_path, false, None)),
&time::OcclumTimeProvider,
&SgxUuidProvider,
)

@ -25,12 +25,12 @@ pub struct SgxUuidProvider;
impl UuidProvider for SgxUuidProvider {
fn generate_uuid(&self) -> SefsUuid {
let mut uuid: [u8; 16] = Default::default();
let status = unsafe { sgx_read_rand(uuid.as_mut_ptr(), uuid.len()) };
if status != sgx_status_t::SGX_SUCCESS {
panic!("sgx_read_rand failed");
}
SefsUuid::new(uuid)
let mut uuid: [u8; 16] = [0u8; 16];
let buf = uuid.as_mut_ptr();
let size = 16;
let status = unsafe { sgx_read_rand(buf, size) };
assert!(status == sgx_status_t::SGX_SUCCESS);
SefsUuid(uuid)
}
}
@ -42,13 +42,17 @@ pub struct SgxStorage {
}
impl SgxStorage {
pub fn new(path: impl AsRef<Path>, integrity_only: bool) -> Self {
pub fn new(
path: impl AsRef<Path>,
integrity_only: bool,
file_mac: Option<sgx_aes_gcm_128bit_tag_t>,
) -> Self {
// assert!(path.as_ref().is_dir());
SgxStorage {
path: path.as_ref().to_path_buf(),
integrity_only: integrity_only,
file_cache: Mutex::new(BTreeMap::new()),
root_mac: None,
root_mac: file_mac,
}
}
/// Get file by `file_id`.
@ -174,6 +178,10 @@ impl Storage for SgxStorage {
caches.remove(&key);
Ok(())
}
fn is_integrity_only(&self) -> bool {
self.integrity_only
}
}
#[derive(Clone)]
@ -232,4 +240,9 @@ impl File for LockedFile {
file.flush().expect("failed to flush SgxFile");
Ok(())
}
fn get_file_mac(&self) -> DevResult<SefsMac> {
let file = self.0.lock().unwrap();
Ok(SefsMac(file.get_mac().unwrap()))
}
}

@ -5,7 +5,7 @@ BUILD_DIR := $(PROJECT_DIR)/build
# Dependencies: need to be compiled but not to run by any Makefile target
TEST_DEPS := dev_null client
# Tests: need to be compiled and run by test-% target
TESTS := empty env hello_world malloc mmap file getpid spawn sched pipe time \
TESTS := empty env hello_world malloc mmap file fs_perms getpid spawn sched pipe time \
truncate readdir mkdir link tls pthread uname rlimit server \
server_epoll unix_socket cout hostfs cpuid rdtsc device
# Benchmarks: need to be compiled and run by bench-% target

@ -6,7 +6,7 @@
#include <stdio.h>
int main(int argc, const char* argv[]) {
const char* file_name = "tmp.txt";
const char* file_name = "/root/test_filesystem_file_read_write.txt";
int fd, flags, mode, len;
off_t offset;
const char* write_msg = "Hello World\n";

5
test/fs_perms/Makefile Normal file

@ -0,0 +1,5 @@
include ../test_common.mk
EXTRA_C_FLAGS :=
EXTRA_LINK_FLAGS :=
BIN_ARGS :=

161
test/fs_perms/main.c Normal file

@ -0,0 +1,161 @@
#include <sys/types.h>
#include <sys/uio.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <stdio.h>
#include "test.h"
// ============================================================================
// Helper macros
// ============================================================================
//#define PRINT_DBG(msg) printf("%s %s %d ", msg, __FUNCTION__, __LINE__);
#define PRINT_DBG(msg)
#define OK (0)
#define NG (-1)
#define NUM_TEST_CASES 6
#define NUM_TEST_FILES 5
// ============================================================================
// Helper functions
// ============================================================================
static int open_file(const char *filename, int flags, int mode) {
int fd = -1;
if ((fd = open(filename, flags, mode)) < 0) {
PRINT_DBG("ERROR: failed to open a file\n");
}
return fd;
}
static const char* write_msg = "Hello SEFS 1234567890\n";
static int write_file(int fd) {
int len = strlen(write_msg);
if ((len = write(fd, write_msg, len)<= 0)) {
PRINT_DBG("ERROR: failed to write to the file\n");
return -1;
}
fsync(fd);
close(fd);
return 0;
}
static int read_file(int fd){
int len;
char read_buf[128] = {0};
if ((len = read(fd, read_buf, sizeof(read_buf) - 1)) <= 0) {
PRINT_DBG("ERROR: failed to read from the file\n");
return -1;
}
close(fd);
if (strcmp(write_msg, read_buf) != 0) {
PRINT_DBG("ERROR: the message read from the file is not as it was written\n");
return -1;
}
return 0;
}
// for each file in test_filename
// open the file with the given flags
// do read or write according to do_write
// check the result of the read/write with the given expected_result
static int do_perm_tests(
const char** files,
size_t num_files,
int flags, int do_write,
int* expected_results)
{
flags |= O_CREAT | O_TRUNC;
for(size_t i = 0; i < num_files; i++) {
const char* filename = files[i];
int expected_result = expected_results[i];
int fd = open_file(filename, flags, 0666);
int result = do_write ? write_file(fd) : read_file(fd);
if (result != expected_result) {
return -1;
}
}
return 0;
}
// ============================================================================
// Test cases
// ============================================================================
// Test files
static const char* test_files[NUM_TEST_FILES] = {
"/test_fs_perms.txt",
"/bin/test_fs_perms.txt",
"/lib/test_fs_perms.txt",
"/root/test_fs_perms.txt",
"/host/test_fs_perms.txt",
};
// Test cases X Test files -> Test Results
static int test_expected_results[NUM_TEST_CASES][NUM_TEST_FILES]= {
// test_open_ro_then_write()
{NG, NG, NG, NG, NG},
// test_open_wo_then_write()
{NG, NG, NG, OK, OK},
// test_open_rw_then_write()
{NG, NG, NG, OK, OK},
// test_open_ro_then_read()
{NG, NG, NG, OK, OK},
// test_open_wo_then_read()
{NG, NG, NG, NG, NG},
// test_open_rw_then_read()
{NG, NG, NG, OK, OK},
};
int test_open_ro_then_write() {
return do_perm_tests(test_files, NUM_TEST_FILES,
O_RDONLY, 1, test_expected_results[0]);
}
int test_open_wo_then_write() {
return do_perm_tests(test_files, NUM_TEST_FILES,
O_WRONLY, 1, test_expected_results[1]);
}
int test_open_rw_then_write() {
return do_perm_tests(test_files, NUM_TEST_FILES,
O_RDWR, 1, test_expected_results[2]);
}
int test_open_ro_then_read() {
return do_perm_tests(test_files, NUM_TEST_FILES,
O_RDONLY, 0, test_expected_results[3]);
}
int test_open_wo_then_read(){
return do_perm_tests(test_files, NUM_TEST_FILES,
O_WRONLY, 0, test_expected_results[4]);
}
int test_open_rw_then_read() {
return do_perm_tests(test_files, NUM_TEST_FILES,
O_RDWR, 0, test_expected_results[5]);
}
// ============================================================================
// Test suite main
// ============================================================================
test_case_t test_cases[NUM_TEST_CASES] = {
TEST_CASE(test_open_ro_then_write),
TEST_CASE(test_open_wo_then_write),
TEST_CASE(test_open_rw_then_write),
TEST_CASE(test_open_ro_then_read),
TEST_CASE(test_open_wo_then_read),
TEST_CASE(test_open_rw_then_read)
};
int main(int argc, const char* argv[]) {
return test_suite_run(test_cases, NUM_TEST_CASES);
}

@ -7,8 +7,8 @@
#include <errno.h>
int main(int argc, const char* argv[]) {
const char* file_name = "tmp.txt";
const char* link_name = "link.txt";
const char* file_name = "/root/test_filesystem_link.txt";
const char* link_name = "/root/link.txt";
const char* write_msg = "Hello World\n";
char read_buf[128] = {0};
int ret;

@ -23,8 +23,10 @@ int main(int argc, const char* argv[]) {
return -1;
}
const char DIR_NAME[] = "test_dir";
const char DIR_PATH[] = "/test_dir";
//const char DIR_NAME[] = "test_dir";
//const char DIR_PATH[] = "/test_dir";
const char DIR_NAME[] = "/root/test_dir";
const char DIR_PATH[] = "/root/test_dir";
const int DIR_MODE = 0664;
ret = mkdir(DIR_NAME, DIR_MODE);
if(ret < 0) {

@ -254,7 +254,7 @@ int test_anonymous_mmap_with_non_page_aligned_len() {
// ============================================================================
int test_file_mmap() {
const char* file_path = "mmap_file.data";
const char* file_path = "/root/mmap_file.data";
int fd = open(file_path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
if (fd < 0) {
throw_error("file creation failed");
@ -292,7 +292,7 @@ int test_file_mmap() {
}
int test_file_mmap_with_offset() {
const char* file_path = "mmap_file.data";
const char* file_path = "/root/mmap_file.data";
int fd = open(file_path, O_CREAT | O_TRUNC | O_RDWR, 0644);
if (fd < 0) {
throw_error("file creation failed");
@ -352,7 +352,7 @@ int test_file_mmap_with_invalid_fd() {
}
int test_file_mmap_with_non_page_aligned_offset() {
const char* file_path = "mmap_file.data";
const char* file_path = "/root/mmap_file.data";
int fd = open(file_path, O_CREAT | O_TRUNC | O_RDWR, 0644);
if (fd < 0) {
throw_error("file creation failed");

@ -6,7 +6,7 @@
#include <stdio.h>
int main(int argc, const char* argv[]) {
const char* FILE_NAME = "tmp.txt";
const char* FILE_NAME = "root/test_filesystem_truncate.txt";
const int TRUNC_LEN = 256;
const int TRUNC_LEN1 = 128;
const int MODE_MASK = 0777;

@ -95,6 +95,8 @@ cmd_build() {
cp "$occlum_dir/build/lib/libcompiler-rt-patch.a" build/lib/
mkdir -p build/src/libos/src/builtin
chmod 531 -R $working_dir/image/bin
chmod 531 -R $working_dir/image/lib
mkdir -p build/mount/
cd "$occlum_dir/deps/sefs/sefs-fuse/bin/" && \
./app \