From 8e3c47322934cf90e230fa9ea14385338dd4f71f Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Fri, 1 Sep 2023 15:22:04 -0400 Subject: [PATCH 1/7] Don't have create_synthetic_resource copy expressions There are three cases here: * function declarations are handled via derive. The existing behavior could be wrong in some cases, such as the presence of noderive * type declarations are illegal inside resources * rules are handled via normal Cascade inheritance mechanisms So the copying is mostly unneeded, and wrong in the noderive case. Additionally, it makes multiple inheritance challenging. --- .../virtual_function_association.cas | 8 +++++++- data/expected_cil/nested_alias.cil | 3 ++- data/policies/nested_alias.cas | 10 ++++++++-- src/compile.rs | 18 +++--------------- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/data/error_policies/virtual_function_association.cas b/data/error_policies/virtual_function_association.cas index 1b54521f..5df8fc72 100644 --- a/data/error_policies/virtual_function_association.cas +++ b/data/error_policies/virtual_function_association.cas @@ -4,7 +4,13 @@ virtual resource tmp { } @associate([tmp]) -virtual domain foo {} +virtual domain foo { + extend tmp { + fn read(domain source) { + allow(source, this, file, read); + } + } +} domain bar inherits foo { diff --git a/data/expected_cil/nested_alias.cil b/data/expected_cil/nested_alias.cil index 9e321b15..6ae0781e 100644 --- a/data/expected_cil/nested_alias.cil +++ b/data/expected_cil/nested_alias.cil @@ -164,8 +164,9 @@ (typeattributeset resource (bar-tmp)) (typealias zap) (typealiasactual zap bar-tmp) -(macro bar-tmp-read ((type this) (type source))) +(macro bar-tmp-read ((type this) (type source)) (allow source this (file (read)))) (macro zap-read ((type this) (type source)) (call bar-tmp-read (this source))) +(macro foo-tmp-read ((type this) (type source)) (allow source this (file (read)))) (allow abc bob (file (read))) (allow abc zap (file (read))) (allow bar bob (file (read))) diff --git a/data/policies/nested_alias.cas b/data/policies/nested_alias.cas index 49baf2fc..52c89bc6 100644 --- a/data/policies/nested_alias.cas +++ b/data/policies/nested_alias.cas @@ -4,7 +4,13 @@ virtual resource tmp { } @associate([tmp]) -virtual domain foo {} +virtual domain foo { + extend tmp { + fn read(domain source) { + allow(source, this, file, read); + } + } +} domain bar inherits foo { allow(this, bob, file, [read]); @@ -20,4 +26,4 @@ domain abc { @alias(bob) extend xyz {} -} \ No newline at end of file +} diff --git a/src/compile.rs b/src/compile.rs index 2019623d..04f068c8 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -687,9 +687,9 @@ pub fn validate_functions<'a>( // Validate that all required functions exist for setype in types.values() { - for parent in &setype.inherits { + for parent in &setype.get_all_parent_names(types) { for required_function_name in classes_to_required_functions - .get(&parent) + .get(parent) .unwrap_or(&BTreeSet::new()) { if !setype.defines_function(required_function_name, &functions) { @@ -1595,7 +1595,6 @@ fn create_synthetic_resource( dup_res_decl.inherits = parent_names; // Virtual resources become concrete when associated to concrete types dup_res_decl.is_virtual = dup_res_decl.is_virtual && dom_info.is_virtual; - let dup_res_is_virtual = dup_res_decl.is_virtual; // The synthetic resource keeps some, but not all annotations from its parent. // Specifically, Makelist and derive are kept from the parent // TODO: This would be cleaner if we convert to AnnotationInfos first and implent the logic as @@ -1606,19 +1605,8 @@ fn create_synthetic_resource( .annotations .retain(|a| a.name.as_ref() == "makelist" || a.name.as_ref() == "derive"); - dup_res_decl - .expressions - .iter_mut() - .for_each(|e| e.set_class_name_if_decl(res_name.clone())); + dup_res_decl.expressions = Vec::new(); - dup_res_decl - .expressions - // If dup_res_decl is concrete, do not inherit virtual functions - // Never inherit statements - .retain(|e| { - (dup_res_is_virtual || !e.is_virtual_function()) - && matches!(e, Expression::Decl(Declaration::Func(_))) - }); if !global_exprs.insert(Expression::Decl(Declaration::Type(Box::new( dup_res_decl.clone(), )))) { From 528272b07e65d68653f33d3d1bf41994c038fc9c Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Mon, 17 Jul 2023 10:08:51 -0400 Subject: [PATCH 2/7] Rewrite association deduplication to handle the resource and domain parts seperately The motivation here comes from combining similar associations, such as two different nested associations of the same resource from different parents, or a nested combined with an annotated association. We need to treat associations of the same resource as fundamentally the same, but having multiple parents, which are the different specific resources associated. At the time we are setting up association, there is always one resource that has already been added to the TypeMap for each association. For annotated associations, this is an explicitly declared resource of the resource name. For nested ones, the nested declaration declares a "dom.res" resource. When we later process annotations, we don't care whether "res" exists in this case. So we track these "real" resources, and build our annotations based on them. --- src/compile.rs | 129 ++++++++++++++++++++++++-------------------- src/internal_rep.rs | 106 ++++++++++++++++++++++++++++++++---- 2 files changed, 165 insertions(+), 70 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index 04f068c8..48d64a7e 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -177,7 +177,7 @@ pub fn extend_type_map( Ok(ww) => { let new_type = ww.inner(&mut warnings); type_map.insert(nested_name.to_string(), new_type)?; - associated_resources.insert(nested_name.clone()); + associated_resources.insert((&nested_name).into()); } Err(e) => errors.append(e), } @@ -191,7 +191,7 @@ pub fn extend_type_map( "").into()); } let ann_to_insert = AnnotationInfo::NestAssociate(Associated { - resources: BTreeSet::from([nested_name.clone()]), + resources: BTreeSet::from([(&nested_name).into()]), }); let annotations = ret.entry(t.name.clone()).or_insert_with(BTreeSet::new); annotations.insert(ann_to_insert); @@ -219,7 +219,7 @@ pub fn extend_type_map( let mut new_type = new_type.inner(&mut warnings); new_type .associated_resources - .append(&mut associated_resources); + .append(&mut (associated_resources).into()); type_map.insert(t.name.to_string(), new_type)?; } Err(e) => errors.append(e), @@ -1539,11 +1539,14 @@ fn create_synthetic_resource( types: &TypeMap, dom_info: &TypeInfo, associated_parents: &Vec<&TypeInfo>, - class: &TypeInfo, + classes: Vec<&TypeInfo>, class_string: &CascadeString, global_exprs: &mut HashSet, extend_annotations: &BTreeMap>, ) -> Result { + // DO NOT MERGE + // Just use first class to get things working while in development + let class = classes[0]; if !class.is_resource(types) { return Err(ErrorItem::make_compile_or_internal_error( "not a resource", @@ -1683,52 +1686,57 @@ fn interpret_associate( let potential_resources: BTreeMap<_, _> = associate .resources .iter() - .map(|r| (r.as_ref(), (r, false))) + .map(|r| (r.name.as_ref(), (r, false))) .collect(); for (_, (res, _)) in potential_resources.iter().filter(|(_, (_, seen))| !seen) { - match types.get(res.as_ref()) { - Some(class) => { - match create_synthetic_resource( - types, - dom_info, - associated_parents, - class, - res, - global_exprs, - extend_annotations, - ) { - Ok(_) => { - // If the associated call is derived, we'll need to add it in later. If - // the association is inherited, we need to make sure to mark it now so we - // know to do that. - let class_name = CascadeString::from(class.basename()); - global_exprs.insert(Expression::Decl(Declaration::Type(Box::new( - TypeDecl { - name: dom_info.name.clone(), - inherits: Vec::new(), - is_virtual: dom_info.is_virtual, - is_trait: dom_info.is_trait, - is_extension: true, - expressions: Vec::new(), - annotations: Annotations { - annotations: vec![Annotation { - name: "associate".into(), - arguments: vec![Argument::List(vec![class_name])], - }], - }, - }, - )))); - } - Err(e) => errors.add_error(e), + let mut classes = Vec::new(); + for real_parent_resource in res.get_class_names() { + match types.get(&real_parent_resource) { + Some(class) => classes.push(class), + None => { + errors.add_error(ErrorItem::make_compile_or_internal_error( + "unknown resource", + dom_info.declaration_file.as_ref(), + res.get_range(), + "didn't find this resource in the policy", + )) } } - None => errors.add_error(ErrorItem::make_compile_or_internal_error( - "unknown resource", - dom_info.declaration_file.as_ref(), - res.get_range(), - "didn't find this resource in the policy", - )), + } + + errors = errors.into_result_self()?; + + match create_synthetic_resource( + types, + dom_info, + associated_parents, + classes, + &res.basename().into(), + global_exprs, + extend_annotations, + ) { + Ok(_) => { + // If the associated call is derived, we'll need to add it in later. If + // the association is inherited, we need to make sure to mark it now so we + // know to do that. + let class_name = CascadeString::from(res.basename()); + global_exprs.insert(Expression::Decl(Declaration::Type(Box::new(TypeDecl { + name: dom_info.name.clone(), + inherits: Vec::new(), + is_virtual: dom_info.is_virtual, + is_trait: dom_info.is_trait, + is_extension: true, + expressions: Vec::new(), + annotations: Annotations { + annotations: vec![Annotation { + name: "associate".into(), + arguments: vec![Argument::List(vec![class_name])], + }], + }, + })))); + } + Err(e) => errors.add_error(e), } } @@ -2128,16 +2136,19 @@ pub fn call_associated_calls<'a>( for ann in &t.annotations { match ann { AnnotationInfo::Associate(associations) => { - if associations - .resources - .iter() - .any(|r| get_synthetic_resource_name(&t.name, r) == resource_name) - { + if associations.resources.iter().any(|r| { + get_synthetic_resource_name(&t.name, &r.basename().into()) + == resource_name + }) { regular_associate = true; } } AnnotationInfo::NestAssociate(associations) => { - if associations.resources.iter().any(|r| r == &resource_name) { + if associations + .resources + .iter() + .any(|r| r.string_is_instance(&resource_name)) + { nested_associate = true; } } @@ -2571,8 +2582,8 @@ mod tests { InheritedAnnotation { annotation: AnnotationInfo::Associate(Associated { resources: BTreeSet::from([ - CascadeString::from("foo"), - CascadeString::from("bar"), + CascadeString::from("foo").into(), + CascadeString::from("bar").into(), ]), }), parents: vec![types.get("resource").unwrap()], @@ -2580,8 +2591,8 @@ mod tests { InheritedAnnotation { annotation: AnnotationInfo::Associate(Associated { resources: BTreeSet::from([ - CascadeString::from("bar"), - CascadeString::from("baz"), + CascadeString::from("bar").into(), + CascadeString::from("baz").into(), ]), }), parents: vec![types.get("domain").unwrap()], @@ -2613,24 +2624,24 @@ mod tests { }, InheritedAnnotation { annotation: AnnotationInfo::Associate(Associated { - resources: BTreeSet::from([CascadeString::from("foo")]), + resources: BTreeSet::from([CascadeString::from("foo").into()]), }), parents: vec![types.get("resource").unwrap()], }, InheritedAnnotation { annotation: AnnotationInfo::Associate(Associated { - resources: BTreeSet::from([CascadeString::from("bar")]), + resources: BTreeSet::from([CascadeString::from("bar").into()]), }), parents: vec![types.get("domain").unwrap(), types.get("resource").unwrap()], }, InheritedAnnotation { annotation: AnnotationInfo::Associate(Associated { - resources: BTreeSet::from([CascadeString::from("baz")]), + resources: BTreeSet::from([CascadeString::from("baz").into()]), }), parents: vec![types.get("domain").unwrap()], }, InheritedAnnotation { - annotation: AnnotationInfo::Alias(CascadeString::from("alias")), + annotation: AnnotationInfo::Alias(CascadeString::from("alias").into()), parents: vec![types.get("domain").unwrap()], }, // Not deduped, because derive doesn't dedup by design diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 1f769156..347fc134 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -31,9 +31,70 @@ const DEFAULT_MLS: &str = "s0"; pub type TypeMap = AliasMap; +#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +pub struct AssociatedResource { + pub name: CascadeString, + pub doms: BTreeSet>, +} + +impl AssociatedResource { + pub fn get_range(&self) -> Option> { + // TODO: This works in the annotation case, but not the nested case + self.name.get_range() + } + + pub fn get_class_names(&self) -> Vec { + self.doms + .iter() + .map(|d| match d { + Some(d) => { + format!("{}.{}", d, &self.name) + } + None => self.name.to_string(), + }) + .collect() + } + + pub fn basename(&self) -> &str { + self.name.as_ref() + } + + // Return true if type_name is one of the resources that have been combined in this + // AssociatedResource + pub fn string_is_instance(&self, type_name: &CascadeString) -> bool { + match type_name.as_ref().split_once('.') { + Some((dom, res)) => { + res == self.name && self.doms.contains(&Some(CascadeString::from(dom))) + } + None => type_name == &self.name && self.doms.contains(&None), + } + } +} + +impl From<&CascadeString> for AssociatedResource { + fn from(cs: &CascadeString) -> Self { + match cs.as_ref().split_once('.') { + Some((dom, res)) => AssociatedResource { + name: res.into(), + doms: [Some(dom.into())].into(), + }, + None => AssociatedResource { + name: cs.clone(), + doms: [None].into(), + }, + } + } +} + +impl From for AssociatedResource { + fn from(cs: CascadeString) -> Self { + (&cs).into() + } +} + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Associated { - pub resources: BTreeSet, + pub resources: BTreeSet, } #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -67,11 +128,18 @@ impl AnnotationInfo { (MakeList, MakeList) => Some(AnnotationInfo::MakeList), (NoDerive, NoDerive) => Some(AnnotationInfo::NoDerive), (Associate(left), Associate(right)) | (NestAssociate(left), NestAssociate(right)) => { - let intersect: BTreeSet = left - .resources - .intersection(&right.resources) - .cloned() - .collect(); + let mut intersect: BTreeSet = BTreeSet::new(); + for l_res in &left.resources { + for r_res in &right.resources { + if l_res.name == r_res.name { + // TODO: Do we need to worry about insert failing? + intersect.insert(AssociatedResource { + name: l_res.name.clone(), + doms: l_res.doms.union(&r_res.doms).cloned().collect(), + }); + } + } + } if intersect.is_empty() { None } else { @@ -119,11 +187,13 @@ impl AnnotationInfo { (MakeList, MakeList) => None, (NoDerive, NoDerive) => None, (Associate(left), Associate(right)) | (NestAssociate(left), NestAssociate(right)) => { - let difference: BTreeSet = left + let difference: BTreeSet = left .resources - .difference(&right.resources) + .iter() + .filter(|l_res| !right.resources.iter().any(|r_res| r_res.name == l_res.name)) .cloned() .collect(); + if difference.is_empty() { None } else { @@ -225,7 +295,7 @@ pub struct TypeInfo { pub list_coercion: bool, // Automatically transform single instances of this type to a single element list pub declaration_file: Option>, // Built in types have no file pub annotations: BTreeSet, - pub associated_resources: BTreeSet, + pub associated_resources: BTreeSet, // TODO: replace with Option<&TypeDecl> pub decl: Option, // If self.is_virtual, then this should always be empty. However, if !self.is_virtual, then @@ -503,7 +573,9 @@ impl TypeInfo { for ann in &self.annotations { if let AnnotationInfo::Associate(associations) = ann { for res in &associations.resources { - if res.as_ref() == associate_name && res.get_range().is_some() { + if res.string_is_instance(&CascadeString::from(associate_name)) + && res.get_range().is_some() + { return res.get_range(); } } @@ -629,7 +701,7 @@ fn get_associate( Ok(AnnotationInfo::Associate(Associated { // Checks for duplicate resources. resources: res_list.iter().try_fold(BTreeSet::new(), |mut s, e| { - if !s.insert(e.clone()) { + if !s.insert(e.into()) { Err(ErrorItem::make_compile_or_internal_error( "Duplicate resource", Some(file), @@ -1463,6 +1535,18 @@ impl<'a> From<&'a TypeInfo> for TypeInstance<'a> { mod tests { use super::*; + #[test] + fn ar_string_is_instance_test() { + let foo_bar = CascadeString::from("foo.bar"); + let bar = CascadeString::from("bar"); + let foo = CascadeString::from("foo"); + let ar = AssociatedResource::from(&foo_bar); + + assert!(ar.string_is_instance(&foo_bar)); + assert!(!ar.string_is_instance(&bar)); + assert!(!ar.string_is_instance(&foo)); + } + #[test] fn basename_test() { let ti = TypeInfo { From 221788ab907fc9fc73d3b2fbe5399f7d918ca24f Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Fri, 18 Aug 2023 16:42:55 -0400 Subject: [PATCH 3/7] Fix range handling --- src/compile.rs | 14 ++++++------ src/internal_rep.rs | 52 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index 48d64a7e..df4db982 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -1694,14 +1694,12 @@ fn interpret_associate( for real_parent_resource in res.get_class_names() { match types.get(&real_parent_resource) { Some(class) => classes.push(class), - None => { - errors.add_error(ErrorItem::make_compile_or_internal_error( - "unknown resource", - dom_info.declaration_file.as_ref(), - res.get_range(), - "didn't find this resource in the policy", - )) - } + None => errors.add_error(ErrorItem::make_compile_or_internal_error( + "unknown resource", + dom_info.declaration_file.as_ref(), + res.get_range(&real_parent_resource), + "didn't find this resource in the policy", + )), } } diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 347fc134..8a9852da 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -31,16 +31,19 @@ const DEFAULT_MLS: &str = "s0"; pub type TypeMap = AliasMap; -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct AssociatedResource { pub name: CascadeString, pub doms: BTreeSet>, + pub ranges: BTreeMap>, } impl AssociatedResource { - pub fn get_range(&self) -> Option> { - // TODO: This works in the annotation case, but not the nested case - self.name.get_range() + // Unlike most get_range() functions, this one takes an argument. An AssociatedResource + // possibly contains information about various associated points, so we need to know the name + // of the resource we want the range for + pub fn get_range(&self, resource_name: &str) -> Option> { + self.ranges.get(resource_name).cloned() } pub fn get_class_names(&self) -> Vec { @@ -73,14 +76,23 @@ impl AssociatedResource { impl From<&CascadeString> for AssociatedResource { fn from(cs: &CascadeString) -> Self { + let mut ranges = BTreeMap::new(); + // If the range is None, we just don't store it and later map lookups will return None, + // which is exactly what we want + if let Some(range) = cs.get_range() { + ranges.insert(cs.to_string(), range); + } + match cs.as_ref().split_once('.') { Some((dom, res)) => AssociatedResource { name: res.into(), doms: [Some(dom.into())].into(), + ranges, }, None => AssociatedResource { name: cs.clone(), doms: [None].into(), + ranges, }, } } @@ -92,6 +104,18 @@ impl From for AssociatedResource { } } +impl PartialOrd for AssociatedResource { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for AssociatedResource { + fn cmp(&self, other: &Self) -> Ordering { + self.name.cmp(&other.name) + } +} + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Associated { pub resources: BTreeSet, @@ -132,10 +156,26 @@ impl AnnotationInfo { for l_res in &left.resources { for r_res in &right.resources { if l_res.name == r_res.name { + // TODO: The whole below should probably be in an impl in + // AssociatedResource. That allows at least ranges to become private + let mut unioned_ranges = BTreeMap::new(); + for (key, val) in &l_res.ranges { + if r_res.ranges.contains_key(key as &String) { + // TODO: I think this could result in weird error messages. + // We're just keeping the left and discarding the right. I'm + // not 100% sure how much that matters, but if there's + // something wrong with right and not left, the error would be + // confusing. Probably the common case is just "there is a + // parent named this", and so it doesn't overly matter if we + // point at right or left... + unioned_ranges.insert(key.to_string(), val.clone()); + } + } // TODO: Do we need to worry about insert failing? intersect.insert(AssociatedResource { name: l_res.name.clone(), doms: l_res.doms.union(&r_res.doms).cloned().collect(), + ranges: unioned_ranges, }); } } @@ -574,9 +614,9 @@ impl TypeInfo { if let AnnotationInfo::Associate(associations) = ann { for res in &associations.resources { if res.string_is_instance(&CascadeString::from(associate_name)) - && res.get_range().is_some() + && res.get_range(associate_name).is_some() { - return res.get_range(); + return res.get_range(associate_name); } } } From 66b994a1b38a4a629a777a600ce9988b5b87df91 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Tue, 22 Aug 2023 17:22:35 -0400 Subject: [PATCH 4/7] Rewrite create_synthetic_resource to work with a list of parents --- src/compile.rs | 57 ++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index df4db982..1c2b42d5 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -1544,23 +1544,27 @@ fn create_synthetic_resource( global_exprs: &mut HashSet, extend_annotations: &BTreeMap>, ) -> Result { - // DO NOT MERGE - // Just use first class to get things working while in development - let class = classes[0]; - if !class.is_resource(types) { - return Err(ErrorItem::make_compile_or_internal_error( + if !classes.iter().all(|c| c.is_resource(types)) { + let mut non_resource_classes = classes.iter().filter(|c| !c.is_resource(types)); + let mut error = ErrorItem::make_compile_or_internal_error( "not a resource", dom_info.declaration_file.as_ref(), - class_string.get_range(), + non_resource_classes.next().and_then(|c| c.name.get_range()), "This should be a resource, not a domain.", - )); - } + ); - let class_name = CascadeString::from(class.basename()); + for c in non_resource_classes { + error = error.maybe_add_additional_message( + dom_info.declaration_file.as_ref(), + c.name.get_range(), + "This should be a resource, not a domain.", + ); + } + return Err(error); + } // Creates a synthetic resource declaration. - let mut dup_res_decl = class.decl.as_ref().ok_or_else(InternalError::new)?.clone(); - let res_name = get_synthetic_resource_name(&dom_info.name, &class_name); + let res_name = get_synthetic_resource_name(&dom_info.name, class_string); if types.get(res_name.as_ref()).is_some() { // A synthetic type with this name already exists, due to a nested association // TODO: I don't think it's an accurate assumption that dom_info is definitely the child in @@ -1572,16 +1576,15 @@ fn create_synthetic_resource( }, ); } - dup_res_decl.name = res_name.clone(); // See TypeDecl::new() in parser.lalrpop for resource inheritance. - let mut parent_names = if associated_parents.is_empty() { + let mut parent_names: Vec = if associated_parents.is_empty() { // This is the full parent.resource name because resource may not exist and parent.resource // is the true parent anyways - vec![class.name.clone()] + classes.iter().map(|c| c.name.clone()).collect() } else { associated_parents .iter() - .map(|parent| get_synthetic_resource_name(&parent.name, &class_name)) + .map(|parent| get_synthetic_resource_name(&parent.name, class_string)) .collect() }; @@ -1595,28 +1598,28 @@ fn create_synthetic_resource( parent_names.append(&mut inherit_ann.clone()); } - dup_res_decl.inherits = parent_names; + let mut new_decl = TypeDecl::new(res_name.clone(), parent_names, Vec::new()); // Virtual resources become concrete when associated to concrete types - dup_res_decl.is_virtual = dup_res_decl.is_virtual && dom_info.is_virtual; + new_decl.is_virtual = classes.iter().all(|c| c.is_virtual) && dom_info.is_virtual; + new_decl.is_trait = false; + new_decl.is_extension = false; // The synthetic resource keeps some, but not all annotations from its parent. // Specifically, Makelist and derive are kept from the parent // TODO: This would be cleaner if we convert to AnnotationInfos first and implent the logic as // a member funtion in AnnotationInfo // See https://github.com/dburgener/cascade/pull/39#discussion_r999510493 for fuller discussion - dup_res_decl - .annotations - .annotations - .retain(|a| a.name.as_ref() == "makelist" || a.name.as_ref() == "derive"); - - dup_res_decl.expressions = Vec::new(); + new_decl.annotations.annotations = classes + .iter() + .flat_map(|c| c.decl.iter().flat_map(|d| d.annotations.annotations.iter())) + .filter(|a| a.name.as_ref() == "makelist" || a.name.as_ref() == "derive") + .cloned() + .collect(); // TODO: dedup? - if !global_exprs.insert(Expression::Decl(Declaration::Type(Box::new( - dup_res_decl.clone(), - )))) { + if !global_exprs.insert(Expression::Decl(Declaration::Type(Box::new(new_decl)))) { // The callers should be handling the situation where the same resource was declared at the // same level of inheritance, but this can arise if a parent associated a resource and a // child associated the same resource. We should find them and return and error message - return match make_duplicate_associate_error(types, dom_info, &class.name) { + return match make_duplicate_associate_error(types, dom_info, class_string) { Some(e) => Err(e.into()), None => Err(InternalError::new().into()), }; From 2cbe37dff2a443bc5fd03c664ba8b81d54c99bbe Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Fri, 22 Sep 2023 14:19:47 -0400 Subject: [PATCH 5/7] Fix a bug I thought I already fixed? I don't know. I'm probably reintroducing a bug here, but it passes tests, so *shrug*? --- src/compile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compile.rs b/src/compile.rs index 1c2b42d5..2288bb51 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -167,7 +167,7 @@ pub fn extend_type_map( if !associated_type.is_extension { let nested_name = get_synthetic_resource_name(&t.name, &associated_type.name); - if type_map.get(associated_type.name.as_ref()).is_none() { + if type_map.get(nested_name.as_ref()).is_none() { // Make the synthetic type to associate // create_synthetic_resource() assumes this must be inherited from a // global parent, which may not be the case From b0e8da142589aa6b6dc00ba27c5c3662146b588b Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Tue, 26 Sep 2023 14:49:01 -0400 Subject: [PATCH 6/7] Fix the bugs --- data/expected_cil/associate.cil | 21 +++++++++++++++ data/policies/associate.cas | 12 +++++++++ src/compile.rs | 16 ++++++----- src/internal_rep.rs | 47 +++++++++++++++++++++++++++++---- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/data/expected_cil/associate.cil b/data/expected_cil/associate.cil index c100a792..bd885973 100644 --- a/data/expected_cil/associate.cil +++ b/data/expected_cil/associate.cil @@ -132,11 +132,20 @@ (typeattribute resource) (typeattribute bin) (typeattributeset resource (bin)) +(typeattribute dom_with_mix) +(typeattributeset domain (dom_with_mix)) +(type dom_with_mix-mix) +(roletype object_r dom_with_mix-mix) +(typeattributeset resource (dom_with_mix-mix)) +(typeattribute dom_with_mix_2) +(typeattributeset domain (dom_with_mix_2)) (typeattribute foo) (typeattributeset domain (foo)) (type kernel_sid) (roletype system_r kernel_sid) (typeattributeset domain (kernel_sid)) +(typeattribute mix) +(typeattributeset resource (mix)) (typeattribute nest_parent) (typeattributeset domain (nest_parent)) (typeattribute nest_parent-nest_resource) @@ -170,6 +179,14 @@ (typeattribute diamond2) (typeattributeset foo (diamond2)) (typeattributeset domain (diamond2)) +(typeattribute dom_with_mix_2-mix) +(typeattributeset mix (dom_with_mix_2-mix)) +(typeattributeset resource (dom_with_mix_2-mix)) +(type dom_with_mix_3) +(roletype system_r dom_with_mix_3) +(typeattributeset dom_with_mix (dom_with_mix_3)) +(typeattributeset dom_with_mix_2 (dom_with_mix_3)) +(typeattributeset domain (dom_with_mix_3)) (typeattribute foo-tmp) (typeattributeset tmp (foo-tmp)) (typeattributeset resource (foo-tmp)) @@ -223,6 +240,10 @@ (typeattributeset diamond1 (diamond3)) (typeattributeset diamond2 (diamond3)) (typeattributeset domain (diamond3)) +(type dom_with_mix_3-mix) +(roletype object_r dom_with_mix_3-mix) +(typeattributeset dom_with_mix_2-mix (dom_with_mix_3-mix)) +(typeattributeset resource (dom_with_mix_3-mix)) (type baz-tmp) (roletype object_r baz-tmp) (typeattributeset bar-tmp (baz-tmp)) diff --git a/data/policies/associate.cas b/data/policies/associate.cas index 5286cab4..47c485c5 100644 --- a/data/policies/associate.cas +++ b/data/policies/associate.cas @@ -116,3 +116,15 @@ virtual domain diamond2 inherits foo {} // Gets two copies of associated resources via multiple inheritance, but they should collapse to one domain diamond3 inherits diamond1, diamond2 {} + + +virtual resource mix {} + +virtual domain dom_with_mix { + resource mix {} +} + +@associate([mix]) +virtual domain dom_with_mix_2 {} + +domain dom_with_mix_3 inherits dom_with_mix, dom_with_mix_2 {} diff --git a/src/compile.rs b/src/compile.rs index 2288bb51..220b5417 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -1528,11 +1528,17 @@ fn generate_type_no_parent_errors(missed_types: Vec<&TypeInfo>, types: &TypeMap) ret } -fn get_synthetic_resource_name( +pub fn get_synthetic_resource_name( dom_name: &CascadeString, associated_resource: &CascadeString, ) -> CascadeString { - format!("{}.{}", dom_name, associated_resource).into() + let cs_name = format!("{}.{}", dom_name, associated_resource); + // We keep the range of the *resource* part specifically, which should always be where this + // resource was defined + match associated_resource.get_range() { + Some(range) => CascadeString::new(cs_name, range), + None => CascadeString::from(cs_name) + } } fn create_synthetic_resource( @@ -1567,10 +1573,8 @@ fn create_synthetic_resource( let res_name = get_synthetic_resource_name(&dom_info.name, class_string); if types.get(res_name.as_ref()).is_some() { // A synthetic type with this name already exists, due to a nested association - // TODO: I don't think it's an accurate assumption that dom_info is definitely the child in - // this case. It's just whichever one happens to be done via annotation return Err( - match make_duplicate_associate_error(types, dom_info, &res_name) { + match make_duplicate_associate_error(types, dom_info, &class_string) { Some(e) => e.into(), None => ErrorItem::Internal(InternalError::new()), }, @@ -1689,7 +1693,7 @@ fn interpret_associate( let potential_resources: BTreeMap<_, _> = associate .resources .iter() - .map(|r| (r.name.as_ref(), (r, false))) + .map(|r| (r.name().as_ref(), (r, false))) .collect(); for (_, (res, _)) in potential_resources.iter().filter(|(_, (_, seen))| !seen) { diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 8a9852da..1b31d78d 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -33,9 +33,9 @@ pub type TypeMap = AliasMap; #[derive(Clone, Debug, Eq, PartialEq)] pub struct AssociatedResource { - pub name: CascadeString, - pub doms: BTreeSet>, - pub ranges: BTreeMap>, + name: CascadeString, + doms: BTreeSet>, + ranges: BTreeMap>, } impl AssociatedResource { @@ -46,6 +46,10 @@ impl AssociatedResource { self.ranges.get(resource_name).cloned() } + pub fn name(&self) -> &CascadeString { + &self.name + } + pub fn get_class_names(&self) -> Vec { self.doms .iter() @@ -564,6 +568,10 @@ impl TypeInfo { self.is_resource(types) && self.name.as_ref().contains('.') } + pub fn get_associated_dom_name(&self) -> Option<&str> { + self.name.as_ref().split_once('.').map(|split| split.0) + } + // Returns false if this type is explicitly declared in source code, and true otherwise pub fn is_synthetic(&self) -> bool { self.name.get_range().is_none() @@ -610,8 +618,10 @@ impl TypeInfo { // specifically associations specifically in source, rather than somehow derived by the // compiler pub fn explicitly_associates(&self, associate_name: &str) -> Option> { + use crate::compile::get_synthetic_resource_name; + for ann in &self.annotations { - if let AnnotationInfo::Associate(associations) = ann { + if let AnnotationInfo::Associate(associations) | AnnotationInfo::NestAssociate(associations) = ann { for res in &associations.resources { if res.string_is_instance(&CascadeString::from(associate_name)) && res.get_range(associate_name).is_some() @@ -621,7 +631,34 @@ impl TypeInfo { } } } - None + let ar_range = self.associated_resources + .iter() + .find(|a| a.string_is_instance(&associate_name.into())) + .and_then(|ar| { + ar.get_range( + get_synthetic_resource_name(&self.name, &associate_name.into()) + .as_ref(), + ) + }); + + if ar_range.is_some() { + return ar_range; + } + + // If the resource is foo.bar, and we are foo, we need to check bar as well. If the + // resource is bar and we are foo, we need to check foo.bar as well + match associate_name.split_once('.') { + Some((dom_name, res_name)) => { + if dom_name == self.name { + self.explicitly_associates(res_name) + } else { + None + } + } + None => { + self.explicitly_associates(get_synthetic_resource_name(&self.name, &associate_name.into()).as_ref()) + } + } } pub fn get_aliases(&self) -> BTreeSet<&CascadeString> { From 25382b67848aad2ed73d8c88aa1452e5a1b472af Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Tue, 26 Sep 2023 15:02:31 -0400 Subject: [PATCH 7/7] Clippy and format --- src/compile.rs | 6 +++--- src/internal_rep.rs | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index 220b5417..1e1a9687 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -219,7 +219,7 @@ pub fn extend_type_map( let mut new_type = new_type.inner(&mut warnings); new_type .associated_resources - .append(&mut (associated_resources).into()); + .append(&mut associated_resources); type_map.insert(t.name.to_string(), new_type)?; } Err(e) => errors.append(e), @@ -1537,7 +1537,7 @@ pub fn get_synthetic_resource_name( // resource was defined match associated_resource.get_range() { Some(range) => CascadeString::new(cs_name, range), - None => CascadeString::from(cs_name) + None => CascadeString::from(cs_name), } } @@ -1574,7 +1574,7 @@ fn create_synthetic_resource( if types.get(res_name.as_ref()).is_some() { // A synthetic type with this name already exists, due to a nested association return Err( - match make_duplicate_associate_error(types, dom_info, &class_string) { + match make_duplicate_associate_error(types, dom_info, class_string) { Some(e) => e.into(), None => ErrorItem::Internal(InternalError::new()), }, diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 1b31d78d..e3503aa3 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -621,7 +621,9 @@ impl TypeInfo { use crate::compile::get_synthetic_resource_name; for ann in &self.annotations { - if let AnnotationInfo::Associate(associations) | AnnotationInfo::NestAssociate(associations) = ann { + if let AnnotationInfo::Associate(associations) + | AnnotationInfo::NestAssociate(associations) = ann + { for res in &associations.resources { if res.string_is_instance(&CascadeString::from(associate_name)) && res.get_range(associate_name).is_some() @@ -631,13 +633,13 @@ impl TypeInfo { } } } - let ar_range = self.associated_resources + let ar_range = self + .associated_resources .iter() .find(|a| a.string_is_instance(&associate_name.into())) .and_then(|ar| { ar.get_range( - get_synthetic_resource_name(&self.name, &associate_name.into()) - .as_ref(), + get_synthetic_resource_name(&self.name, &associate_name.into()).as_ref(), ) }); @@ -655,9 +657,9 @@ impl TypeInfo { None } } - None => { - self.explicitly_associates(get_synthetic_resource_name(&self.name, &associate_name.into()).as_ref()) - } + None => self.explicitly_associates( + get_synthetic_resource_name(&self.name, &associate_name.into()).as_ref(), + ), } }