Commit 31bb5d9f authored by Alexis Janon's avatar Alexis Janon
Browse files

forbid writes and reads to the root cgroup or cuttr's cgroups

Change in the check_path interface: there are now two different methods,
check_file and check_dir.
check_dir is the previous check_path.
check_file checks that the file is not inside a root cgroup and it also
checks that it is not in one of cuttr's cgroups. This is to make sure
that a user cannot make any changes to the values which are not his.
parent 2b852d77
Pipeline #4069 failed with stage
in 2 minutes and 25 seconds
......@@ -7,6 +7,7 @@ use std::ops::Deref;
use std::path;
const PROC_CGROUPS: &str = "/proc/cgroups";
const PROC_SELF_CGROUP: &str = "/proc/self/cgroup";
const MOUNTINFO_PATHS: &[&str] = &["/proc/mounts", "/proc/self/mounts", "/etc/mtab"];
......@@ -26,7 +27,10 @@ pub enum SysFsType {
}
#[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
pub struct SysFsControllerList(HashMap<String, SysFsController>);
pub struct SysFsControllerList {
list: HashMap<String, SysFsController>,
own: Option<Box<SysFsControllerList>>,
}
#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Serialize, Deserialize)]
pub struct SysFsController {
......@@ -36,7 +40,10 @@ pub struct SysFsController {
impl SysFsControllerList {
pub fn new() -> Self {
SysFsControllerList(HashMap::new())
SysFsControllerList {
list: HashMap::new(),
own: None,
}
}
pub fn find_path(&self, slice: &str, key: &str) -> Result<path::PathBuf> {
......@@ -60,31 +67,55 @@ impl SysFsControllerList {
})
}
pub fn check_path(&self, path: &path::Path) -> Result<CommandResult> {
// inspired by https://github.com/vitiral/path_abs
let mut absolute_path = path::PathBuf::from("/");
for component in path.components() {
match component {
path::Component::ParentDir => if !absolute_path.pop() {
return Err(Error::from(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("Could not access {}: permission denied", path.display()),
)));
},
path::Component::Normal(_) => absolute_path.push(&component),
_ => (),
}
pub fn check_dir(&self, path: &path::Path) -> Result<CommandResult> {
let abs_path = absolute_path(path)?;
match self.is_path_in_controller(&abs_path) {
false => Err(Error::from(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("Cannot access {}: permission denied", path.display()),
))),
true => Ok(CommandResult::from(())),
}
match self.is_authorized_path(&absolute_path) {
}
pub fn check_file(&self, path: &path::Path) -> Result<CommandResult> {
let abs_path = absolute_path(path)?;
match self.is_path_in_controller(&abs_path)
&& !self.is_in_own_cgroup(&abs_path)
&& !self.is_in_root_controller(&abs_path)
{
false => Err(Error::from(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("Could not access {}: permission denied", path.display()),
format!("Cannot access {}: permission denied", path.display()),
))),
true => Ok(CommandResult::from(())),
}
}
pub fn is_authorized_path(&self, path: &path::Path) -> bool {
fn is_in_own_cgroup(&self, path: &path::Path) -> bool {
match path.parent() {
Some(parent) => match self.own {
Some(ref own) => own
.values()
.map(|controller| &controller.path)
.any(|cpath| parent == cpath),
None => false,
},
None => false,
}
}
fn is_in_root_controller(&self, path: &path::Path) -> bool {
match path.parent() {
Some(parent) => self
.values()
.map(|controller| &controller.path)
.any(|cpath| parent == cpath),
None => false,
}
}
fn is_path_in_controller(&self, path: &path::Path) -> bool {
self.values()
.map(|controller| &controller.path)
.any(|cpath| path.starts_with(cpath))
......@@ -93,7 +124,29 @@ impl SysFsControllerList {
impl From<HashMap<String, SysFsController>> for SysFsControllerList {
fn from(map: HashMap<String, SysFsController>) -> Self {
SysFsControllerList(map)
SysFsControllerList {
list: map,
own: None,
}
}
}
impl
From<(
HashMap<String, SysFsController>,
HashMap<String, SysFsController>,
)> for SysFsControllerList
{
fn from(
maps: (
HashMap<String, SysFsController>,
HashMap<String, SysFsController>,
),
) -> Self {
SysFsControllerList {
list: maps.0,
own: Some(Box::new(SysFsControllerList::from(maps.1))),
}
}
}
......@@ -101,7 +154,7 @@ impl Deref for SysFsControllerList {
type Target = HashMap<String, SysFsController>;
fn deref(&self) -> &HashMap<String, SysFsController> {
&self.0
&self.list
}
}
......@@ -123,6 +176,24 @@ impl SysFsController {
}
}
fn absolute_path(path: &path::Path) -> Result<path::PathBuf> {
// inspired by https://github.com/vitiral/path_abs
let mut absolute_path = path::PathBuf::from("/");
for component in path.components() {
match component {
path::Component::ParentDir => if !absolute_path.pop() {
return Err(Error::from(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("Could not access {}: permission denied", path.display()),
)));
},
path::Component::Normal(_) => absolute_path.push(&component),
_ => (),
}
}
Ok(absolute_path)
}
fn parse_cgroups(mountinfo_contents: &str) -> io::Result<SysFsControllerList> {
let cgroups_contents = fs::read_to_string(PROC_CGROUPS)?;
let cgroups = cgroups_contents
......@@ -130,8 +201,8 @@ fn parse_cgroups(mountinfo_contents: &str) -> io::Result<SysFsControllerList> {
.map(|s| s.split_whitespace().next().unwrap())
.collect::<Vec<_>>();
Ok(SysFsControllerList(
mountinfo_contents
Ok(SysFsControllerList {
list: mountinfo_contents
.lines()
.filter(|s| s.starts_with("cgroup") && !s.starts_with("cgroup2"))
.filter_map(|s| s.split_whitespace().nth(1))
......@@ -143,13 +214,14 @@ fn parse_cgroups(mountinfo_contents: &str) -> io::Result<SysFsControllerList> {
SysFsController::new(path::PathBuf::from(path), SysFsType::Cgroup),
)
}).collect(),
))
own: None,
})
}
fn parse_mountinfo(mountinfo_contents: &str) -> io::Result<SysFsControllerList> {
// TODO: cleanup
Ok(SysFsControllerList(
mountinfo_contents
Ok(SysFsControllerList {
list: mountinfo_contents
.lines()
.filter(|&s| s.starts_with("cgroup2"))
.filter_map(|s| s.split_whitespace().nth(1))
......@@ -170,7 +242,31 @@ fn parse_mountinfo(mountinfo_contents: &str) -> io::Result<SysFsControllerList>
)
}),
).collect(),
))
own: None,
})
}
fn parse_self_cgroup(controllers: &SysFsControllerList) -> io::Result<SysFsControllerList> {
let self_cgroup_contents = fs::read_to_string(PROC_SELF_CGROUP)?;
Ok(SysFsControllerList {
list: self_cgroup_contents
.lines()
.map(|s| s.split(':'))
.map(|iter| iter.collect::<Vec<_>>())
.map(|vec| (vec[1], vec[2]))
.filter_map(|(cname, cpath)| {
controllers.get(cname).map(|controller| {
(
cname.to_owned(),
SysFsController::new(
controller.path.join(cpath),
controller.fs_type.clone(),
),
)
})
}).collect(),
own: None,
})
}
pub fn get_sys_fs_controllers() -> Result<SysFsControllerList> {
......@@ -178,8 +274,9 @@ pub fn get_sys_fs_controllers() -> Result<SysFsControllerList> {
let mountinfo_contents = fs::read_to_string(MOUNTINFO_PATHS[0])?;
let mut controllers = parse_mountinfo(&mountinfo_contents)?;
controllers
.0
.extend((parse_cgroups(&mountinfo_contents)?).0);
.list
.extend((parse_cgroups(&mountinfo_contents)?).list);
controllers.own = Some(Box::new(parse_self_cgroup(&controllers)?));
Ok(controllers)
}
......@@ -196,10 +293,14 @@ mod tests {
const NO_KEY_NAME: &str = "test_find_no_key";
const DUPLICATE_NAME: &str = "test_find_duplicate";
const KEY: &str = "test1.prop";
const INVALID_PATH_SIMPLE: &str = "../test_check_invalid";
const VALID_PATH_SIMPLE: &str = "test_check_valid";
const VALID_PATH_COMPLEX: &str =
const INVALID_DIR_SIMPLE: &str = "../test_check_invalid";
const VALID_DIR_SIMPLE: &str = "test_check_valid";
const VALID_DIR_COMPLEX: &str =
"../fake_parent/../fake_grandparent/fake_parent/../../tmp/test_check_valid_complex/fake_child/..";
const INVALID_FILE_ROOT: &str = "/tmp/test_check_invalid_file_simple";
const INVALID_FILE_OWN: &str = "/tmp/cgroup/test_check_invalid_file_simple";
const VALID_FILE_SIMPLE: &str = "/tmp/cgroup/test_check_invalid_file_simple";
const VALID_FILE_OWN: &str = "/tmp/cgroup/test_check_invalid_file_simple";
#[test]
fn test_controllers() {
......@@ -229,7 +330,7 @@ mod tests {
.open(&filepath)
.is_ok()
);
let controllers = SysFsControllerList(map);
let controllers = SysFsControllerList::from(map);
// Test
assert_eq!(controllers.find_path(FIND_NAME, KEY).unwrap(), filepath);
// Cleanup
......@@ -260,7 +361,7 @@ mod tests {
.open(&filepath)
.is_ok()
);
let controllers = SysFsControllerList(map);
let controllers = SysFsControllerList::from(map);
// Test
assert!(controllers.find_path(NO_KEY_NAME, "no_key").is_err());
// Cleanup
......@@ -301,7 +402,7 @@ mod tests {
.is_ok()
);
}
let controllers = SysFsControllerList(map);
let controllers = SysFsControllerList::from(map);
// Test
assert!(controllers.find_path(DUPLICATE_NAME, KEY).is_err());
// Cleanup
......@@ -318,7 +419,7 @@ mod tests {
}
#[test]
fn test_invalid_simple() {
fn test_invalid_dir_simple() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
......@@ -327,13 +428,28 @@ mod tests {
let controllers = SysFsControllerList::from(map);
assert!(
controllers
.check_path(&path::Path::new(&format!("/tmp/{}", INVALID_PATH_SIMPLE)))
.check_dir(&path::Path::new(&format!("/tmp/{}", INVALID_DIR_SIMPLE)))
.is_err()
);
}
#[test]
fn test_valid_simple() {
fn test_valid_dir_simple() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/"), SysFsType::Cgroup),
);
let controllers = SysFsControllerList::from(map);
assert!(
controllers
.check_dir(&path::Path::new(&format!("/tmp/{}", VALID_DIR_SIMPLE)))
.is_ok()
);
}
#[test]
fn test_valid_dir_complex() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
......@@ -342,13 +458,48 @@ mod tests {
let controllers = SysFsControllerList::from(map);
assert!(
controllers
.check_path(&path::Path::new(&format!("/tmp/{}", VALID_PATH_SIMPLE)))
.check_dir(&path::Path::new(&format!("/tmp/{}", VALID_DIR_COMPLEX)))
.is_ok()
);
}
#[test]
fn test_valid_complex() {
fn test_invalid_file_root() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/"), SysFsType::Cgroup),
);
let controllers = SysFsControllerList::from(map);
assert!(
controllers
.check_file(&path::Path::new(&INVALID_FILE_ROOT))
.is_err()
);
}
#[test]
fn test_invalid_file_own() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/"), SysFsType::Cgroup),
);
let mut own = HashMap::new();
own.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/cgroup"), SysFsType::Cgroup),
);
let controllers = SysFsControllerList::from((map, own));
assert!(
controllers
.check_file(&path::Path::new(&INVALID_FILE_OWN))
.is_err()
);
}
#[test]
fn test_valid_file_simple() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
......@@ -357,7 +508,27 @@ mod tests {
let controllers = SysFsControllerList::from(map);
assert!(
controllers
.check_path(&path::Path::new(&format!("/tmp/{}", VALID_PATH_COMPLEX)))
.check_file(&path::Path::new(&VALID_FILE_SIMPLE))
.is_ok()
);
}
#[test]
fn test_valid_file_own() {
let mut map = HashMap::new();
map.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/"), SysFsType::Cgroup),
);
let mut own = HashMap::new();
own.insert(
"tmp".to_owned(),
SysFsController::new(path::PathBuf::from("/tmp/other_cgroup"), SysFsType::Cgroup),
);
let controllers = SysFsControllerList::from((map, own));
assert!(
controllers
.check_file(&path::Path::new(&VALID_FILE_OWN))
.is_ok()
);
}
......
......@@ -21,7 +21,7 @@ impl CreateDir {
impl Command for CreateDir {
fn exec(&mut self) -> Result<CommandResult> {
debug!("Creating directory {}", self.path.display());
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_dir(&self.path) {
return Err(err);
}
match fs::create_dir(&self.path) {
......@@ -45,7 +45,7 @@ impl Command for CreateDir {
fn rollback(&mut self) -> Result<CommandResult> {
debug!("{{rollback}} Removing directory {}", self.path.display());
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_dir(&self.path) {
return Err(err);
}
match fs::remove_dir(&self.path) {
......
......@@ -59,7 +59,7 @@ impl Command for FindGetSetValue {
Some(ref value) => value,
None => return Err(Error::from(format!("Missing previous path value to key {} for slice {}. (probably an internal bug, you should report it)", self.name, self.key)))
};
if let Err(err) = self.controllers.check_path(&path) {
if let Err(err) = self.controllers.check_file(&path) {
return Err(err);
}
trace!("Reading from file {}", path.display());
......@@ -114,7 +114,7 @@ impl Command for FindGetSetValue {
}
}
};
if let Err(err) = self.controllers.check_path(&path) {
if let Err(err) = self.controllers.check_file(&path) {
return Err(err);
}
debug!(
......
......@@ -25,7 +25,7 @@ impl GetSetValue {
impl Command for GetSetValue {
fn exec(&mut self) -> Result<CommandResult> {
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_file(&self.path) {
return Err(err);
}
if !self.path.exists() {
......@@ -60,7 +60,7 @@ impl Command for GetSetValue {
}
fn rollback(&mut self) -> Result<CommandResult> {
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_file(&self.path) {
return Err(err);
}
debug!("{{rollback}} Reading and writing {}", self.path.display());
......
......@@ -22,7 +22,7 @@ impl RemoveDir {
impl Command for RemoveDir {
fn exec(&mut self) -> Result<CommandResult> {
debug!("Removing directory {}", self.path.display());
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_dir(&self.path) {
return Err(err);
}
match fs::remove_dir(&self.path) {
......@@ -47,7 +47,7 @@ impl Command for RemoveDir {
fn rollback(&mut self) -> Result<CommandResult> {
debug!("{{rollback}} Creating directory {}", self.path.display());
if let Err(err) = self.controllers.check_path(&self.path) {
if let Err(err) = self.controllers.check_dir(&self.path) {
return Err(err);
}
match fs::create_dir(&self.path) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment