From 72cd9b4c312e728372044fdef29a3ee9691ecd43 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 11 Jan 2023 13:53:14 -0700 Subject: [PATCH] More verification cleanup, and changing of FS rule structure --- Cargo.toml | 1 - src/compile.rs | 96 ++++++++++++--------------------------------- src/internal_rep.rs | 96 ++++++++++++++++++++++++++++++++------------- 3 files changed, 94 insertions(+), 99 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9e287340..c00295f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,6 @@ atty = "0.2" backtrace = "0.3" clap = { version = "4", features = ["derive"] } codespan-reporting = "0.11" -derivative = "2.2.0" flate2 = "1" lalrpop-util = "0.19" regex = "1" diff --git a/src/compile.rs b/src/compile.rs index b545cfbb..21afc749 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -5,7 +5,6 @@ use std::borrow::Cow; use std::collections::hash_map::Entry; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::convert::TryFrom; -use std::ops::Range; use crate::ast::{ Argument, CascadeString, Declaration, Expression, FuncCall, LetBinding, Machine, Module, @@ -348,32 +347,25 @@ pub fn build_func_map<'a>( } pub fn validate_fs_context_duplicates( - fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>>, + fsc_rules: BTreeMap>, ) -> Result<(), CascadeErrors> { let mut errors = CascadeErrors::new(); - 'key_loop: for (_, v) in fsc_rules { + 'key_loop: for v in fsc_rules.values() { // We only have 1 or 0 elements, thus we cannot have a semi duplicate if v.len() <= 1 { continue; } let mut error: Option = None; - for rule in &v { + for rule in v { match rule.fscontext_type { // If we ever see a duplicate of xattr task or trans we know something is wrong FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => { - let mut range: Range = 1..1; - for arg in &rule.func_call.args { - if let Argument::Quote(cas_arg) = &arg.0 { - if *cas_arg == rule.fs_name { - range = cas_arg.get_range().unwrap_or_default(); - } - } - } error = Some(add_or_create_compile_error(error, "Duplicate filesystem context.", &rule.file, - range, + // TODO once Issue #92 is resolved we would rather do a unwrap_or make internal error + rule.fs_name.get_range().unwrap_or_default(), &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); } FSContextType::GenFSCon => { @@ -381,89 +373,48 @@ pub fn validate_fs_context_duplicates( // If we find a genfscon with the same path, they must have the same context and object type. if let Some(path) = &rule.path { // Look through the rules again - for inner_rule in &v { + for inner_rule in v { // Only check path if it was provided as part of the rule if let Some(inner_path) = &inner_rule.path { // If our paths match, check if our contexts match if path == inner_path && rule.context != inner_rule.context { - // In this case we are safe in knowing args[0] is what we need. - let inner_range: Option> = - inner_rule.func_call.args[0].0.get_range(); // error is not None so we have already found something, so we just // need to add a new error message if let Some(unwrapped_error) = error { error = Some(unwrapped_error.add_additional_message(&inner_rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - inner_range.unwrap_or_else(||inner_rule.func_call.get_name_range().unwrap()), + inner_rule.context_range.clone(), &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context))); } else { - let outer_range: Option> = - rule.func_call.args[0].0.get_range(); // error is none so we need to make a new one - error = Some(CompileError::new( + let tmp_error = CompileError::new( "Duplicate genfscon contexts", &rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - outer_range.unwrap_or_else(||rule.func_call.get_name_range().unwrap()), - &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", rule.fs_name, rule.context))); - // We just made the error we are okay to unwrap it - error = Some(error.unwrap().add_additional_message(&inner_rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - inner_range.unwrap_or_else(||inner_rule.func_call.get_name_range().unwrap()), + rule.context_range.clone(), + &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", rule.fs_name, rule.context)); + error = Some(tmp_error.add_additional_message(&inner_rule.file, + inner_rule.context_range.clone(), &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context))); } - // Our paths are the same but our file types differ, we must also have a file type. + // Our paths are the same but our file types differ. We must also have a file type. } else if path == inner_path && rule.file_type != inner_rule.file_type && rule.file_type.is_some() { - let mut inner_range: Option> = None; - // We need to look through the function's args to get the range on the file type arg we are looking for - for arg in &inner_rule.func_call.args { - if let Argument::List(cas_args) = &arg.0 { - for cas_arg in cas_args { - // unwrap is safe here due to else if above - if *cas_arg - == inner_rule.file_type.unwrap().to_string() - { - inner_range = cas_arg.get_range(); - } - } - } - } // error is not None so we have already found something, so we just // need to add a new error message if let Some(unwrapped_error) = error { error = Some(unwrapped_error.add_additional_message(&inner_rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - inner_range.unwrap_or_else(||inner_rule.func_call.get_name_range().unwrap()), + inner_rule.file_type_range.clone(), &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); } else { - // Just like above we need to look through the outer function's args to get the correct range - let mut outer_range: Option> = None; - for arg in &rule.func_call.args { - if let Argument::List(cas_args) = &arg.0 { - for cas_arg in cas_args { - // unwrap is safe here to else if above - if *cas_arg - == rule.file_type.unwrap().to_string() - { - outer_range = cas_arg.get_range(); - } - } - } - } // error is none so we need to make a new one. - error = Some(CompileError::new( + let tmp_error = CompileError::new( "Duplicate genfscon file types", - &inner_rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - outer_range.unwrap_or_else(||rule.func_call.get_name_range().unwrap()), - &format!("Found duplicate genfscon rules for filesystem {} with differing file types", rule.fs_name))); - // We just made the error we are safe to unwrap - error = Some(error.unwrap().add_additional_message(&inner_rule.file, - // The unwrap in the unwrap_or is safe since we know we have a function call w/ range at this point - inner_range.unwrap_or_else(||inner_rule.func_call.get_name_range().unwrap()), + &rule.file, + rule.file_type_range.clone(), + &format!("Found duplicate genfscon rules for filesystem {} with differing file types", rule.fs_name)); + error = Some(tmp_error.add_additional_message(&inner_rule.file, + inner_rule.file_type_range.clone(), &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); } } @@ -491,11 +442,14 @@ pub fn validate_fs_context_duplicates( pub fn validate_rules(statements: &BTreeSet) -> Result<(), CascadeErrors> { let mut errors = CascadeErrors::new(); - let mut fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>> = BTreeMap::new(); + let mut fsc_rules: BTreeMap> = BTreeMap::new(); for statement in statements { // Add all file system context rules to a new map to check for semi duplicates later if let ValidatedStatement::FscRule(fs) = statement { - fsc_rules.entry(&fs.fs_name).or_default().insert(fs); + fsc_rules + .entry(fs.fs_name.to_string()) + .or_default() + .insert(fs); } } diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 9486617c..75e2f5a1 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -21,9 +21,6 @@ use crate::context::{BlockType, Context as BlockContext}; use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; use crate::obj_class::perm_list_to_sexp; -extern crate derivative; -use derivative::Derivative; - const DEFAULT_USER: &str = "system_u"; const DEFAULT_OBJECT_ROLE: &str = "object_r"; const DEFAULT_DOMAIN_ROLE: &str = "system_r"; @@ -1594,25 +1591,54 @@ impl FromStr for FSContextType { } } -#[derive(Derivative)] -#[derivative(Clone, Debug, PartialEq, PartialOrd, Ord)] +#[derive(Clone, Debug)] pub struct FileSystemContextRule<'a> { pub fscontext_type: FSContextType, - pub fs_name: String, - pub path: Option, + pub fs_name: CascadeString, + pub path: Option, pub file_type: Option, pub context: Context<'a>, - #[derivative(PartialEq = "ignore")] - #[derivative(PartialOrd = "ignore")] - #[derivative(Ord = "ignore")] pub file: SimpleFile, - #[derivative(PartialEq = "ignore")] - #[derivative(PartialOrd = "ignore")] - #[derivative(Ord = "ignore")] - pub func_call: FuncCall, + pub file_type_range: Range, //Note: if a file type is not given this will be the range of the function name + pub context_range: Range, } impl Eq for FileSystemContextRule<'_> {} +impl PartialEq for FileSystemContextRule<'_> { + fn eq(&self, other: &Self) -> bool { + self.fscontext_type == other.fscontext_type + && self.fs_name == other.fs_name + && self.path == other.path + && self.file_type == other.file_type + && self.context == other.context + } +} + +impl PartialOrd for FileSystemContextRule<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for FileSystemContextRule<'_> { + fn cmp(&self, other: &Self) -> Ordering { + ( + &self.fscontext_type, + &self.fs_name, + &self.path, + self.file_type, + &self.context, + ) + .cmp(&( + &other.fscontext_type, + &other.fs_name, + &other.path, + other.file_type, + &other.context, + )) + } +} + impl FileSystemContextRule<'_> { fn get_renamed_statement(&self, renames: &BTreeMap) -> Self { FileSystemContextRule { @@ -1622,7 +1648,8 @@ impl FileSystemContextRule<'_> { file_type: self.file_type, context: self.context.get_renamed_context(renames), file: self.file.clone(), - func_call: self.func_call.clone(), + file_type_range: self.file_type_range.clone(), + context_range: self.context_range.clone(), } } } @@ -1635,7 +1662,7 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => Ok(list(&[ atom_s("fsuse"), Sexp::Atom(Atom::S(f.fscontext_type.to_string())), - atom_s(f.fs_name.trim_matches('"')), + atom_s(f.fs_name.to_string().trim_matches('"')), Sexp::from(&f.context), ])), FSContextType::GenFSCon => { @@ -1649,7 +1676,7 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { if false { return Ok(list(&[ atom_s("genfscon"), - atom_s(f.fs_name.trim_matches('"')), + atom_s(f.fs_name.to_string().trim_matches('"')), atom_s(p.as_ref()), Sexp::Atom(Atom::S(file_type.to_string())), Sexp::from(&f.context), @@ -1660,7 +1687,7 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { // reduce redundant lines of code Ok(list(&[ atom_s("genfscon"), - atom_s(f.fs_name.trim_matches('"')), + atom_s(f.fs_name.to_string().trim_matches('"')), atom_s(p.as_ref()), Sexp::from(&f.context), ])) @@ -1739,10 +1766,10 @@ fn call_to_fsc_rules<'a>( let mut args_iter = validated_args.iter(); let mut ret = Vec::new(); - let context_str = args_iter + let context_str_arg = args_iter .next() - .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? - .get_name_or_string(context)?; + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))?; + let context_str = context_str_arg.get_name_or_string(context)?; let fs_context = match Context::try_from(context_str.to_string()) { Ok(c) => c, Err(_) => { @@ -1756,11 +1783,13 @@ fn call_to_fsc_rules<'a>( )) } }; + + // BUG: There is an issue with get_name_or_string in which the range of the + // returned CascadeString will be None. let fs_name = args_iter .next() .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? - .get_name_or_string(context)? - .to_string(); + .get_name_or_string(context)?; let fscontext_str = args_iter .next() .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? @@ -1781,7 +1810,8 @@ fn call_to_fsc_rules<'a>( let regex_string_arg = args_iter .next() .ok_or_else(|| ErrorItem::Internal(InternalError::new()))?; - let regex_string = regex_string_arg.get_name_or_string(context)?.to_string(); + let regex_string = regex_string_arg.get_name_or_string(context)?; + let file_types_arg = args_iter .next() .ok_or_else(|| ErrorItem::Internal(InternalError::new()))?; @@ -1801,7 +1831,11 @@ fn call_to_fsc_rules<'a>( file_type: None, context: fs_context.clone(), file: file.clone(), - func_call: c.clone(), + // file_type_range shouldn't ever be used for xattr, task, or trans but I would rather not + // have to deal with Option/unwrap stuff later + file_type_range: c.get_name_range().unwrap_or_default(), + // TODO once Issue #92 is resolved we would rather do a unwrap_or make internal error + context_range: context_str_arg.get_range().unwrap_or_default(), }); } let mut errors = CascadeErrors::new(); @@ -1838,7 +1872,11 @@ fn call_to_fsc_rules<'a>( file_type: None, context: fs_context.clone(), file: file.clone(), - func_call: c.clone(), + // file_type_range shouldn't need to be used here since file_type is None, but I would rather not + // have to deal with Option/unwrap stuff later + file_type_range: c.get_name_range().unwrap_or_default(), + // TODO once Issue #92 is resolved we would rather do a unwrap_or make internal error + context_range: context_str_arg.get_range().unwrap_or_default(), }); } else { for file_type in file_types { @@ -1863,7 +1901,11 @@ fn call_to_fsc_rules<'a>( file_type: Some(file_type), context: fs_context.clone(), file: file.clone(), - func_call: c.clone(), + file_type_range: file_types_arg + .get_range() + .unwrap_or_else(|| c.get_name_range().unwrap_or_default()), + // TODO once Issue #92 is resolved we would rather do a unwrap_or make internal error + context_range: context_str_arg.get_range().unwrap_or_default(), }); } }