From b10faae2ed9a0cbc0c3138a6da6307009347bddd Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 23 Nov 2022 10:49:03 -0700 Subject: [PATCH 01/19] Adding support for the fs_context function This function will cover both fsuse and genfscon. --- src/ast.rs | 5 +- src/compile.rs | 13 +++ src/constants.rs | 5 +- src/internal_rep.rs | 260 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 281 insertions(+), 2 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index eadc3eb4..efdf431c 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -386,6 +386,7 @@ pub enum BuiltIns { AvRule, FileContext, ResourceTransition, + FileSystemContext, DomainTransition, } @@ -429,8 +430,10 @@ impl FuncCall { if self.name == constants::RESOURCE_TRANS_FUNCTION_NAME { return Some(BuiltIns::ResourceTransition); } + if self.name == constants::FS_CONTEXT_FUNCTION_NAME { + return Some(BuiltIns::FileSystemContext); + } if self.name == constants::DOMTRANS_FUNCTION_NAME { - return Some(BuiltIns::DomainTransition); } None } diff --git a/src/compile.rs b/src/compile.rs index aac4655e..13fb8f63 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -217,6 +217,19 @@ pub fn get_built_in_types_map() -> Result { if let Some(i) = built_in_types.get_mut(constants::SELF) { i.inherits = vec![CascadeString::from(constants::RESOURCE)]; } + // Add xattr, task, trans, and genfscon as children of fs_type + if let Some(i) = built_in_types.get_mut("xattr") { + i.inherits = vec![CascadeString::from(constants::FS_TYPE)]; + } + if let Some(i) = built_in_types.get_mut("task") { + i.inherits = vec![CascadeString::from(constants::FS_TYPE)]; + } + if let Some(i) = built_in_types.get_mut("trans") { + i.inherits = vec![CascadeString::from(constants::FS_TYPE)]; + } + if let Some(i) = built_in_types.get_mut("genfscon") { + i.inherits = vec![CascadeString::from(constants::FS_TYPE)]; + } Ok(built_in_types) } diff --git a/src/constants.rs b/src/constants.rs index 6887b4ff..a717d31b 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -6,6 +6,7 @@ pub const AUDITALLOW_FUNCTION_NAME: &str = "auditallow"; pub const NEVERALLOW_FUNCTION_NAME: &str = "neverallow"; pub const FILE_CONTEXT_FUNCTION_NAME: &str = "file_context"; pub const RESOURCE_TRANS_FUNCTION_NAME: &str = "resource_transition"; +pub const FS_CONTEXT_FUNCTION_NAME: &str = "fs_context"; pub const DOMTRANS_FUNCTION_NAME: &str = "domain_transition"; pub const SYSTEM_TYPE: &str = "machine_type"; pub const MONOLITHIC: &str = "monolithic"; @@ -24,9 +25,11 @@ pub const PERM: &str = "perm"; pub const CLASS: &str = "obj_class"; pub const MODULE: &str = "module"; pub const SELF: &str = "self"; +pub const FS_TYPE: &str = "fs_type"; pub const BUILT_IN_TYPES: &[&str] = &[ - DOMAIN, RESOURCE, MODULE, "path", "string", CLASS, PERM, "context", SELF, "*", + DOMAIN, RESOURCE, MODULE, "path", "string", CLASS, PERM, "context", SELF, FS_TYPE, + "xattr", "task", "trans", "genfscon", "*", ]; pub const SYSTEM_CONFIG_DEFAULTS: &[(&str, &str)] = diff --git a/src/internal_rep.rs b/src/internal_rep.rs index f80ac4e9..be8a8ff4 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1545,6 +1545,248 @@ fn call_to_fc_rules<'a>( Ok(ret) } +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum FSContextType { + XAttr, + Task, + Trans, + GenFSCon, +} + +impl fmt::Display for FSContextType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + match self { + FSContextType::XAttr => "xattr", + FSContextType::Task => "task", + FSContextType::Trans => "trans", + FSContextType::GenFSCon => "genfscon", + } + ) + } +} + +impl FromStr for FSContextType { + type Err = (); + fn from_str(s: &str) -> Result { + match s { + "xattr" => Ok(FSContextType::XAttr), + "task" => Ok(FSContextType::Task), + "trans" => Ok(FSContextType::Trans), + "genfscon" => Ok(FSContextType::GenFSCon), + _ => Err(()), + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub struct FileSystemContextRule<'a> { + pub fscontext_type: FSContextType, + pub fs_name: String, + pub path: Option, + pub file_type: Option, + pub context: Context<'a>, +} + +// TODO convert to TryFrom/try_from see comment below +impl From<&FileSystemContextRule<'_>> for sexp::Sexp { + fn from(f: &FileSystemContextRule) -> sexp::Sexp { + match f.fscontext_type { + FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => list(&[ + atom_s("fsuse"), + Sexp::Atom(Atom::S(f.fscontext_type.to_string())), + atom_s(&f.fs_name.trim_matches('"')), + Sexp::from(&f.context), + ]), + FSContextType::GenFSCon => { + // Since path is an optional arg and I dont want to get + // into unwrap issue we are doing an 'if let' here. The lack + // of path should be caught ealier, so if we dont have a path + // we will return an empty list. The more correct way to fix this + // is convert this to a try_from, but this causes issues with some + // of our match statements and mixing returns. + if let Some(p) = &f.path { + match &f.file_type { + Some(file_type) => list(&[ + atom_s("genfscon"), + atom_s(&f.fs_name.trim_matches('"')), + atom_s(p.as_ref()), + Sexp::Atom(Atom::S(file_type.to_string())), + Sexp::from(&f.context), + ]), + None => list(&[ + atom_s("genfscon"), + atom_s(&f.fs_name.trim_matches('"')), + atom_s(p.as_ref()), + Sexp::from(&f.context), + ]), + } + } else { + list(&[]) + } + } + } + } +} + +fn call_to_fsc_rules<'a>( + c: &'a FuncCall, + types: &'a TypeMap, + class_perms: &ClassList, + context: &BlockContext<'a>, + file: &'a SimpleFile, +) -> Result>, CascadeErrors> { + let target_args = vec![ + FunctionArgument::new( + &DeclaredArgument { + param_type: CascadeString::from(constants::RESOURCE), + is_list_param: false, + name: CascadeString::from("fs_label"), + default: None, + }, + types, + None, + )?, + FunctionArgument::new( + &DeclaredArgument { + param_type: CascadeString::from("string"), + is_list_param: false, + name: CascadeString::from("fs_name"), + default: None, + }, + types, + None, + )?, + FunctionArgument::new( + &DeclaredArgument { + param_type: CascadeString::from("fs_type"), + is_list_param: false, + name: CascadeString::from("fscontext_type"), + default: None, + }, + types, + None, + )?, + FunctionArgument::new( + &DeclaredArgument { + param_type: CascadeString::from("path"), + is_list_param: false, + name: CascadeString::from("path_regex"), + default: Some(Argument::Quote(CascadeString::from("/"))), + }, + types, + None, + )?, + FunctionArgument::new( + &DeclaredArgument { + param_type: CascadeString::from("obj_class"), //TODO: not really + is_list_param: true, + name: CascadeString::from("file_type"), + default: Some(Argument::List(vec![])), + }, + types, + None, + )?, + ]; + let validated_args = validate_arguments(c, &target_args, types, class_perms, context, file)?; + let mut args_iter = validated_args.iter(); + let mut ret = Vec::new(); + + let context_str = args_iter + .next() + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? + .get_name_or_string(context)?; + let fs_context = match Context::try_from(context_str.to_string()) { + Ok(c) => c, + Err(_) => { + return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + "Invalid context", + file, + context_str.get_range(), + "Cannot parse this into a context", + )))) + } + }; + let fs_name = args_iter + .next() + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? + .get_name_or_string(context)? + .to_string(); + let fscontext_str = args_iter + .next() + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? + .get_name_or_string(context)?; + let fscontext_type = match fscontext_str.to_string().parse::() { + Ok(f) => f, + Err(_) => { + return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + "Not a valid file system type", + file, + fscontext_str.get_range(), //TODO error not showing correctly + "File system type must be 'xattr', 'task', 'trans', or 'genfscon'", + )))); + } + }; + let regex_string = args_iter + .next() + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? + .get_name_or_string(context)? + .to_string(); + let file_types = args_iter + .next() + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? + .get_list(context)?; + + match fscontext_type { + FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => { + ret.push(FileSystemContextRule { + fscontext_type: fscontext_type, + fs_name: fs_name.clone(), + path: None, + file_type: None, + context: fs_context.clone(), + }); + } + FSContextType::GenFSCon => { + if file_types.len() == 0 { + ret.push(FileSystemContextRule { + fscontext_type: fscontext_type, + fs_name: fs_name.clone(), + path: Some(regex_string.clone()), + file_type: None, + context: fs_context.clone(), + }); + } else { + for file_type in file_types { + let file_type = match file_type.to_string().parse::() { + Ok(f) => f, + Err(_) => { + return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + "Not a valid file type", + file, + file_type.get_range(), + "", + )))) + } + }; + + ret.push(FileSystemContextRule { + fscontext_type: fscontext_type.clone(), + fs_name: fs_name.clone(), + path: Some(regex_string.clone()), + file_type: Some(file_type), + context: fs_context.clone(), + }); + } + } + } + } + + Ok(ret) +} + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct DomtransRule<'a> { pub source: Cow<'a, CascadeString>, @@ -2225,6 +2467,7 @@ impl TryFrom<&FunctionInfo<'_>> for sexp::Sexp { ValidatedStatement::AvRule(a) => macro_cil.push(Sexp::from(a)), ValidatedStatement::FcRule(f) => macro_cil.push(Sexp::from(f)), ValidatedStatement::ResourcetransRule(r) => macro_cil.push(Sexp::from(r)), + ValidatedStatement::FscRule(fs) => macro_cil.push(Sexp::from(fs)), ValidatedStatement::DomtransRule(d) => macro_cil.push(Sexp::from(d)), } } @@ -2311,6 +2554,7 @@ pub enum ValidatedStatement<'a> { AvRule(AvRule<'a>), FcRule(FileContextRule<'a>), ResourcetransRule(ResourcetransRule<'a>), + FscRule(FileSystemContextRule<'a>), DomtransRule(DomtransRule<'a>), } @@ -2373,6 +2617,21 @@ impl<'a> ValidatedStatement<'a> { )) } } + Some(BuiltIns::FileSystemContext) => { + if in_resource { + Ok(call_to_fsc_rules(c, types, class_perms, &*context, file)? + .into_iter() + .map(ValidatedStatement::FscRule) + .collect()) + } else { + Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + "file_context() calls are only allowed in resources", + file, + c.name.get_range(), + "Not allowed here", + )))) + } + } Some(BuiltIns::DomainTransition) => { if !in_resource { Ok( @@ -2458,6 +2717,7 @@ impl From<&ValidatedStatement<'_>> for sexp::Sexp { ValidatedStatement::AvRule(a) => Sexp::from(a), ValidatedStatement::FcRule(f) => Sexp::from(f), ValidatedStatement::ResourcetransRule(r) => Sexp::from(r), + ValidatedStatement::FscRule(fs) => Sexp::from(fs), ValidatedStatement::DomtransRule(d) => Sexp::from(d), } } From 51864802081a14827e27461aac4f17929df9fdd0 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 23 Nov 2022 12:42:38 -0700 Subject: [PATCH 02/19] fmt, clippy, docs, tests --- data/error_policies/fs_context.cas | 11 +++ data/expected_cil/fs_context.cil | 137 +++++++++++++++++++++++++++++ data/policies/fs_context.cas | 12 +++ doc/TE.md | 37 ++++++++ src/internal_rep.rs | 20 ++--- src/lib.rs | 20 +++++ 6 files changed, 227 insertions(+), 10 deletions(-) create mode 100644 data/error_policies/fs_context.cas create mode 100644 data/expected_cil/fs_context.cil create mode 100644 data/policies/fs_context.cas diff --git a/data/error_policies/fs_context.cas b/data/error_policies/fs_context.cas new file mode 100644 index 00000000..71f7acd3 --- /dev/null +++ b/data/error_policies/fs_context.cas @@ -0,0 +1,11 @@ +resource foo { + fs_context(bob, "ext3", xattr); + fs_context(this, "sockfs", fs_type); + fs_context(this, "sockfs", foo); + fs_context(this, "proc", zap); + + fs_context(this, "sysfs", genfscon, "/zap", [bar]); + + // Policies must include at least one av rule + allow(domain, foo, file, [read]); +} diff --git a/data/expected_cil/fs_context.cil b/data/expected_cil/fs_context.cil new file mode 100644 index 00000000..2e0d2087 --- /dev/null +++ b/data/expected_cil/fs_context.cil @@ -0,0 +1,137 @@ +(class alg_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class anon_inode (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class appletalk_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class association (sendto recvfrom setcontext polmatch)) +(class atmpvc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class atmsvc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class ax25_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class binder (impersonate call set_context_mgr transfer)) +(class blk_file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class bluetooth_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class bpf (map_create map_read map_write prog_load prog_run)) +(class caif_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class can_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class cap2_userns (mac_override mac_admin syslog wake_alarm block_suspend audit_read perfmon bpf checkpoint_restore)) +(class cap_userns (chown dac_override dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config mknod lease audit_write audit_control setfcap)) +(class capability (chown dac_override dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config mknod lease audit_write audit_control setfcap)) +(class capability2 (mac_override mac_admin syslog wake_alarm block_suspend audit_read perfmon bpf checkpoint_restore)) +(class chr_file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class dbus (acquire_svc send_msg)) +(class dccp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind name_connect)) +(class decnet_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class dir (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads add_name remove_name reparent search rmdir)) +(class fd (use)) +(class fifo_file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads execute_no_trans entrypoint)) +(class filesystem (mount remount unmount getattr relabelfrom relabelto associate quotamod quotaget watch)) +(class icmp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind)) +(class ieee802154_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class infiniband_endpoint (manage_subnet)) +(class infiniband_pkey (access)) +(class ipc (create destroy getattr setattr read write associate unix_read unix_write)) +(class ipx_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class irda_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class isdn_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class iucv_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class kcm_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class kernel_service (use_as_override create_files_as)) +(class key (view read write search link setattr create)) +(class key_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class llc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class lnk_file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class lockdown (integrity confidentiality)) +(class memprotect (mmap_zero)) +(class msg (send receive)) +(class msgq (create destroy getattr setattr read write associate unix_read unix_write enqueue)) +(class netif (ingress egress)) +(class netlink_audit_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind nlmsg_read nlmsg_write nlmsg_relay nlmsg_readpriv nlmsg_tty_audit)) +(class netlink_connector_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_crypto_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_dnrt_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_fib_lookup_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_generic_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_iscsi_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_kobject_uevent_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_netfilter_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_nflog_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_rdma_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_route_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind nlmsg_read nlmsg_write)) +(class netlink_scsitransport_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_selinux_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class netlink_tcpdiag_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind nlmsg_read nlmsg_write)) +(class netlink_xfrm_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind nlmsg_read nlmsg_write)) +(class netrom_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class nfc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class node (recvfrom sendto)) +(class node_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind)) +(class packet (send recv relabelto forward_in forward_out)) +(class packet_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class peer (recv)) +(class perf_event (open cpu kernel tracepoint read write)) +(class phonet_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class pppox_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class process (fork transition sigchld sigkill sigstop signull signal ptrace getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh setrlimit rlimitinh dyntransition setcurrent execmem execstack execheap setkeycreate setsockcreate getrlimit)) +(class process2 (nnp_transition nosuid_transition)) +(class qipcrtr_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class rawip_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind)) +(class rds_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class rose_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class rxrpc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class sctp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind name_connect association)) +(class security (compute_av compute_create compute_member check_context load_policy compute_relabel compute_user setenforce setbool setsecparam setcheckreqprot read_policy validate_trans)) +(class sem (create destroy getattr setattr read write associate unix_read unix_write)) +(class service (start stop status reload enable disable)) +(class shm (create destroy getattr setattr read write associate unix_read unix_write lock)) +(class smc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class sock_file (ioctl read write create getattr setattr lock relabelfrom relabelto append map unlink link rename execute quotaon mounton audit_access open execmod watch watch_mount watch_sb watch_with_perm watch_reads)) +(class socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class system (ipc_info syslog_read syslog_mod syslog_console module_request module_load halt reboot status start stop enable disable reload)) +(class tcp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind name_connect)) +(class tipc_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class tun_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind attach_queue)) +(class udp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind node_bind)) +(class unix_dgram_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class unix_stream_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind connectto)) +(class vsock_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class x25_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class xdp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(classorder (alg_socket anon_inode appletalk_socket association atmpvc_socket atmsvc_socket ax25_socket binder blk_file bluetooth_socket bpf caif_socket can_socket cap2_userns cap_userns capability capability2 chr_file dbus dccp_socket decnet_socket dir fd fifo_file file filesystem icmp_socket ieee802154_socket infiniband_endpoint infiniband_pkey ipc ipx_socket irda_socket isdn_socket iucv_socket kcm_socket kernel_service key key_socket llc_socket lnk_file lockdown memprotect msg msgq netif netlink_audit_socket netlink_connector_socket netlink_crypto_socket netlink_dnrt_socket netlink_fib_lookup_socket netlink_generic_socket netlink_iscsi_socket netlink_kobject_uevent_socket netlink_netfilter_socket netlink_nflog_socket netlink_rdma_socket netlink_route_socket netlink_scsitransport_socket netlink_selinux_socket netlink_socket netlink_tcpdiag_socket netlink_xfrm_socket netrom_socket nfc_socket node node_socket packet packet_socket peer perf_event phonet_socket pppox_socket process process2 qipcrtr_socket rawip_socket rds_socket rose_socket rxrpc_socket sctp_socket security sem service shm smc_socket sock_file socket system tcp_socket tipc_socket tun_socket udp_socket unix_dgram_socket unix_stream_socket vsock_socket x25_socket xdp_socket)) +(sensitivity s0) +(sensitivityorder (s0)) +(user system_u) +(role system_r) +(role object_r) +(userrole system_u system_r) +(userrole system_u object_r) +(userlevel system_u (s0)) +(userrange system_u ((s0) (s0))) +(handleunknown allow) +(typeattribute domain) +(typeattribute resource) +(type foo) +(roletype object_r foo) +(typeattributeset resource (foo)) +(type kernel_sid) +(roletype system_r kernel_sid) +(typeattributeset domain (kernel_sid)) +(type security_sid) +(roletype object_r security_sid) +(typeattributeset resource (security_sid)) +(type unlabeled_sid) +(roletype object_r unlabeled_sid) +(typeattributeset resource (unlabeled_sid)) +(allow domain foo (file (read))) +(fsuse xattr ext3 (system_u object_r foo ((s0) (s0)))) +(fsuse task sockfs (system_u object_r foo ((s0) (s0)))) +(fsuse trans proc (system_u object_r foo ((s0) (s0)))) +(genfscon proc "/" (system_u object_r foo ((s0) (s0)))) +(genfscon sysfs "/zap" file (system_u object_r foo ((s0) (s0)))) +(genfscon tmpfs "/" (system_u object_r foo ((s0) (s0)))) +(sid kernel) +(sidcontext kernel (system_u system_r kernel_sid ((s0) (s0)))) +(sid security) +(sidcontext security (system_u object_r security_sid ((s0) (s0)))) +(sid unlabeled) +(sidcontext unlabeled (system_u object_r unlabeled_sid ((s0) (s0)))) +(sidorder (kernel security unlabeled)) \ No newline at end of file diff --git a/data/policies/fs_context.cas b/data/policies/fs_context.cas new file mode 100644 index 00000000..1d1406a3 --- /dev/null +++ b/data/policies/fs_context.cas @@ -0,0 +1,12 @@ +resource foo { + fs_context(foo, "ext3", xattr); + fs_context(this, "sockfs", task); + fs_context(this, "proc", trans); + + fs_context(this, "proc", genfscon, "/"); + fs_context(this, "tmpfs", genfscon); + fs_context(this, "sysfs", genfscon, "/zap", [file]); + + // Policies must include at least one av rule + allow(domain, foo, file, [read]); +} \ No newline at end of file diff --git a/doc/TE.md b/doc/TE.md index 445873d0..660e1a62 100644 --- a/doc/TE.md +++ b/doc/TE.md @@ -170,6 +170,11 @@ The following types are built in to the language: * user * mls * bool +* fs_type +* xattr +* task +* trans +* genfscon * [T] (a list of any other types) TODO: Which of these can be in a standard library instead of the compiler? @@ -290,6 +295,38 @@ TODO: default contexts. This will be part of the next phase of design TODO: file_contexts.subs_dist (although is it necessary with advanced file_context handling features?) +### Filesystem labeling + +All filesystem are labeled using the following prototype: + +`fs_context(resource fs_label, string fs_name, fs_type type, path path, file_type [obj_class])` + +* `fs_label` is label you wish to apply to the filesystem. +* `fs_name` is the OS recognized name for the filesystem. (e.g. ext4, proc, tmpfs) +* `fs_type` must be one of the following options: + * `xattr`: Filesystems supporting the extended attribute security.selinux. The labeling is persistent for filesystems that support extended attributes. + * `task`: For pseudo filesystems supporting task related services such as pipes and sockets. + * `trans`: For pseudo filesystems such as pseudo terminals and temporary objects. + * `genfscon`: Used to allocate a security context to filesystems that cannot support any of the previous file labeling options +* `path` is an optional path relative to root of the mounted filesystem. Currently this is only valid for the proc filesystem, all other types must be "/". If not given, the field will default to "/". +* `file_type` is an optional keyword representing a file type to apply the label to. Valid values are the same as in file_context function. If not given, [any] is assumed. + * Note: You must use SELinux userspace tools version 3.4 or newer to use this field. + +`xattr`, `task`, and `trans` all represent filesystems that support SELinux security contexts. The filesystem itself has a labeled applied to it as a whole, which is the `fs_label` provided in this function. All files on the filesystem also store their own SELinux security context in their own extended attributes. + +`genfscon` represents filesystems that do not support SELinux security contexts. Generally a filesystem has a single default security context, `fs_label` provided in this function, assigned by `genfscon` from the root (/) that is inherited by all files and directories on that filesystem. The exception to this is the /proc filesystem, where directories can be labeled with a specific security context. + +This call must be part of a resource block. + +#### Examples +fs_context(foo, "ext3", xattr); +fs_context(this, "sockfs", task); +fs_context(this, "tmpfs", trans); + +fs_context(this, "cgroup", genfscon); +fs_context(this, "sysfs", genfscon, "/"); +fs_context(this, "proc", genfscon, "/zap", [file]); + ## Constants Constants may be a helpful way to name data for later reference. They are diff --git a/src/internal_rep.rs b/src/internal_rep.rs index be8a8ff4..c25aee29 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1597,7 +1597,7 @@ impl From<&FileSystemContextRule<'_>> for sexp::Sexp { FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => 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.trim_matches('"')), Sexp::from(&f.context), ]), FSContextType::GenFSCon => { @@ -1611,14 +1611,14 @@ impl From<&FileSystemContextRule<'_>> for sexp::Sexp { match &f.file_type { Some(file_type) => list(&[ atom_s("genfscon"), - atom_s(&f.fs_name.trim_matches('"')), + atom_s(f.fs_name.trim_matches('"')), atom_s(p.as_ref()), Sexp::Atom(Atom::S(file_type.to_string())), Sexp::from(&f.context), ]), None => list(&[ atom_s("genfscon"), - atom_s(&f.fs_name.trim_matches('"')), + atom_s(f.fs_name.trim_matches('"')), atom_s(p.as_ref()), Sexp::from(&f.context), ]), @@ -1674,7 +1674,7 @@ fn call_to_fsc_rules<'a>( param_type: CascadeString::from("path"), is_list_param: false, name: CascadeString::from("path_regex"), - default: Some(Argument::Quote(CascadeString::from("/"))), + default: Some(Argument::Quote(CascadeString::from("\"/\""))), }, types, None, @@ -1742,19 +1742,19 @@ fn call_to_fsc_rules<'a>( match fscontext_type { FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => { ret.push(FileSystemContextRule { - fscontext_type: fscontext_type, - fs_name: fs_name.clone(), + fscontext_type, + fs_name, path: None, file_type: None, context: fs_context.clone(), }); } FSContextType::GenFSCon => { - if file_types.len() == 0 { + if file_types.is_empty() { ret.push(FileSystemContextRule { - fscontext_type: fscontext_type, - fs_name: fs_name.clone(), - path: Some(regex_string.clone()), + fscontext_type, + fs_name, + path: Some(regex_string), file_type: None, context: fs_context.clone(), }); diff --git a/src/lib.rs b/src/lib.rs index 88953622..ba62da43 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1205,6 +1205,26 @@ mod tests { #[test] fn valid_self() { valid_policy_test("self.cas", &["allow qux self (file (read))"], &[]); + + #[test] + fn valid_fs_context() { + valid_policy_test( + "fs_context.cas", + &[ + "fsuse xattr ext3 (system_u object_r foo ((s0) (s0)))", + "fsuse task sockfs (system_u object_r foo ((s0) (s0)))", + "fsuse trans proc (system_u object_r foo ((s0) (s0)))", + "genfscon proc \"/\" (system_u object_r foo ((s0) (s0)))", + "genfscon sysfs \"/zap\" file (system_u object_r foo ((s0) (s0)))", + "genfscon tmpfs \"/\" (system_u object_r foo ((s0) (s0)))", + ], + &[], + ); + } + + #[test] + fn invalid_fs_context() { + error_policy_test!("fs_context.cas", 5, ErrorItem::Compile(_)); } #[test] From 6b8e8c44b7be24ebc9a87ac90dc7d3ec54a2980c Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Mon, 28 Nov 2022 10:54:12 -0700 Subject: [PATCH 03/19] adding duplicate fs context checks --- data/error_policies/fs_context_dup.cas | 16 +++++ data/expected_cil/fs_context.cil | 10 +++- data/policies/fs_context.cas | 12 +++- src/compile.rs | 81 +++++++++++++++++++++++++- src/error.rs | 23 ++++++++ src/internal_rep.rs | 51 +++++++++++----- src/lib.rs | 13 ++++- 7 files changed, 182 insertions(+), 24 deletions(-) create mode 100644 data/error_policies/fs_context_dup.cas diff --git a/data/error_policies/fs_context_dup.cas b/data/error_policies/fs_context_dup.cas new file mode 100644 index 00000000..76687c5c --- /dev/null +++ b/data/error_policies/fs_context_dup.cas @@ -0,0 +1,16 @@ +resource foo { + fs_context(foo, "ext3", xattr); + fs_context(foo, "ext3", task); + + fs_context(this, "sysfs", genfscon, "/zap", [dir]); + fs_context(this, "sysfs", genfscon, "/zap", [file]); + + fs_context(this, "test", genfscon, "/zap/baa", [file]); + + // Policies must include at least one av rule + allow(domain, foo, file, [read]); +} + +resource bar { + fs_context(this, "test", genfscon, "/zap/baa", [file]); +} \ No newline at end of file diff --git a/data/expected_cil/fs_context.cil b/data/expected_cil/fs_context.cil index 2e0d2087..9b097145 100644 --- a/data/expected_cil/fs_context.cil +++ b/data/expected_cil/fs_context.cil @@ -109,6 +109,9 @@ (handleunknown allow) (typeattribute domain) (typeattribute resource) +(type bar) +(roletype object_r bar) +(typeattributeset resource (bar)) (type foo) (roletype object_r foo) (typeattributeset resource (foo)) @@ -124,10 +127,11 @@ (allow domain foo (file (read))) (fsuse xattr ext3 (system_u object_r foo ((s0) (s0)))) (fsuse task sockfs (system_u object_r foo ((s0) (s0)))) -(fsuse trans proc (system_u object_r foo ((s0) (s0)))) +(fsuse trans tmpfs (system_u object_r foo ((s0) (s0)))) +(genfscon cgroup "/" (system_u object_r foo ((s0) (s0)))) (genfscon proc "/" (system_u object_r foo ((s0) (s0)))) -(genfscon sysfs "/zap" file (system_u object_r foo ((s0) (s0)))) -(genfscon tmpfs "/" (system_u object_r foo ((s0) (s0)))) +(genfscon sysfs "/zap" dir (system_u object_r foo ((s0) (s0)))) +(genfscon sysfs "/zap/baa" file (system_u object_r bar ((s0) (s0)))) (sid kernel) (sidcontext kernel (system_u system_r kernel_sid ((s0) (s0)))) (sid security) diff --git a/data/policies/fs_context.cas b/data/policies/fs_context.cas index 1d1406a3..8da4c5e2 100644 --- a/data/policies/fs_context.cas +++ b/data/policies/fs_context.cas @@ -1,12 +1,18 @@ resource foo { fs_context(foo, "ext3", xattr); fs_context(this, "sockfs", task); - fs_context(this, "proc", trans); + fs_context(this, "tmpfs", trans); + fs_context(this, "tmpfs", trans); fs_context(this, "proc", genfscon, "/"); - fs_context(this, "tmpfs", genfscon); - fs_context(this, "sysfs", genfscon, "/zap", [file]); + fs_context(this, "proc", genfscon, "/"); + fs_context(this, "cgroup", genfscon); + fs_context(this, "sysfs", genfscon, "/zap", [dir]); // Policies must include at least one av rule allow(domain, foo, file, [read]); +} + +resource bar { + fs_context(this, "sysfs", genfscon, "/zap/baa", [file]); } \ No newline at end of file diff --git a/src/compile.rs b/src/compile.rs index 13fb8f63..c2363aea 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -12,11 +12,11 @@ use crate::ast::{ }; use crate::constants; use crate::context::{BlockType, Context as BlockContext}; -use crate::error::{CascadeErrors, ErrorItem, InternalError}; +use crate::error::{CascadeErrors, ErrorItem, InternalError, InvalidFileSystemError}; use crate::internal_rep::{ argument_to_typeinfo, argument_to_typeinfo_vec, generate_sid_rules, type_slice_to_variant, validate_derive_args, Annotated, AnnotationInfo, ArgForValidation, Associated, BoundTypeInfo, - ClassList, Context, FunctionArgument, FunctionInfo, FunctionMap, MachineMap, ModuleMap, Sid, + ClassList, Context, FileSystemContextRule, FSContextType, FunctionArgument, FunctionInfo, FunctionMap, MachineMap, ModuleMap, Sid, TypeInfo, TypeMap, ValidatedCall, ValidatedMachine, ValidatedModule, ValidatedStatement, }; @@ -342,6 +342,81 @@ pub fn build_func_map<'a>( Ok(decl_map) } +#[allow(clippy::collapsible_if)] +pub fn validate_rules(statements: &BTreeSet) -> Result<(), CascadeErrors> { + let mut errors = CascadeErrors::new(); + let mut fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>> = 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); + } + } + + 'key_loop: for (_, v) in fsc_rules { + // We only have 1 or 0 elements, thus we cannot have a semi duplicate + if v.len() <= 1 { + continue; + } + for rule in &v { + match rule.fscontext_type { + FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => { + // If we ever see a duplicate and one of them is xattr, task, or trans something is wrong + // return an error and look for more. + errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( + "Duplicate filesystem context.\n Found two different filesystem type declarations for filesystem: {}", + rule.fs_name + )))); + continue 'key_loop; + } + FSContextType::GenFSCon => { + // genfscon gets more complicated. We can have simlar rules as long as the paths different. + // If we find a genfscon with the same path, they must have the same context and object type. + if let Some(path) = &rule.path { + for inner_rule in &v { + if let Some(inner_path) = &inner_rule.path { + if path == inner_path && rule.context != inner_rule.context { + errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( + "Duplicate genfscon.\n Found duplicate genfscon rules with differing contexts: {}\ + \n\tPath: {}\n\tContext 1: {}\n\tContext 2: {}", + rule.fs_name, + path, + rule.context, + inner_rule.context, + )))); + continue 'key_loop; + // else if let is not suppored currently (https://github.com/rust-lang/rust/issues/53667). + // Thus we check if we are not none and then unwrap + } else if path == inner_path + && rule.file_type.is_some() + && inner_rule.file_type.is_some() + { + if rule.file_type.as_ref().unwrap() + != inner_rule.file_type.as_ref().unwrap() + { + errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( + "Duplicate genfscon.\n Found duplicate genfscon rules with differing object types: {}\ + \n\tPath: {}\n\tObject Type 1: {}\n\tObject Type 2: {}", + rule.fs_name, + path, + rule.file_type.as_ref().unwrap(), + inner_rule.file_type.as_ref().unwrap(), + )))); + continue 'key_loop; + } + } + } + } + } + } + } + } + } + + errors.into_result(()) +} + // Mutate hash map to set the validated body pub fn validate_functions<'a>( mut functions: FunctionMap<'a>, @@ -822,6 +897,8 @@ pub fn get_reduced_infos<'a>( // Get the policy rules let new_policy_rules = get_policy_rules(policies, &new_type_map, classlist, &new_func_map)?; + validate_rules(&new_policy_rules)?; + // generate_sexp(...) is called at this step because new_func_map and new_policy_rules, // which are needed for the generate_sexp call, cannot be returned from this function. // This is because they reference the local variable, new_func_map_copy, which cannot be diff --git a/src/error.rs b/src/error.rs index 83f8c20d..a97a65fb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -211,6 +211,21 @@ impl InvalidMachineError { } } +#[derive(Error, Clone, Debug)] +#[error("{diagnostic}")] +pub struct InvalidFileSystemError { + pub diagnostic: Diag, +} + +impl InvalidFileSystemError { + pub fn new(msg: &str) -> Self { + let diagnostic = Diagnostic::error().with_message(msg); + InvalidFileSystemError { + diagnostic: diagnostic.into(), + } + } +} + #[derive(Error, Debug)] pub enum ErrorItem { #[error("Compilation error: {0}")] @@ -224,6 +239,8 @@ pub enum ErrorItem { IO(#[from] io::Error), #[error("Invalid machine error: {0}")] InvalidMachine(#[from] InvalidMachineError), + #[error("Invalid filesystem error: {0}")] + InvalidFileSystem(#[from] InvalidFileSystemError), } impl ErrorItem { @@ -353,6 +370,12 @@ impl From for CascadeErrors { } } +impl From for CascadeErrors { + fn from(error: InvalidFileSystemError) -> Self { + CascadeErrors::from(ErrorItem::from(error)) + } +} + impl Iterator for CascadeErrors { type Item = ErrorItem; fn next(&mut self) -> Option { diff --git a/src/internal_rep.rs b/src/internal_rep.rs index c25aee29..f220d451 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -995,6 +995,16 @@ impl<'a> TryFrom for Context<'a> { } } +impl<'a> fmt::Display for Context<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}:{}:{}:{} - {}", + self.user, self.role, self.setype, self.mls_low, self.mls_high, + ) + } +} + pub struct Sid<'a> { name: &'a str, context: Context<'a>, @@ -1631,6 +1641,18 @@ impl From<&FileSystemContextRule<'_>> for sexp::Sexp { } } +impl FileSystemContextRule<'_> { + fn get_renamed_statement(&self, renames: &BTreeMap) -> Self { + FileSystemContextRule { + fscontext_type: self.fscontext_type.clone(), + fs_name: self.fs_name.clone(), + path: self.path.clone(), + file_type: self.file_type, + context: self.context.get_renamed_context(renames), + } + } +} + fn call_to_fsc_rules<'a>( c: &'a FuncCall, types: &'a TypeMap, @@ -1690,7 +1712,7 @@ fn call_to_fsc_rules<'a>( None, )?, ]; - let validated_args = validate_arguments(c, &target_args, types, class_perms, context, file)?; + let validated_args = validate_arguments(c, &target_args, types, class_perms, context, Some(file))?; let mut args_iter = validated_args.iter(); let mut ret = Vec::new(); @@ -1701,12 +1723,12 @@ fn call_to_fsc_rules<'a>( let fs_context = match Context::try_from(context_str.to_string()) { Ok(c) => c, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "Invalid context", - file, + Some(file), context_str.get_range(), "Cannot parse this into a context", - )))) + ))) } }; let fs_name = args_iter @@ -1721,12 +1743,12 @@ fn call_to_fsc_rules<'a>( let fscontext_type = match fscontext_str.to_string().parse::() { Ok(f) => f, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "Not a valid file system type", - file, + Some(file), fscontext_str.get_range(), //TODO error not showing correctly "File system type must be 'xattr', 'task', 'trans', or 'genfscon'", - )))); + ))); } }; let regex_string = args_iter @@ -1763,12 +1785,12 @@ fn call_to_fsc_rules<'a>( let file_type = match file_type.to_string().parse::() { Ok(f) => f, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "Not a valid file type", - file, + Some(file), file_type.get_range(), "", - )))) + ))) } }; @@ -2624,12 +2646,12 @@ impl<'a> ValidatedStatement<'a> { .map(ValidatedStatement::FscRule) .collect()) } else { - Err(CascadeErrors::from(ErrorItem::Compile(CompileError::new( + Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "file_context() calls are only allowed in resources", - file, + Some(file), c.name.get_range(), "Not allowed here", - )))) + ))) } } Some(BuiltIns::DomainTransition) => { @@ -2706,6 +2728,9 @@ impl<'a> ValidatedStatement<'a> { ValidatedStatement::ResourcetransRule(r) => { ValidatedStatement::ResourcetransRule(r.get_renamed_statement(renames)) } + ValidatedStatement::FscRule(f) => { + ValidatedStatement::FscRule(f.get_renamed_statement(renames)) + } } } } diff --git a/src/lib.rs b/src/lib.rs index ba62da43..103528af 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1205,6 +1205,7 @@ mod tests { #[test] fn valid_self() { valid_policy_test("self.cas", &["allow qux self (file (read))"], &[]); + } #[test] fn valid_fs_context() { @@ -1213,10 +1214,11 @@ mod tests { &[ "fsuse xattr ext3 (system_u object_r foo ((s0) (s0)))", "fsuse task sockfs (system_u object_r foo ((s0) (s0)))", - "fsuse trans proc (system_u object_r foo ((s0) (s0)))", + "fsuse trans tmpfs (system_u object_r foo ((s0) (s0)))", "genfscon proc \"/\" (system_u object_r foo ((s0) (s0)))", - "genfscon sysfs \"/zap\" file (system_u object_r foo ((s0) (s0)))", - "genfscon tmpfs \"/\" (system_u object_r foo ((s0) (s0)))", + "genfscon sysfs \"/zap\" dir (system_u object_r foo ((s0) (s0)))", + "genfscon sysfs \"/zap/baa\" file (system_u object_r bar ((s0) (s0)))", + "genfscon cgroup \"/\" (system_u object_r foo ((s0) (s0)))", ], &[], ); @@ -1227,6 +1229,11 @@ mod tests { error_policy_test!("fs_context.cas", 5, ErrorItem::Compile(_)); } + #[test] + fn invalid_fs_context_dup() { + error_policy_test!("fs_context_dup.cas", 3, ErrorItem::InvalidFileSystem(_)); + } + #[test] fn invalid_resourcetrans() { error_policy_test!("resource_trans.cas", 5, ErrorItem::Compile(_)); From 3b6cfcf3518ce84798cdbf6bbabc323497fcb818 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Tue, 29 Nov 2022 11:45:07 -0700 Subject: [PATCH 04/19] forcing old genfscon behavior for time being --- data/expected_cil/fs_context.cil | 5 ---- data/policies/fs_context.cas | 10 ++++---- src/internal_rep.rs | 41 +++++++++++++++++++------------- src/lib.rs | 5 ++-- 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/data/expected_cil/fs_context.cil b/data/expected_cil/fs_context.cil index 9b097145..0d5956a0 100644 --- a/data/expected_cil/fs_context.cil +++ b/data/expected_cil/fs_context.cil @@ -109,9 +109,6 @@ (handleunknown allow) (typeattribute domain) (typeattribute resource) -(type bar) -(roletype object_r bar) -(typeattributeset resource (bar)) (type foo) (roletype object_r foo) (typeattributeset resource (foo)) @@ -130,8 +127,6 @@ (fsuse trans tmpfs (system_u object_r foo ((s0) (s0)))) (genfscon cgroup "/" (system_u object_r foo ((s0) (s0)))) (genfscon proc "/" (system_u object_r foo ((s0) (s0)))) -(genfscon sysfs "/zap" dir (system_u object_r foo ((s0) (s0)))) -(genfscon sysfs "/zap/baa" file (system_u object_r bar ((s0) (s0)))) (sid kernel) (sidcontext kernel (system_u system_r kernel_sid ((s0) (s0)))) (sid security) diff --git a/data/policies/fs_context.cas b/data/policies/fs_context.cas index 8da4c5e2..93ebe58a 100644 --- a/data/policies/fs_context.cas +++ b/data/policies/fs_context.cas @@ -7,12 +7,14 @@ resource foo { fs_context(this, "proc", genfscon, "/"); fs_context(this, "proc", genfscon, "/"); fs_context(this, "cgroup", genfscon); - fs_context(this, "sysfs", genfscon, "/zap", [dir]); + // TODO re-add when secilc check is in place + // fs_context(this, "sysfs", genfscon, "/zap", [dir]); // Policies must include at least one av rule allow(domain, foo, file, [read]); } -resource bar { - fs_context(this, "sysfs", genfscon, "/zap/baa", [file]); -} \ No newline at end of file +// TODO re-add when secilc check is in place +// resource bar { +// fs_context(this, "sysfs", genfscon, "/zap/baa", [file]); +//} \ No newline at end of file diff --git a/src/internal_rep.rs b/src/internal_rep.rs index f220d451..91844cdc 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1611,28 +1611,37 @@ impl From<&FileSystemContextRule<'_>> for sexp::Sexp { Sexp::from(&f.context), ]), FSContextType::GenFSCon => { - // Since path is an optional arg and I dont want to get + // Since path is an optional arg and I don't want to get // into unwrap issue we are doing an 'if let' here. The lack - // of path should be caught ealier, so if we dont have a path + // of path should be caught earlier, so if we don't have a path // we will return an empty list. The more correct way to fix this // is convert this to a try_from, but this causes issues with some // of our match statements and mixing returns. if let Some(p) = &f.path { - match &f.file_type { - Some(file_type) => list(&[ - atom_s("genfscon"), - atom_s(f.fs_name.trim_matches('"')), - atom_s(p.as_ref()), - Sexp::Atom(Atom::S(file_type.to_string())), - Sexp::from(&f.context), - ]), - None => list(&[ - atom_s("genfscon"), - atom_s(f.fs_name.trim_matches('"')), - atom_s(p.as_ref()), - Sexp::from(&f.context), - ]), + if let Some(file_type) = &f.file_type { + // TODO add secilc check here. Right now our github pipeline + // supports an older version of secilc. So to get things moving forward + // we are forcing the old behavior. The new behavior has been tested locally. + // REMEMBER TO UPDATE THE TESTS + // if secilc/libsepol version is new enough { + if false { + return list(&[ + atom_s("genfscon"), + atom_s(f.fs_name.trim_matches('"')), + atom_s(p.as_ref()), + Sexp::Atom(Atom::S(file_type.to_string())), + Sexp::from(&f.context), + ]); + } } + // We are purposefully falling through without an else to + // reduce redundant lines of code + list(&[ + atom_s("genfscon"), + atom_s(f.fs_name.trim_matches('"')), + atom_s(p.as_ref()), + Sexp::from(&f.context), + ]) } else { list(&[]) } diff --git a/src/lib.rs b/src/lib.rs index 103528af..c9304256 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1216,8 +1216,9 @@ mod tests { "fsuse task sockfs (system_u object_r foo ((s0) (s0)))", "fsuse trans tmpfs (system_u object_r foo ((s0) (s0)))", "genfscon proc \"/\" (system_u object_r foo ((s0) (s0)))", - "genfscon sysfs \"/zap\" dir (system_u object_r foo ((s0) (s0)))", - "genfscon sysfs \"/zap/baa\" file (system_u object_r bar ((s0) (s0)))", + // TODO re-add when secilc check is in place + // "genfscon sysfs \"/zap\" dir (system_u object_r foo ((s0) (s0)))", + // "genfscon sysfs \"/zap/baa\" file (system_u object_r bar ((s0) (s0)))", "genfscon cgroup \"/\" (system_u object_r foo ((s0) (s0)))", ], &[], From 25abe7648bde7f8130f5d060c438ce181a09609b Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Fri, 2 Dec 2022 09:22:21 -0700 Subject: [PATCH 05/19] review comments * Converted FileSystemContextRule to try_from * Added more error checking * formatting and typo fixes --- data/error_policies/fs_context.cas | 7 +- data/error_policies/fs_context_dup.cas | 4 +- data/policies/fs_context.cas | 4 +- doc/TE.md | 171 ++++++++++++++----------- src/compile.rs | 7 +- src/internal_rep.rs | 108 +++++++++------- src/lib.rs | 2 +- 7 files changed, 174 insertions(+), 129 deletions(-) diff --git a/data/error_policies/fs_context.cas b/data/error_policies/fs_context.cas index 71f7acd3..bd89a16a 100644 --- a/data/error_policies/fs_context.cas +++ b/data/error_policies/fs_context.cas @@ -5,7 +5,10 @@ resource foo { fs_context(this, "proc", zap); fs_context(this, "sysfs", genfscon, "/zap", [bar]); + fs_context(this, "sysfs", genfscon, "/zap", [file bar]); + fs_context(this, "fs1", xattr, "/zap", [file dir]); + fs_context(this, "fs2", task, "/zap"); - // Policies must include at least one av rule - allow(domain, foo, file, [read]); + // Policies must include at least one av rule + allow(domain, foo, file, [read]); } diff --git a/data/error_policies/fs_context_dup.cas b/data/error_policies/fs_context_dup.cas index 76687c5c..33ecede3 100644 --- a/data/error_policies/fs_context_dup.cas +++ b/data/error_policies/fs_context_dup.cas @@ -7,8 +7,8 @@ resource foo { fs_context(this, "test", genfscon, "/zap/baa", [file]); - // Policies must include at least one av rule - allow(domain, foo, file, [read]); + // Policies must include at least one av rule + allow(domain, foo, file, [read]); } resource bar { diff --git a/data/policies/fs_context.cas b/data/policies/fs_context.cas index 93ebe58a..ca639902 100644 --- a/data/policies/fs_context.cas +++ b/data/policies/fs_context.cas @@ -10,8 +10,8 @@ resource foo { // TODO re-add when secilc check is in place // fs_context(this, "sysfs", genfscon, "/zap", [dir]); - // Policies must include at least one av rule - allow(domain, foo, file, [read]); + // Policies must include at least one av rule + allow(domain, foo, file, [read]); } // TODO re-add when secilc check is in place diff --git a/doc/TE.md b/doc/TE.md index 660e1a62..d8df80b9 100644 --- a/doc/TE.md +++ b/doc/TE.md @@ -4,12 +4,12 @@ type enforcement, objects on the system (processes, files, sockets, etc) are each assigned a unique identifier known as a type. Policy rules specify how types may interact. -Type Enforcement policies have a subject, which attempts to perform some -action, an object which is acted upon, and a description of the action(s). In -SELinux this description is made up of an 'object class', which describes what -sort of resource the object is, and permissions (such as read or write) which -describe the access. Permissions are defined in relation to object classes (so -'listen' is a valid permission on sockets, but not on files). +Type Enforcement policies have a subject, which attempts to perform some action, +an object which is acted upon, and a description of the action(s). In SELinux +this description is made up of an 'object class', which describes what sort of +resource the object is, and permissions (such as read or write) which describe +the access. Permissions are defined in relation to object classes (so 'listen' +is a valid permission on sockets, but not on files). For more information, review the official SELinux documentation at https://selinuxproject.org/page/Main_Page @@ -22,8 +22,8 @@ types from the language perspective, however there are also additional types, described below. All other types in a policy must derive from one of the below types. -A 'domain' is a type describing a process on a Linux system and therefore may -be a subject in a TE rule.[1] Domains may also be the object in a TE rule. Two +A 'domain' is a type describing a process on a Linux system and therefore may be +a subject in a TE rule.[1] Domains may also be the object in a TE rule. Two common examples of this are: 1. Files in /proc typically have labels matching the process they describe. @@ -45,8 +45,8 @@ The block enclosed in curly braces may be used to specify access rules and member functions on this type. The rationale for this distinction between domains and resources is that it -allows us to take advantage of this semantic knowledge at compile time for -use in built in functions, syntax improvements and linting. +allows us to take advantage of this semantic knowledge at compile time for use +in built in functions, syntax improvements and linting. Cascade is statically typed and function calls must specify the types of their arguments. For arguments that may be either domains or resources, the "type" @@ -58,8 +58,8 @@ Additional built in types are listed below. Lists of objects of the same type are supported and specified by the syntax [type]. -[1] In the SELinux base policy language, non-domains may actually be subjects -in certain contexts (e.g. the filesystem associate permission). In Cascade we +[1] In the SELinux base policy language, non-domains may actually be subjects in +certain contexts (e.g. the filesystem associate permission). In Cascade we enforce the rule that only domains may be subjects and handle situations where resources are subjects through another mechanism (to be determined). @@ -86,10 +86,10 @@ extend foo { Policy authors can define functions using the keyword 'fn'. Functions defined inside the curly block for a type will be member functions of that type, and will be able to refer to the containing type using the keyword 'this'. To call -a member function, you use the `.` operator, with the name of the type, -followed by a `.`, followed by the function name. For example if the read() -function is defined on the `iptables_conf` resource, then it can be called -using `iptables_conf.read()`. +a member function, you use the `.` operator, with the name of the type, followed +by a `.`, followed by the function name. For example if the read() function is +defined on the `iptables_conf` resource, then it can be called using +`iptables_conf.read()`. If a function is called inside a curly block, Cascade will attempt to pass the type of the curly block as the first argument to the function. If this is not @@ -112,8 +112,8 @@ domain iptables { A function marked with the `virtual` keyword is not allowed to be called directly. The purpose of such functions is to be inherited and called on the child type. If a parent function is marked virtual the child *must* define it, -either explicitely or via a derive (if some parent implementation exists). -This is used to define an expected interface that children are guaranteed to +either explicitely or via a derive (if some parent implementation exists). This +is used to define an expected interface that children are guaranteed to implement. ### Type inheritance @@ -124,19 +124,19 @@ Child types inherit the following from their parents: When inheriting from multiple types, different types may provide member functions with conflicting names (in object oriented language parlance, this is -commonly refered to as "the diamond problem"). If that is the case, the -derived type *must* override that member function with one of its own. This -can be done by manually defining a new funtion (which may call parent classes -using the syntax classname::function_name()), or using the "derive" annotation. -See the section on annotations for details on deriving. +commonly refered to as "the diamond problem"). If that is the case, the derived +type *must* override that member function with one of its own. This can be done +by manually defining a new funtion (which may call parent classes using the +syntax classname::function_name()), or using the "derive" annotation. See the +section on annotations for details on deriving. The common case for type inheritance will be inheriting from virtual types, which conveniently allows reference to all derived types, automatically generates common resources, and reduces boilerplate by including common functions. -Inheriting from concrete types can be desirable for various applications, and -is made more useful than previous cloning implementations by the resource +Inheriting from concrete types can be desirable for various applications, and is +made more useful than previous cloning implementations by the resource association mechanism, but is not currently supported in the current version. ### Virtual types @@ -193,8 +193,8 @@ Associating a resource with a domain does three things: 1. It causes that resource to be grouped together with the domain when the domain is included in a module or full system policy -2. It calls any @associated_call member functions in the resource with the domain -as the first argument. +2. It calls any @associated_call member functions in the resource with the +domain as the first argument. 3. If the domain is inherited from, a child resource is automatically created and is associated with the child class. This resource is named `[child name].[resource name]`. @@ -209,11 +209,11 @@ Access vector rules define what happens on an access attempt. They are defined on the quadruple of: source type, target type, target object class, permissions. There are five kinds of access vector rules provided by Cascade: allow, -auditallow, dontaudit, neverallow, delete. Allow rules grant access. -Auditallow audit access when it is granted (by a separate allow rule). -Dontaudit rules disable auditing for denied access. Neverallow rules are a -compile time assertion that certain access should not be allowed. Delete rules -remove access if it is allowed elsewhere. +auditallow, dontaudit, neverallow, delete. Allow rules grant access. Auditallow +audit access when it is granted (by a separate allow rule). Dontaudit rules +disable auditing for denied access. Neverallow rules are a compile time +assertion that certain access should not be allowed. Delete rules remove access +if it is allowed elsewhere. These rules are defined by five built-in functions: allow(), audit(), dontaudit(), neverallow(), delete(). Note the rename from the SELinux base @@ -227,9 +227,9 @@ fn allow(domain source, type target, class obj_class, [permission] perm); ``` And likewise for audit(), dontaudit(), neverallow() and delete(). Lists of -sources and targets are intentionally omitted to encourage clarity and readability. -Policy authors are encouraged to create virtual types to specify groups and/or -declare their own functions grouping related av rules. +sources and targets are intentionally omitted to encourage clarity and +readability. Policy authors are encouraged to create virtual types to specify +groups and/or declare their own functions grouping related av rules. Note the use the makelist annotation to allow automatic coercion from a single class or permission to a list (See 'annotations' section). @@ -254,8 +254,8 @@ Default contexts specify what labels should be applied to objects that cannot store labels in extended attributes. For example, files on filesystems that do not support extended attributes would get default contexts. Additionally, certain non-file objects such as ports or packets will get contexts on creation -as defined by the policy (although packets have their own special handling -which is outside the scope of this document). +as defined by the policy (although packets have their own special handling which +is outside the scope of this document). Reference Policy implementations treat these types of labeling independently, which reflects how they are handled at a kernel/system level, but not how high @@ -289,32 +289,56 @@ TODO: Named resource transitions TODO: File context labeling -This is under discussion now. We agree that we should take advantage of the fact that we are parsing paths, not arbitrary strings to provide appropriate semantic checking. Mickael to provide motivating use case for query syntax approach. +This is under discussion now. We agree that we should take advantage of the +fact that we are parsing paths, not arbitrary strings to provide appropriate +semantic checking. Mickael to provide motivating use case for query syntax +approach. TODO: default contexts. This will be part of the next phase of design -TODO: file_contexts.subs_dist (although is it necessary with advanced file_context handling features?) +TODO: file_contexts.subs_dist (although is it necessary with advanced +file_context handling features?) ### Filesystem labeling -All filesystem are labeled using the following prototype: +All filesystems are labeled using the following prototype: -`fs_context(resource fs_label, string fs_name, fs_type type, path path, file_type [obj_class])` +`fs_context(resource fs_label, string fs_name, fs_type type, path path, +file_type [obj_class])` -* `fs_label` is label you wish to apply to the filesystem. -* `fs_name` is the OS recognized name for the filesystem. (e.g. ext4, proc, tmpfs) +* `fs_label` is the label you wish to apply to the filesystem. +* `fs_name` is the OS recognized name for the filesystem. (e.g. "ext4", "proc", + "tmpfs") * `fs_type` must be one of the following options: - * `xattr`: Filesystems supporting the extended attribute security.selinux. The labeling is persistent for filesystems that support extended attributes. - * `task`: For pseudo filesystems supporting task related services such as pipes and sockets. - * `trans`: For pseudo filesystems such as pseudo terminals and temporary objects. - * `genfscon`: Used to allocate a security context to filesystems that cannot support any of the previous file labeling options -* `path` is an optional path relative to root of the mounted filesystem. Currently this is only valid for the proc filesystem, all other types must be "/". If not given, the field will default to "/". -* `file_type` is an optional keyword representing a file type to apply the label to. Valid values are the same as in file_context function. If not given, [any] is assumed. - * Note: You must use SELinux userspace tools version 3.4 or newer to use this field. - -`xattr`, `task`, and `trans` all represent filesystems that support SELinux security contexts. The filesystem itself has a labeled applied to it as a whole, which is the `fs_label` provided in this function. All files on the filesystem also store their own SELinux security context in their own extended attributes. - -`genfscon` represents filesystems that do not support SELinux security contexts. Generally a filesystem has a single default security context, `fs_label` provided in this function, assigned by `genfscon` from the root (/) that is inherited by all files and directories on that filesystem. The exception to this is the /proc filesystem, where directories can be labeled with a specific security context. + * `xattr`: Filesystems supporting the extended attribute security.selinux. The + labeling is persistent for filesystems that support extended attributes. + * `task`: For pseudo filesystems supporting task related services such as + pipes and sockets. + * `trans`: For pseudo filesystems such as pseudo terminals and temporary + objects. + * `genfscon`: Used to allocate a security context to filesystems that cannot + support any of the previous file labeling options +* `path` is an optional path relative to root of the mounted filesystem. + Currently this is only valid for the proc filesystem, all other types must be + "/". If not given, the field will default to "/". +* `file_type` is an optional keyword representing a file type to apply the label + to. Valid values are the same as in file_context function. If not given, + [any] is assumed. + * Note: You must use SELinux userspace tools version 3.4 or newer to use this + field. + +`xattr`, `task`, and `trans` all represent filesystems that support SELinux +security contexts. The filesystem itself has a labeled applied to it as a +whole, which is the `fs_label` provided in this function. All files on the +filesystem also store their own SELinux security context in their own extended +attributes. + +`genfscon` represents filesystems that do not support SELinux security contexts. +Generally a filesystem has a single default security context, `fs_label` +provided in this function, assigned by `genfscon` from the root (/) that is +inherited by all files and directories on that filesystem. The exception to this +is the /proc filesystem, where directories can be labeled with a specific +security context. This call must be part of a resource block. @@ -337,8 +361,8 @@ let read_file_perms = [ read, open, getattr ]; ``` The name `read_file_perms` can be used elsewhere and will be replaced at compile -time with the list of permissions. The compiler infers the type of the -constant from what is assigned to it (this is case the type is [perm]). +time with the list of permissions. The compiler infers the type of the constant +from what is assigned to it (this is case the type is [perm]). ## Annotations @@ -404,15 +428,12 @@ Example using additional fields: domain foo { ... } ``` -The following arguments may be specified: -source: The scontext field of a denial -target: The tcontext field of a denial -class: The tclass field of a denial -perm: Any permission listed in a denial -hint: A text string to display when using newaudit2allow -attack: Set to True to indicate that this denial should be treated as a -possible security incident -cve: Reference a CVE associated with this denial +The following arguments may be specified: source: The scontext field of a denial +target: The tcontext field of a denial class: The tclass field of a denial perm: +Any permission listed in a denial hint: A text string to display when using +newaudit2allow attack: Set to True to indicate that this denial should be +treated as a possible security incident cve: Reference a CVE associated with +this denial ### ignore annotation @@ -426,11 +447,11 @@ if (condition) { ``` The above code would ordinarily display a warning because conditional -definitions can lead to unexpected behavior. If we wish to leave the code as -is and suppress the warning, we can do so via this annotation. Of course, -warnings are typically supplied for good reason, and you should seriously -consider whether you really want to suppress the warning rather than fixing -the underlying issue. +definitions can lead to unexpected behavior. If we wish to leave the code as is +and suppress the warning, we can do so via this annotation. Of course, warnings +are typically supplied for good reason, and you should seriously consider +whether you really want to suppress the warning rather than fixing the +underlying issue. ### makelist annotation @@ -452,14 +473,12 @@ baz(bar); // Converts to [bar] because of annotation. Would be a compiler error The alias annotation tells the compiler to provide an alternate name for referrering to the same item. -This is often used for interoperability. For example, if one is renaming a -type or function in an already deployed policy, one can provide an alias to the -old name for backwards compatibility during a transition period until labels or +This is often used for interoperability. For example, if one is renaming a type +or function in an already deployed policy, one can provide an alias to the old +name for backwards compatibility during a transition period until labels or callers have been updated. -`` -@alias(bar) -resource foo {} +`` @alias(bar) resource foo {} ``` ## Traits diff --git a/src/compile.rs b/src/compile.rs index c2363aea..3391b9cc 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -49,7 +49,7 @@ pub fn generate_sexp( // TODO: The rest of compilation let cil_types = type_list_to_sexp(type_decl_list, type_map); let headers = generate_cil_headers(classlist, *machine_configurations); - let cil_rules = rules_list_to_sexp(policy_rules); + let cil_rules = rules_list_to_sexp(policy_rules)?; let cil_macros = func_map_to_sexp(func_map)?; let sid_statements = generate_sid_rules(generate_sids("kernel_sid", "security_sid", "unlabeled_sid")); @@ -1684,11 +1684,12 @@ fn get_rules_vec_for_type(ti: &TypeInfo, s: sexp::Sexp, type_map: &TypeMap) -> V ret } -fn rules_list_to_sexp<'a, T>(rules: T) -> Vec +fn rules_list_to_sexp<'a, T>(rules: T) -> Result, ErrorItem> where T: IntoIterator>, { - rules.into_iter().map(|r| Sexp::from(&r)).collect() + let ret: Result, _> = rules.into_iter().map(|r| Sexp::try_from(&r)).collect(); + ret } fn generate_sids<'a>( diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 91844cdc..dfd267d2 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -18,7 +18,7 @@ use crate::ast::{ }; use crate::constants; use crate::context::{BlockType, Context as BlockContext}; -use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; +use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError, InvalidFileSystemError}; use crate::obj_class::perm_list_to_sexp; const DEFAULT_USER: &str = "system_u"; @@ -1600,23 +1600,18 @@ pub struct FileSystemContextRule<'a> { pub context: Context<'a>, } -// TODO convert to TryFrom/try_from see comment below -impl From<&FileSystemContextRule<'_>> for sexp::Sexp { - fn from(f: &FileSystemContextRule) -> sexp::Sexp { +impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { + type Error = ErrorItem; + + fn try_from(f: &FileSystemContextRule) -> Result { match f.fscontext_type { - FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => list(&[ + 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('"')), Sexp::from(&f.context), - ]), + ])), FSContextType::GenFSCon => { - // Since path is an optional arg and I don't want to get - // into unwrap issue we are doing an 'if let' here. The lack - // of path should be caught earlier, so if we don't have a path - // we will return an empty list. The more correct way to fix this - // is convert this to a try_from, but this causes issues with some - // of our match statements and mixing returns. if let Some(p) = &f.path { if let Some(file_type) = &f.file_type { // TODO add secilc check here. Right now our github pipeline @@ -1625,25 +1620,31 @@ impl From<&FileSystemContextRule<'_>> for sexp::Sexp { // REMEMBER TO UPDATE THE TESTS // if secilc/libsepol version is new enough { if false { - return list(&[ + return Ok(list(&[ atom_s("genfscon"), atom_s(f.fs_name.trim_matches('"')), atom_s(p.as_ref()), Sexp::Atom(Atom::S(file_type.to_string())), Sexp::from(&f.context), - ]); + ])); } } // We are purposefully falling through without an else to // reduce redundant lines of code - list(&[ + Ok(list(&[ atom_s("genfscon"), atom_s(f.fs_name.trim_matches('"')), atom_s(p.as_ref()), Sexp::from(&f.context), - ]) + ])) } else { - list(&[]) + Err(ErrorItem::InvalidFileSystem(InvalidFileSystemError::new( + &format!( + "Genfscon missing path.\n No path given for genfscon rule:\ + \n\tFilesystem name: {}\n\tContext: {}", + f.fs_name, f.context, + ), + ))) } } } @@ -1755,30 +1756,49 @@ fn call_to_fsc_rules<'a>( return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "Not a valid file system type", Some(file), - fscontext_str.get_range(), //TODO error not showing correctly + fscontext_str.get_range(), "File system type must be 'xattr', 'task', 'trans', or 'genfscon'", ))); } }; - let regex_string = args_iter + let regex_string_arg = args_iter .next() - .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? - .get_name_or_string(context)? - .to_string(); - let file_types = args_iter + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))?; + let regex_string = regex_string_arg.get_name_or_string(context)?.to_string(); + let file_types_arg = args_iter .next() - .ok_or_else(|| ErrorItem::Internal(InternalError::new()))? - .get_list(context)?; + .ok_or_else(|| ErrorItem::Internal(InternalError::new()))?; + let file_types = file_types_arg.get_list(context)?; match fscontext_type { FSContextType::XAttr | FSContextType::Task | FSContextType::Trans => { - ret.push(FileSystemContextRule { - fscontext_type, - fs_name, - path: None, - file_type: None, - context: fs_context.clone(), - }); + // The 'regex_string_arg.get_range().is_none()' is a hacky way to + // to check if arg was actually provided or not. Since we set the + // default for regex_string to "/" this is the only way I could find + // to test if the actual arg was passed or not + if regex_string_arg.get_range().is_none() && file_types.is_empty() { + ret.push(FileSystemContextRule { + fscontext_type, + fs_name, + path: None, + file_type: None, + context: fs_context.clone(), + }); + } else if !file_types.is_empty() { + return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( + "File types can only be provided for 'genfscon'", + Some(file), + file_types_arg.get_range(), + "", + ))); + } else { + return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( + "File path can only be provided for 'genfscon'", + Some(file), + regex_string_arg.get_range(), + "", + ))); + } } FSContextType::GenFSCon => { if file_types.is_empty() { @@ -2498,7 +2518,7 @@ impl TryFrom<&FunctionInfo<'_>> for sexp::Sexp { ValidatedStatement::AvRule(a) => macro_cil.push(Sexp::from(a)), ValidatedStatement::FcRule(f) => macro_cil.push(Sexp::from(f)), ValidatedStatement::ResourcetransRule(r) => macro_cil.push(Sexp::from(r)), - ValidatedStatement::FscRule(fs) => macro_cil.push(Sexp::from(fs)), + ValidatedStatement::FscRule(fs) => macro_cil.push(Sexp::try_from(fs)?), ValidatedStatement::DomtransRule(d) => macro_cil.push(Sexp::from(d)), } } @@ -2656,7 +2676,7 @@ impl<'a> ValidatedStatement<'a> { .collect()) } else { Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "file_context() calls are only allowed in resources", + "fs_context() calls are only allowed in resources", Some(file), c.name.get_range(), "Not allowed here", @@ -2744,15 +2764,16 @@ impl<'a> ValidatedStatement<'a> { } } -impl From<&ValidatedStatement<'_>> for sexp::Sexp { - fn from(statement: &ValidatedStatement) -> sexp::Sexp { +impl TryFrom<&ValidatedStatement<'_>> for sexp::Sexp { + type Error = ErrorItem; + fn try_from(statement: &ValidatedStatement) -> Result { match statement { - ValidatedStatement::Call(c) => Sexp::from(&**c), - ValidatedStatement::AvRule(a) => Sexp::from(a), - ValidatedStatement::FcRule(f) => Sexp::from(f), - ValidatedStatement::ResourcetransRule(r) => Sexp::from(r), - ValidatedStatement::FscRule(fs) => Sexp::from(fs), - ValidatedStatement::DomtransRule(d) => Sexp::from(d), + ValidatedStatement::Call(c) => Ok(Sexp::from(&**c)), + ValidatedStatement::AvRule(a) => Ok(Sexp::from(a)), + ValidatedStatement::FcRule(f) => Ok(Sexp::from(f)), + ValidatedStatement::ResourcetransRule(r) => Ok(Sexp::from(r)), + ValidatedStatement::FscRule(fs) => Sexp::try_from(fs), + ValidatedStatement::DomtransRule(d) => Ok(Sexp::from(d)), } } } @@ -3862,7 +3883,8 @@ mod tests { let mut renames = BTreeMap::new(); renames.insert("old_name".to_string(), "new_name".to_string()); let renamed_statement = statement.get_renamed_statement(&renames); - let sexp = Sexp::from(&renamed_statement); + // Since this is a controlled test these should always unrwap + let sexp = Sexp::try_from(&renamed_statement).unwrap(); assert!(sexp.to_string().contains("new_name")); assert!(!sexp.to_string().contains("old_name")); } diff --git a/src/lib.rs b/src/lib.rs index c9304256..63421e33 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1227,7 +1227,7 @@ mod tests { #[test] fn invalid_fs_context() { - error_policy_test!("fs_context.cas", 5, ErrorItem::Compile(_)); + error_policy_test!("fs_context.cas", 8, ErrorItem::Compile(_)); } #[test] From d3d16673543b409c39dc879c899f6a4c2db7d173 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 7 Dec 2022 03:19:16 -0700 Subject: [PATCH 06/19] additional review comments --- doc/TE.md | 8 ++++---- src/compile.rs | 41 ++++++++++++++++++++++------------------- src/internal_rep.rs | 14 ++++++++++---- src/lib.rs | 2 +- 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/doc/TE.md b/doc/TE.md index d8df80b9..c7c139b4 100644 --- a/doc/TE.md +++ b/doc/TE.md @@ -322,16 +322,16 @@ file_type [obj_class])` Currently this is only valid for the proc filesystem, all other types must be "/". If not given, the field will default to "/". * `file_type` is an optional keyword representing a file type to apply the label - to. Valid values are the same as in file_context function. If not given, + to. Valid values are the same as in the file_context function. If not given, [any] is assumed. * Note: You must use SELinux userspace tools version 3.4 or newer to use this field. `xattr`, `task`, and `trans` all represent filesystems that support SELinux security contexts. The filesystem itself has a labeled applied to it as a -whole, which is the `fs_label` provided in this function. All files on the -filesystem also store their own SELinux security context in their own extended -attributes. +whole, which is the `fs_label` provided in this function. Individual files +may also have specific security contexts stored in their extended attributes +if supported by the filesystem. `genfscon` represents filesystems that do not support SELinux security contexts. Generally a filesystem has a single default security context, `fs_label` diff --git a/src/compile.rs b/src/compile.rs index 3391b9cc..e476b47a 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -342,8 +342,9 @@ pub fn build_func_map<'a>( Ok(decl_map) } -#[allow(clippy::collapsible_if)] -pub fn validate_rules(statements: &BTreeSet) -> Result<(), CascadeErrors> { +pub fn validate_fs_context_duplicates( + statements: &BTreeSet, +) -> Result<(), CascadeErrors> { let mut errors = CascadeErrors::new(); let mut fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>> = BTreeMap::new(); @@ -371,7 +372,7 @@ pub fn validate_rules(statements: &BTreeSet) -> Result<(), C continue 'key_loop; } FSContextType::GenFSCon => { - // genfscon gets more complicated. We can have simlar rules as long as the paths different. + // genfscon gets more complicated. We can have similar rules as long as the paths are different. // If we find a genfscon with the same path, they must have the same context and object type. if let Some(path) = &rule.path { for inner_rule in &v { @@ -386,25 +387,19 @@ pub fn validate_rules(statements: &BTreeSet) -> Result<(), C inner_rule.context, )))); continue 'key_loop; - // else if let is not suppored currently (https://github.com/rust-lang/rust/issues/53667). - // Thus we check if we are not none and then unwrap } else if path == inner_path + && rule.file_type != inner_rule.file_type && rule.file_type.is_some() - && inner_rule.file_type.is_some() { - if rule.file_type.as_ref().unwrap() - != inner_rule.file_type.as_ref().unwrap() - { - errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( - "Duplicate genfscon.\n Found duplicate genfscon rules with differing object types: {}\ - \n\tPath: {}\n\tObject Type 1: {}\n\tObject Type 2: {}", - rule.fs_name, - path, - rule.file_type.as_ref().unwrap(), - inner_rule.file_type.as_ref().unwrap(), - )))); - continue 'key_loop; - } + errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( + "Duplicate genfscon.\n Found duplicate genfscon rules with differing object types: {}\ + \n\tPath: {}\n\tObject Type 1: {}\n\tObject Type 2: {}", + rule.fs_name, + path, + rule.file_type.as_ref().unwrap(), + inner_rule.file_type.as_ref().unwrap(), + )))); + continue 'key_loop; } } } @@ -413,7 +408,15 @@ pub fn validate_rules(statements: &BTreeSet) -> Result<(), C } } } + errors.into_result(()) +} + +pub fn validate_rules(statements: &BTreeSet) -> Result<(), CascadeErrors> { + let mut errors = CascadeErrors::new(); + if let Err(call_errors) = validate_fs_context_duplicates(statements) { + errors.append(call_errors); + } errors.into_result(()) } diff --git a/src/internal_rep.rs b/src/internal_rep.rs index dfd267d2..b5425c54 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1784,21 +1784,27 @@ fn call_to_fsc_rules<'a>( file_type: None, context: fs_context.clone(), }); - } else if !file_types.is_empty() { - return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( + } + let mut errors = CascadeErrors::new(); + if !file_types.is_empty() { + errors.append(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "File types can only be provided for 'genfscon'", Some(file), file_types_arg.get_range(), "", ))); - } else { - return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( + } + if regex_string_arg.get_range().is_some() { + errors.append(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( "File path can only be provided for 'genfscon'", Some(file), regex_string_arg.get_range(), "", ))); } + if !errors.is_empty() { + return Err(errors); + } } FSContextType::GenFSCon => { if file_types.is_empty() { diff --git a/src/lib.rs b/src/lib.rs index 63421e33..b47b7f5c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1227,7 +1227,7 @@ mod tests { #[test] fn invalid_fs_context() { - error_policy_test!("fs_context.cas", 8, ErrorItem::Compile(_)); + error_policy_test!("fs_context.cas", 9, ErrorItem::Compile(_)); } #[test] From 34fa9ab8d743b0f9d8403272be738e1e0a2d8045 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Mon, 12 Dec 2022 09:13:48 -0700 Subject: [PATCH 07/19] fixing rebase issues --- src/internal_rep.rs | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/internal_rep.rs b/src/internal_rep.rs index b5425c54..1a26ad52 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1600,6 +1600,18 @@ pub struct FileSystemContextRule<'a> { pub context: Context<'a>, } +impl FileSystemContextRule<'_> { + fn get_renamed_statement(&self, renames: &BTreeMap) -> Self { + FileSystemContextRule { + fscontext_type: self.fscontext_type.clone(), + fs_name: self.fs_name.clone(), + path: self.path.clone(), + file_type: self.file_type, + context: self.context.get_renamed_context(renames), + } + } +} + impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { type Error = ErrorItem; @@ -1651,18 +1663,6 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { } } -impl FileSystemContextRule<'_> { - fn get_renamed_statement(&self, renames: &BTreeMap) -> Self { - FileSystemContextRule { - fscontext_type: self.fscontext_type.clone(), - fs_name: self.fs_name.clone(), - path: self.path.clone(), - file_type: self.file_type, - context: self.context.get_renamed_context(renames), - } - } -} - fn call_to_fsc_rules<'a>( c: &'a FuncCall, types: &'a TypeMap, @@ -3889,10 +3889,16 @@ mod tests { let mut renames = BTreeMap::new(); renames.insert("old_name".to_string(), "new_name".to_string()); let renamed_statement = statement.get_renamed_statement(&renames); - // Since this is a controlled test these should always unrwap - let sexp = Sexp::try_from(&renamed_statement).unwrap(); - assert!(sexp.to_string().contains("new_name")); - assert!(!sexp.to_string().contains("old_name")); + match Sexp::try_from(&renamed_statement) { + Ok(sexp) => { + assert!(sexp.to_string().contains("new_name")); + assert!(!sexp.to_string().contains("old_name")); + } + Err(_) => { + // We should never get here in testing but if we do assert false + assert!(false); + } + } } } } From 34a51a2af73c4b00d3fdeba6791f63cccc288f71 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Mon, 12 Dec 2022 09:59:10 -0700 Subject: [PATCH 08/19] fixing fmt --- src/constants.rs | 4 ++-- src/internal_rep.rs | 3 ++- src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index a717d31b..72baeeb5 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -28,8 +28,8 @@ pub const SELF: &str = "self"; pub const FS_TYPE: &str = "fs_type"; pub const BUILT_IN_TYPES: &[&str] = &[ - DOMAIN, RESOURCE, MODULE, "path", "string", CLASS, PERM, "context", SELF, FS_TYPE, - "xattr", "task", "trans", "genfscon", "*", + DOMAIN, RESOURCE, MODULE, "path", "string", CLASS, PERM, "context", SELF, FS_TYPE, "xattr", + "task", "trans", "genfscon", "*", ]; pub const SYSTEM_CONFIG_DEFAULTS: &[(&str, &str)] = diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 1a26ad52..85dd67e0 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1722,7 +1722,8 @@ fn call_to_fsc_rules<'a>( None, )?, ]; - let validated_args = validate_arguments(c, &target_args, types, class_perms, context, Some(file))?; + let validated_args = + validate_arguments(c, &target_args, types, class_perms, context, Some(file))?; let mut args_iter = validated_args.iter(); let mut ret = Vec::new(); diff --git a/src/lib.rs b/src/lib.rs index b47b7f5c..ff998f2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1206,7 +1206,7 @@ mod tests { fn valid_self() { valid_policy_test("self.cas", &["allow qux self (file (read))"], &[]); } - + #[test] fn valid_fs_context() { valid_policy_test( From e715d688a3253cab2642efde4dba563a59d69f76 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 14 Dec 2022 13:16:31 -0700 Subject: [PATCH 09/19] moved some logic --- src/compile.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index e476b47a..72dcbd73 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -343,17 +343,9 @@ pub fn build_func_map<'a>( } pub fn validate_fs_context_duplicates( - statements: &BTreeSet, + fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>>, ) -> Result<(), CascadeErrors> { let mut errors = CascadeErrors::new(); - let mut fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>> = 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); - } - } 'key_loop: for (_, v) in fsc_rules { // We only have 1 or 0 elements, thus we cannot have a semi duplicate @@ -414,7 +406,15 @@ pub fn validate_fs_context_duplicates( pub fn validate_rules(statements: &BTreeSet) -> Result<(), CascadeErrors> { let mut errors = CascadeErrors::new(); - if let Err(call_errors) = validate_fs_context_duplicates(statements) { + let mut fsc_rules: BTreeMap<&String, BTreeSet<&FileSystemContextRule>> = 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); + } + } + + if let Err(call_errors) = validate_fs_context_duplicates(fsc_rules) { errors.append(call_errors); } errors.into_result(()) From 0ad12d4c72e0355effc0b00bb56ae04e6260e87c Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Thu, 15 Dec 2022 08:10:07 -0700 Subject: [PATCH 10/19] Large overhaul of error handling and doc update --- Cargo.toml | 1 + data/error_policies/fs_context_dup.cas | 7 ++ doc/TE.md | 16 +++- src/compile.rs | 127 ++++++++++++++++++++----- src/error.rs | 26 +---- src/internal_rep.rs | 45 ++++++--- src/lib.rs | 4 +- 7 files changed, 159 insertions(+), 67 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c00295f8..9e287340 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ 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/data/error_policies/fs_context_dup.cas b/data/error_policies/fs_context_dup.cas index 33ecede3..38d7b006 100644 --- a/data/error_policies/fs_context_dup.cas +++ b/data/error_policies/fs_context_dup.cas @@ -1,9 +1,12 @@ resource foo { fs_context(foo, "ext3", xattr); fs_context(foo, "ext3", task); + fs_context(foo, "ext3", trans); fs_context(this, "sysfs", genfscon, "/zap", [dir]); fs_context(this, "sysfs", genfscon, "/zap", [file]); + fs_context(this, "sysfs", genfscon, "/zap", [any]); + fs_context(this, "sysfs", genfscon, "/zap"); fs_context(this, "test", genfscon, "/zap/baa", [file]); @@ -13,4 +16,8 @@ resource foo { resource bar { fs_context(this, "test", genfscon, "/zap/baa", [file]); +} + +resource xyz { + fs_context(this, "test", genfscon, "/zap/baa", [file]); } \ No newline at end of file diff --git a/doc/TE.md b/doc/TE.md index c7c139b4..ac3e8e34 100644 --- a/doc/TE.md +++ b/doc/TE.md @@ -328,10 +328,18 @@ file_type [obj_class])` field. `xattr`, `task`, and `trans` all represent filesystems that support SELinux -security contexts. The filesystem itself has a labeled applied to it as a -whole, which is the `fs_label` provided in this function. Individual files -may also have specific security contexts stored in their extended attributes -if supported by the filesystem. +security contexts. The filesystem itself has a label applied to it as a +whole, which is the `fs_label` provided in this function. +* As stated above, `xattr` supports the security.selinux extended attribute. + This means objects on the filesystem have persistent labeling stored in this + attribute. +* The `task` pseudo filesystem typically assigns the context of the creating + process to the object. +* The `trans` pseudo filesystem typically assigns the context based on + the context of the creating process and the context associated with the + filesystem type. This normally takes the form of a `resource_transition` + call. If no `resource_transition` call is present, the object is assigned + the label of filesystem. `genfscon` represents filesystems that do not support SELinux security contexts. Generally a filesystem has a single default security context, `fs_label` diff --git a/src/compile.rs b/src/compile.rs index 72dcbd73..86059388 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -5,6 +5,7 @@ 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, @@ -12,7 +13,7 @@ use crate::ast::{ }; use crate::constants; use crate::context::{BlockType, Context as BlockContext}; -use crate::error::{CascadeErrors, ErrorItem, InternalError, InvalidFileSystemError}; +use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; use crate::internal_rep::{ argument_to_typeinfo, argument_to_typeinfo_vec, generate_sid_rules, type_slice_to_variant, validate_derive_args, Annotated, AnnotationInfo, ArgForValidation, Associated, BoundTypeInfo, @@ -352,16 +353,32 @@ pub fn validate_fs_context_duplicates( if v.len() <= 1 { continue; } + let mut error: Option = None; 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 => { - // If we ever see a duplicate and one of them is xattr, task, or trans something is wrong - // return an error and look for more. - errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( - "Duplicate filesystem context.\n Found two different filesystem type declarations for filesystem: {}", - rule.fs_name - )))); - continue 'key_loop; + let mut range: Range = 0..0; + 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 is not None so we have already found something + if let Some(unwrapped_error) = error { + error = Some(unwrapped_error.add_additional_message(&rule.file, + range, + &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); + } else { + // error is none so we need to make a new one. + error = Some(CompileError::new( + "Duplicate filesystem context.", + &rule.file, + range, + &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); + } } FSContextType::GenFSCon => { // genfscon gets more complicated. We can have similar rules as long as the paths are different. @@ -370,35 +387,93 @@ pub fn validate_fs_context_duplicates( for inner_rule in &v { if let Some(inner_path) = &inner_rule.path { if path == inner_path && rule.context != inner_rule.context { - errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( - "Duplicate genfscon.\n Found duplicate genfscon rules with differing contexts: {}\ - \n\tPath: {}\n\tContext 1: {}\n\tContext 2: {}", - rule.fs_name, - path, - rule.context, - inner_rule.context, - )))); - continue 'key_loop; + // 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 + if let Some(unwrapped_error) = error { + error = Some(unwrapped_error.add_additional_message(&inner_rule.file, + inner_range.unwrap_or_default(), + &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( + "Duplicate genfscon contexts", + &rule.file, + outer_range.unwrap_or_default(), + &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, + inner_range.unwrap_or_default(), + &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context))); + } } else if path == inner_path && rule.file_type != inner_rule.file_type && rule.file_type.is_some() { - errors.append(CascadeErrors::from(InvalidFileSystemError::new(&format!( - "Duplicate genfscon.\n Found duplicate genfscon rules with differing object types: {}\ - \n\tPath: {}\n\tObject Type 1: {}\n\tObject Type 2: {}", - rule.fs_name, - path, - rule.file_type.as_ref().unwrap(), - inner_rule.file_type.as_ref().unwrap(), - )))); - continue 'key_loop; + let mut inner_range: Option> = None; + 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 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 + if let Some(unwrapped_error) = error { + error = Some(unwrapped_error.add_additional_message(&inner_rule.file, + inner_range.unwrap_or_default(), + &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); + } else { + 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( + "Duplicate genfscon file types", + &inner_rule.file, + outer_range.unwrap_or_default(), + &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, + inner_range.unwrap_or_default(), + &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); + } } } } + // If we have found an error we don't want to look through + // the inner loop again because it will cause duplicate errors + // in the "other" matching directions. + // So an error for A -> B and B -> A + if let Some(unwraped_error) = error { + errors.add_error(unwraped_error); + continue 'key_loop; + } } } } } + if let Some(unwraped_error) = error { + errors.add_error(unwraped_error); + } } errors.into_result(()) } diff --git a/src/error.rs b/src/error.rs index a97a65fb..6dbaf1a4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -211,21 +211,6 @@ impl InvalidMachineError { } } -#[derive(Error, Clone, Debug)] -#[error("{diagnostic}")] -pub struct InvalidFileSystemError { - pub diagnostic: Diag, -} - -impl InvalidFileSystemError { - pub fn new(msg: &str) -> Self { - let diagnostic = Diagnostic::error().with_message(msg); - InvalidFileSystemError { - diagnostic: diagnostic.into(), - } - } -} - #[derive(Error, Debug)] pub enum ErrorItem { #[error("Compilation error: {0}")] @@ -239,8 +224,6 @@ pub enum ErrorItem { IO(#[from] io::Error), #[error("Invalid machine error: {0}")] InvalidMachine(#[from] InvalidMachineError), - #[error("Invalid filesystem error: {0}")] - InvalidFileSystem(#[from] InvalidFileSystemError), } impl ErrorItem { @@ -370,12 +353,6 @@ impl From for CascadeErrors { } } -impl From for CascadeErrors { - fn from(error: InvalidFileSystemError) -> Self { - CascadeErrors::from(ErrorItem::from(error)) - } -} - impl Iterator for CascadeErrors { type Item = ErrorItem; fn next(&mut self) -> Option { @@ -415,7 +392,8 @@ mod tests { "This is the word 'of' in file 1", ); - error = error.add_additional_message(&file2, 12..16, "This is the word file in file 2"); + error = + error.add_additional_message(&file2, 12..16, "This is the word file in file 2"); let labels = error.diagnostic.inner.labels; assert_eq!(labels.len(), 2); diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 85dd67e0..55627c0d 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -18,9 +18,12 @@ use crate::ast::{ }; use crate::constants; use crate::context::{BlockType, Context as BlockContext}; -use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError, InvalidFileSystemError}; +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"; @@ -1591,14 +1594,24 @@ impl FromStr for FSContextType { } } -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Derivative)] +#[derivative(Clone, Debug, PartialEq, PartialOrd, Ord)] pub struct FileSystemContextRule<'a> { pub fscontext_type: FSContextType, pub fs_name: String, 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, } +impl Eq for FileSystemContextRule<'_> {} impl FileSystemContextRule<'_> { fn get_renamed_statement(&self, renames: &BTreeMap) -> Self { @@ -1608,6 +1621,8 @@ impl FileSystemContextRule<'_> { path: self.path.clone(), file_type: self.file_type, context: self.context.get_renamed_context(renames), + file: self.file.clone(), + func_call: self.func_call.clone(), } } } @@ -1650,13 +1665,15 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { Sexp::from(&f.context), ])) } else { - Err(ErrorItem::InvalidFileSystem(InvalidFileSystemError::new( - &format!( - "Genfscon missing path.\n No path given for genfscon rule:\ - \n\tFilesystem name: {}\n\tContext: {}", - f.fs_name, f.context, - ), - ))) + // We should never get here since we are defaulting to "/" + // when we call this normally but if someone calls this in + // an unexpected way we will get this call. + Err(ErrorItem::make_compile_or_internal_error( + "Genfscon missing path", + Some(&f.file), + f.func_call.get_name_range(), + "Path must be given for genfscon", + )) } } } @@ -1784,6 +1801,8 @@ fn call_to_fsc_rules<'a>( path: None, file_type: None, context: fs_context.clone(), + file: file.clone(), + func_call: c.clone(), }); } let mut errors = CascadeErrors::new(); @@ -1815,6 +1834,8 @@ fn call_to_fsc_rules<'a>( path: Some(regex_string), file_type: None, context: fs_context.clone(), + file: file.clone(), + func_call: c.clone(), }); } else { for file_type in file_types { @@ -1836,6 +1857,8 @@ fn call_to_fsc_rules<'a>( path: Some(regex_string.clone()), file_type: Some(file_type), context: fs_context.clone(), + file: file.clone(), + func_call: c.clone(), }); } } @@ -3896,8 +3919,8 @@ mod tests { assert!(!sexp.to_string().contains("old_name")); } Err(_) => { - // We should never get here in testing but if we do assert false - assert!(false); + // We should never get here in testing, but if we do panic + panic!(); } } } diff --git a/src/lib.rs b/src/lib.rs index ff998f2e..b1a328ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -410,7 +410,7 @@ mod tests { } let file_out_path = &[filename, "_test.cil"].concat(); let cil_out_path = &[filename, "_test_out_policy"].concat(); - let mut out_file = fs::File::create(&file_out_path).unwrap(); + let mut out_file = fs::File::create(file_out_path).unwrap(); out_file.write_all(policy_contents.as_bytes()).unwrap(); let output = Command::new("secilc") .arg(["--output=", cil_out_path].concat()) @@ -1232,7 +1232,7 @@ mod tests { #[test] fn invalid_fs_context_dup() { - error_policy_test!("fs_context_dup.cas", 3, ErrorItem::InvalidFileSystem(_)); + error_policy_test!("fs_context_dup.cas", 3, ErrorItem::Compile(_)); } #[test] From 39c31ec1c1e5333a1d60ca3fc9f7bdd18836d699 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Thu, 15 Dec 2022 08:37:58 -0700 Subject: [PATCH 11/19] fmt and fixing a rebase issue --- src/compile.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index 86059388..e7eed99e 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -17,8 +17,9 @@ use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; use crate::internal_rep::{ argument_to_typeinfo, argument_to_typeinfo_vec, generate_sid_rules, type_slice_to_variant, validate_derive_args, Annotated, AnnotationInfo, ArgForValidation, Associated, BoundTypeInfo, - ClassList, Context, FileSystemContextRule, FSContextType, FunctionArgument, FunctionInfo, FunctionMap, MachineMap, ModuleMap, Sid, - TypeInfo, TypeMap, ValidatedCall, ValidatedMachine, ValidatedModule, ValidatedStatement, + ClassList, Context, FSContextType, FileSystemContextRule, FunctionArgument, FunctionInfo, + FunctionMap, MachineMap, ModuleMap, Sid, TypeInfo, TypeMap, ValidatedCall, ValidatedMachine, + ValidatedModule, ValidatedStatement, }; use codespan_reporting::files::SimpleFile; From 4a0fa00d1b8444d77a3f6cdb6f95addf217c9972 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 28 Dec 2022 05:42:04 -0700 Subject: [PATCH 12/19] minor review fixes wip --- doc/TE.md | 15 +++++++++------ src/internal_rep.rs | 7 +------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/doc/TE.md b/doc/TE.md index ac3e8e34..f5f49aa1 100644 --- a/doc/TE.md +++ b/doc/TE.md @@ -436,12 +436,15 @@ Example using additional fields: domain foo { ... } ``` -The following arguments may be specified: source: The scontext field of a denial -target: The tcontext field of a denial class: The tclass field of a denial perm: -Any permission listed in a denial hint: A text string to display when using -newaudit2allow attack: Set to True to indicate that this denial should be -treated as a possible security incident cve: Reference a CVE associated with -this denial +The following arguments may be specified: +* source: The scontext field of a denial +* target: The tcontext field of a denial +* class: The tclass field of a denial +* perm: Any permission listed in a denial +* hint: A text string to display when using newaudit2allow +* attack: Set to True to indicate that this denial should be treated as a +possible security incident +* cve: Reference a CVE associated with this denial ### ignore annotation diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 55627c0d..acbe9c0d 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1668,12 +1668,7 @@ impl TryFrom<&FileSystemContextRule<'_>> for sexp::Sexp { // We should never get here since we are defaulting to "/" // when we call this normally but if someone calls this in // an unexpected way we will get this call. - Err(ErrorItem::make_compile_or_internal_error( - "Genfscon missing path", - Some(&f.file), - f.func_call.get_name_range(), - "Path must be given for genfscon", - )) + Err(ErrorItem::Internal(InternalError::new())) } } } From 02ca191eac39a6acccf76a231ba39d8526b180b3 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 4 Jan 2023 11:05:33 -0700 Subject: [PATCH 13/19] minor cleanup, want to discuss --- src/compile.rs | 38 ++++++++++++++++++++------------------ src/error.rs | 14 ++++++++++++++ 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index e7eed99e..b16411a0 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -13,7 +13,9 @@ use crate::ast::{ }; use crate::constants; use crate::context::{BlockType, Context as BlockContext}; -use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; +use crate::error::{ + add_or_create_compile_error, CascadeErrors, CompileError, ErrorItem, InternalError, +}; use crate::internal_rep::{ argument_to_typeinfo, argument_to_typeinfo_vec, generate_sid_rules, type_slice_to_variant, validate_derive_args, Annotated, AnnotationInfo, ArgForValidation, Associated, BoundTypeInfo, @@ -367,31 +369,27 @@ pub fn validate_fs_context_duplicates( } } } - // error is not None so we have already found something - if let Some(unwrapped_error) = error { - error = Some(unwrapped_error.add_additional_message(&rule.file, - range, - &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); - } else { - // error is none so we need to make a new one. - error = Some(CompileError::new( - "Duplicate filesystem context.", - &rule.file, - range, - &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); - } + error = Some(add_or_create_compile_error(error, + "Duplicate filesystem context.", + &rule.file, + range, + &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); } FSContextType::GenFSCon => { // genfscon gets more complicated. We can have similar rules as long as the paths are different. // 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 { + // 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 + // 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, inner_range.unwrap_or_default(), @@ -399,7 +397,7 @@ pub fn validate_fs_context_duplicates( } 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 is none so we need to make a new one error = Some(CompileError::new( "Duplicate genfscon contexts", &rule.file, @@ -410,15 +408,17 @@ pub fn validate_fs_context_duplicates( inner_range.unwrap_or_default(), &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. } 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 to else if above + // unwrap is safe here due to else if above if *cas_arg == inner_rule.file_type.unwrap().to_string() { @@ -427,12 +427,14 @@ pub fn validate_fs_context_duplicates( } } } - // error is not None so we have already found something + // 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, inner_range.unwrap_or_default(), &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 { diff --git a/src/error.rs b/src/error.rs index 6dbaf1a4..f1483442 100644 --- a/src/error.rs +++ b/src/error.rs @@ -90,6 +90,20 @@ impl CompileError { } } +pub fn add_or_create_compile_error( + error: Option, + msg: &str, + file: &SimpleFile, + range: Option>, + help: &str, +) -> CompileError { + if let Some(unwrapped_error) = error { + unwrapped_error.add_additional_message(file, range, help) + } else { + CompileError::new(msg, file, range, help) + } +} + #[derive(Error, Clone, Debug)] #[error("{backtrace:?}")] pub struct InternalError { From 32d5f58c99ae3c6d1f9fe9c2fb636230385d2e28 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Sat, 7 Jan 2023 18:41:13 -0700 Subject: [PATCH 14/19] rebase fixes, do not review --- src/compile.rs | 20 +++++++---- src/error.rs | 5 ++- src/internal_rep.rs | 84 ++++++++++++++++++++++++++------------------- 3 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index b16411a0..0ea0fd8e 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -361,7 +361,7 @@ pub fn validate_fs_context_duplicates( 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 = 0..0; + 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 { @@ -392,7 +392,8 @@ pub fn validate_fs_context_duplicates( // need to add a new error message if let Some(unwrapped_error) = error { error = Some(unwrapped_error.add_additional_message(&inner_rule.file, - inner_range.unwrap_or_default(), + // 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()), &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context))); } else { let outer_range: Option> = @@ -401,11 +402,13 @@ pub fn validate_fs_context_duplicates( error = Some(CompileError::new( "Duplicate genfscon contexts", &rule.file, - outer_range.unwrap_or_default(), + // 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, - inner_range.unwrap_or_default(), + // 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()), &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. @@ -431,7 +434,8 @@ pub fn validate_fs_context_duplicates( // need to add a new error message if let Some(unwrapped_error) = error { error = Some(unwrapped_error.add_additional_message(&inner_rule.file, - inner_range.unwrap_or_default(), + // 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()), &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 @@ -452,11 +456,13 @@ pub fn validate_fs_context_duplicates( error = Some(CompileError::new( "Duplicate genfscon file types", &inner_rule.file, - outer_range.unwrap_or_default(), + // 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, - inner_range.unwrap_or_default(), + // 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()), &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); } } diff --git a/src/error.rs b/src/error.rs index f1483442..dc35f15a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -94,7 +94,7 @@ pub fn add_or_create_compile_error( error: Option, msg: &str, file: &SimpleFile, - range: Option>, + range: Range, help: &str, ) -> CompileError { if let Some(unwrapped_error) = error { @@ -406,8 +406,7 @@ mod tests { "This is the word 'of' in file 1", ); - error = - error.add_additional_message(&file2, 12..16, "This is the word file in file 2"); + error = error.add_additional_message(&file2, 12..16, "This is the word file in file 2"); let labels = error.diagnostic.inner.labels; assert_eq!(labels.len(), 2); diff --git a/src/internal_rep.rs b/src/internal_rep.rs index acbe9c0d..5d7bdc39 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1746,12 +1746,14 @@ fn call_to_fsc_rules<'a>( let fs_context = match Context::try_from(context_str.to_string()) { Ok(c) => c, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "Invalid context", - Some(file), - context_str.get_range(), - "Cannot parse this into a context", - ))) + return Err(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "Invalid context", + Some(file), + context_str.get_range(), + "Cannot parse this into a context", + ), + )) } }; let fs_name = args_iter @@ -1766,12 +1768,14 @@ fn call_to_fsc_rules<'a>( let fscontext_type = match fscontext_str.to_string().parse::() { Ok(f) => f, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "Not a valid file system type", - Some(file), - fscontext_str.get_range(), - "File system type must be 'xattr', 'task', 'trans', or 'genfscon'", - ))); + return Err(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "Not a valid file system type", + Some(file), + fscontext_str.get_range(), + "File system type must be 'xattr', 'task', 'trans', or 'genfscon'", + ), + )); } }; let regex_string_arg = args_iter @@ -1802,20 +1806,24 @@ fn call_to_fsc_rules<'a>( } let mut errors = CascadeErrors::new(); if !file_types.is_empty() { - errors.append(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "File types can only be provided for 'genfscon'", - Some(file), - file_types_arg.get_range(), - "", - ))); + errors.append(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "File types can only be provided for 'genfscon'", + Some(file), + file_types_arg.get_range(), + "", + ), + )); } if regex_string_arg.get_range().is_some() { - errors.append(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "File path can only be provided for 'genfscon'", - Some(file), - regex_string_arg.get_range(), - "", - ))); + errors.append(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "File path can only be provided for 'genfscon'", + Some(file), + regex_string_arg.get_range(), + "", + ), + )); } if !errors.is_empty() { return Err(errors); @@ -1837,12 +1845,14 @@ fn call_to_fsc_rules<'a>( let file_type = match file_type.to_string().parse::() { Ok(f) => f, Err(_) => { - return Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "Not a valid file type", - Some(file), - file_type.get_range(), - "", - ))) + return Err(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "Not a valid file type", + Some(file), + file_type.get_range(), + "", + ), + )) } }; @@ -2700,12 +2710,14 @@ impl<'a> ValidatedStatement<'a> { .map(ValidatedStatement::FscRule) .collect()) } else { - Err(CascadeErrors::from(ErrorItem::make_compile_or_internal_error( - "fs_context() calls are only allowed in resources", - Some(file), - c.name.get_range(), - "Not allowed here", - ))) + Err(CascadeErrors::from( + ErrorItem::make_compile_or_internal_error( + "fs_context() calls are only allowed in resources", + Some(file), + c.name.get_range(), + "Not allowed here", + ), + )) } } Some(BuiltIns::DomainTransition) => { From dc92074240d3572236552094a29ada22b32ed2dc Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 11 Jan 2023 13:53:14 -0700 Subject: [PATCH 15/19] 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 0ea0fd8e..d27ef350 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, @@ -347,32 +346,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 => { @@ -380,89 +372,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))); } } @@ -490,11 +441,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 5d7bdc39..d66e704b 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(), }); } } From bda4fc95720166d17fb94c7790889ea3f6eb3876 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Wed, 11 Jan 2023 17:33:29 -0700 Subject: [PATCH 16/19] rebase fixes --- data/expected_cil/fs_context.cil | 19 ++++++++++++++++++- src/ast.rs | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/data/expected_cil/fs_context.cil b/data/expected_cil/fs_context.cil index 0d5956a0..ec6d24c8 100644 --- a/data/expected_cil/fs_context.cil +++ b/data/expected_cil/fs_context.cil @@ -95,8 +95,25 @@ (class unix_stream_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind connectto)) (class vsock_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) (class x25_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) +(class x_application_data (paste paste_after_confirm copy)) +(class x_client (destroy getattr setattr manage)) +(class x_colormap (create destroy read write getattr add_color remove_color install uninstall use)) +(class x_cursor (create destroy read write getattr setattr use)) +(class x_drawable (create destroy read write blend getattr setattr list_child add_child remove_child list_property get_property set_property manage override show hide send receive)) +(class x_event (send receive)) +(class x_extension (query use)) +(class x_font (create destroy getattr add_glyph remove_glyph use)) +(class x_gc (create destroy getattr setattr use)) +(class x_keyboard (getattr setattr use read write getfocus setfocus bell force_cursor freeze grab manage list_property get_property set_property add remove create destroy)) +(class x_pointer (getattr setattr use read write getfocus setfocus bell force_cursor freeze grab manage list_property get_property set_property add remove create destroy)) +(class x_property (create destroy read write append getattr setattr)) +(class x_resource (read write)) +(class x_screen (getattr setattr hide_cursor show_cursor saver_getattr saver_setattr saver_hide saver_show)) +(class x_selection (read write getattr setattr)) +(class x_server (getattr setattr record debug grab manage)) +(class x_synthetic_event (send receive)) (class xdp_socket (ioctl read write create getattr setattr lock relabelfrom relabelto append map bind connect listen accept getopt setopt shutdown recvfrom sendto name_bind)) -(classorder (alg_socket anon_inode appletalk_socket association atmpvc_socket atmsvc_socket ax25_socket binder blk_file bluetooth_socket bpf caif_socket can_socket cap2_userns cap_userns capability capability2 chr_file dbus dccp_socket decnet_socket dir fd fifo_file file filesystem icmp_socket ieee802154_socket infiniband_endpoint infiniband_pkey ipc ipx_socket irda_socket isdn_socket iucv_socket kcm_socket kernel_service key key_socket llc_socket lnk_file lockdown memprotect msg msgq netif netlink_audit_socket netlink_connector_socket netlink_crypto_socket netlink_dnrt_socket netlink_fib_lookup_socket netlink_generic_socket netlink_iscsi_socket netlink_kobject_uevent_socket netlink_netfilter_socket netlink_nflog_socket netlink_rdma_socket netlink_route_socket netlink_scsitransport_socket netlink_selinux_socket netlink_socket netlink_tcpdiag_socket netlink_xfrm_socket netrom_socket nfc_socket node node_socket packet packet_socket peer perf_event phonet_socket pppox_socket process process2 qipcrtr_socket rawip_socket rds_socket rose_socket rxrpc_socket sctp_socket security sem service shm smc_socket sock_file socket system tcp_socket tipc_socket tun_socket udp_socket unix_dgram_socket unix_stream_socket vsock_socket x25_socket xdp_socket)) +(classorder (alg_socket anon_inode appletalk_socket association atmpvc_socket atmsvc_socket ax25_socket binder blk_file bluetooth_socket bpf caif_socket can_socket cap2_userns cap_userns capability capability2 chr_file dbus dccp_socket decnet_socket dir fd fifo_file file filesystem icmp_socket ieee802154_socket infiniband_endpoint infiniband_pkey ipc ipx_socket irda_socket isdn_socket iucv_socket kcm_socket kernel_service key key_socket llc_socket lnk_file lockdown memprotect msg msgq netif netlink_audit_socket netlink_connector_socket netlink_crypto_socket netlink_dnrt_socket netlink_fib_lookup_socket netlink_generic_socket netlink_iscsi_socket netlink_kobject_uevent_socket netlink_netfilter_socket netlink_nflog_socket netlink_rdma_socket netlink_route_socket netlink_scsitransport_socket netlink_selinux_socket netlink_socket netlink_tcpdiag_socket netlink_xfrm_socket netrom_socket nfc_socket node node_socket packet packet_socket peer perf_event phonet_socket pppox_socket process process2 qipcrtr_socket rawip_socket rds_socket rose_socket rxrpc_socket sctp_socket security sem service shm smc_socket sock_file socket system tcp_socket tipc_socket tun_socket udp_socket unix_dgram_socket unix_stream_socket vsock_socket x25_socket x_application_data x_client x_colormap x_cursor x_drawable x_event x_extension x_font x_gc x_keyboard x_pointer x_property x_resource x_screen x_selection x_server x_synthetic_event xdp_socket)) (sensitivity s0) (sensitivityorder (s0)) (user system_u) diff --git a/src/ast.rs b/src/ast.rs index efdf431c..d91e0a9f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -434,6 +434,7 @@ impl FuncCall { return Some(BuiltIns::FileSystemContext); } if self.name == constants::DOMTRANS_FUNCTION_NAME { + return Some(BuiltIns::DomainTransition); } None } From d8a5897a0a22265605c67baa40e2a4c3b3e8efb1 Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Fri, 13 Jan 2023 14:20:26 -0700 Subject: [PATCH 17/19] fixing unwrap_or statements --- src/compile.rs | 4 ++-- src/internal_rep.rs | 34 ++++++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index d27ef350..d9a84852 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -22,6 +22,7 @@ use crate::internal_rep::{ FunctionMap, MachineMap, ModuleMap, Sid, TypeInfo, TypeMap, ValidatedCall, ValidatedMachine, ValidatedModule, ValidatedStatement, }; +use crate::unwrap_or_return; use codespan_reporting::files::SimpleFile; @@ -363,8 +364,7 @@ pub fn validate_fs_context_duplicates( error = Some(add_or_create_compile_error(error, "Duplicate filesystem context.", &rule.file, - // TODO once Issue #92 is resolved we would rather do a unwrap_or make internal error - rule.fs_name.get_range().unwrap_or_default(), + unwrap_or_return!(rule.fs_name.get_range(), Err(CascadeErrors::from(InternalError::new()))), &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); } FSContextType::GenFSCon => { diff --git a/src/internal_rep.rs b/src/internal_rep.rs index d66e704b..55a68795 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -21,6 +21,16 @@ use crate::context::{BlockType, Context as BlockContext}; use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; use crate::obj_class::perm_list_to_sexp; +#[macro_export] +macro_rules! unwrap_or_return { + ($e:expr, $r:expr) => { + match $e { + Some(e) => e, + None => return $r, + } + }; +} + const DEFAULT_USER: &str = "system_u"; const DEFAULT_OBJECT_ROLE: &str = "object_r"; const DEFAULT_DOMAIN_ROLE: &str = "system_r"; @@ -1784,8 +1794,6 @@ 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()))? @@ -1832,10 +1840,12 @@ fn call_to_fsc_rules<'a>( context: fs_context.clone(), file: file.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 + // have to deal with Option 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(), + context_range: unwrap_or_return!( + context_str_arg.get_range(), + Err(CascadeErrors::from(InternalError::new())) + ), }); } let mut errors = CascadeErrors::new(); @@ -1873,10 +1883,12 @@ fn call_to_fsc_rules<'a>( context: fs_context.clone(), file: file.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 + // have to deal with Option 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(), + context_range: unwrap_or_return!( + context_str_arg.get_range(), + Err(CascadeErrors::from(InternalError::new())) + ), }); } else { for file_type in file_types { @@ -1904,8 +1916,10 @@ fn call_to_fsc_rules<'a>( 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(), + context_range: unwrap_or_return!( + context_str_arg.get_range(), + Err(CascadeErrors::from(InternalError::new())) + ), }); } } From e7e54457b6fbaa355f9577ee1d98b165c1cbfedc Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Mon, 16 Jan 2023 00:16:09 -0700 Subject: [PATCH 18/19] refactoring error handling --- src/compile.rs | 82 +++++++++++++++++++++++++-------------------- src/internal_rep.rs | 31 +++++------------ 2 files changed, 55 insertions(+), 58 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index d9a84852..782df6fe 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -5,6 +5,7 @@ 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, @@ -22,7 +23,6 @@ use crate::internal_rep::{ FunctionMap, MachineMap, ModuleMap, Sid, TypeInfo, TypeMap, ValidatedCall, ValidatedMachine, ValidatedModule, ValidatedStatement, }; -use crate::unwrap_or_return; use codespan_reporting::files::SimpleFile; @@ -346,6 +346,34 @@ pub fn build_func_map<'a>( Ok(decl_map) } +// Helper function to deal with the case where we need to either create a +// new error or add to an existing one, but specifically for this issue +// we need to create a new error and immediately add to it. +#[allow(clippy::too_many_arguments)] +fn new_error_helper( + error: Option, + msg: &str, + file_a: &SimpleFile, + file_b: &SimpleFile, + range_a: Range, + range_b: Range, + help_a: &str, + help_b: &str, +) -> CompileError { + let mut tmp_error; + // 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 { + tmp_error = unwrapped_error.add_additional_message(file_a, range_a, help_a); + } else { + // error is none so we need to make a new one + tmp_error = CompileError::new(msg, file_b, range_b, help_b); + tmp_error = tmp_error.add_additional_message(file_a, range_a, help_a); + } + + tmp_error +} + pub fn validate_fs_context_duplicates( fsc_rules: BTreeMap>, ) -> Result<(), CascadeErrors> { @@ -364,7 +392,7 @@ pub fn validate_fs_context_duplicates( error = Some(add_or_create_compile_error(error, "Duplicate filesystem context.", &rule.file, - unwrap_or_return!(rule.fs_name.get_range(), Err(CascadeErrors::from(InternalError::new()))), + rule.fs_name.get_range().ok_or_else(||CascadeErrors::from(InternalError::new()))?, &format!("Found multiple different filesystem type declarations for filesystem: {}", rule.fs_name))); } FSContextType::GenFSCon => { @@ -377,45 +405,27 @@ pub fn validate_fs_context_duplicates( 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 { - // 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, - inner_rule.context_range.clone(), - &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context))); - } else { - // error is none so we need to make a new one - let tmp_error = CompileError::new( - "Duplicate genfscon contexts", - &rule.file, - 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))); - } + error = Some(new_error_helper(error, + "Duplicate genfscon contexts", + &inner_rule.file, + &rule.file, + inner_rule.context_range.clone(), + rule.context_range.clone(), + &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", inner_rule.fs_name, inner_rule.context), + &format!("Found duplicate genfscon rules for filesystem {} with differing contexts: {}", rule.fs_name, rule.context))); // 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() { - // 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, - inner_rule.file_type_range.clone(), - &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name))); - } else { - // error is none so we need to make a new one. - let tmp_error = CompileError::new( - "Duplicate genfscon file types", - &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))); - } + error = Some(new_error_helper(error, + "Duplicate genfscon file types", + &inner_rule.file, + &rule.file, + inner_rule.file_type_range.clone(), + rule.file_type_range.clone(), + &format!("Found duplicate genfscon rules for filesystem {} with differing file types", inner_rule.fs_name), + &format!("Found duplicate genfscon rules for filesystem {} with differing file types", rule.fs_name))); } } } diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 55a68795..32108da4 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -21,16 +21,6 @@ use crate::context::{BlockType, Context as BlockContext}; use crate::error::{CascadeErrors, CompileError, ErrorItem, InternalError}; use crate::obj_class::perm_list_to_sexp; -#[macro_export] -macro_rules! unwrap_or_return { - ($e:expr, $r:expr) => { - match $e { - Some(e) => e, - None => return $r, - } - }; -} - const DEFAULT_USER: &str = "system_u"; const DEFAULT_OBJECT_ROLE: &str = "object_r"; const DEFAULT_DOMAIN_ROLE: &str = "system_r"; @@ -1842,10 +1832,9 @@ fn call_to_fsc_rules<'a>( // file_type_range shouldn't ever be used for xattr, task, or trans but I would rather not // have to deal with Option stuff later file_type_range: c.get_name_range().unwrap_or_default(), - context_range: unwrap_or_return!( - context_str_arg.get_range(), - Err(CascadeErrors::from(InternalError::new())) - ), + context_range: context_str_arg + .get_range() + .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, }); } let mut errors = CascadeErrors::new(); @@ -1885,10 +1874,9 @@ fn call_to_fsc_rules<'a>( // 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 stuff later file_type_range: c.get_name_range().unwrap_or_default(), - context_range: unwrap_or_return!( - context_str_arg.get_range(), - Err(CascadeErrors::from(InternalError::new())) - ), + context_range: context_str_arg + .get_range() + .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, }); } else { for file_type in file_types { @@ -1916,10 +1904,9 @@ fn call_to_fsc_rules<'a>( file_type_range: file_types_arg .get_range() .unwrap_or_else(|| c.get_name_range().unwrap_or_default()), - context_range: unwrap_or_return!( - context_str_arg.get_range(), - Err(CascadeErrors::from(InternalError::new())) - ), + context_range: context_str_arg + .get_range() + .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, }); } } From 0635079a4e5e6e1dd250c16ae472427d61af67ab Mon Sep 17 00:00:00 2001 From: Matt Sheets Date: Mon, 16 Jan 2023 19:53:25 -0700 Subject: [PATCH 19/19] review comments --- src/compile.rs | 10 +++++----- src/internal_rep.rs | 30 ++++++++++++------------------ 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/compile.rs b/src/compile.rs index 782df6fe..9830f54c 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -360,18 +360,18 @@ fn new_error_helper( help_a: &str, help_b: &str, ) -> CompileError { - let mut tmp_error; + let mut ret; // 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 { - tmp_error = unwrapped_error.add_additional_message(file_a, range_a, help_a); + ret = unwrapped_error.add_additional_message(file_a, range_a, help_a); } else { // error is none so we need to make a new one - tmp_error = CompileError::new(msg, file_b, range_b, help_b); - tmp_error = tmp_error.add_additional_message(file_a, range_a, help_a); + ret = CompileError::new(msg, file_b, range_b, help_b); + ret = ret.add_additional_message(file_a, range_a, help_a); } - tmp_error + ret } pub fn validate_fs_context_duplicates( diff --git a/src/internal_rep.rs b/src/internal_rep.rs index 32108da4..1f5ce20c 100644 --- a/src/internal_rep.rs +++ b/src/internal_rep.rs @@ -1597,10 +1597,11 @@ pub struct FileSystemContextRule<'a> { pub fs_name: CascadeString, pub path: Option, pub file_type: Option, + //Note: if a file type is not given this will be the range of the function name + pub file_type_range: Range, pub context: Context<'a>, - pub file: SimpleFile, - 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, + pub file: SimpleFile, } impl Eq for FileSystemContextRule<'_> {} @@ -1827,14 +1828,14 @@ fn call_to_fsc_rules<'a>( fs_name, path: None, file_type: None, - context: fs_context.clone(), - file: file.clone(), // file_type_range shouldn't ever be used for xattr, task, or trans but I would rather not // have to deal with Option stuff later file_type_range: c.get_name_range().unwrap_or_default(), + context: fs_context.clone(), context_range: context_str_arg .get_range() .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, + file: file.clone(), }); } let mut errors = CascadeErrors::new(); @@ -1869,14 +1870,14 @@ fn call_to_fsc_rules<'a>( fs_name, path: Some(regex_string), file_type: None, - context: fs_context.clone(), - file: file.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 stuff later file_type_range: c.get_name_range().unwrap_or_default(), + context: fs_context.clone(), context_range: context_str_arg .get_range() .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, + file: file.clone(), }); } else { for file_type in file_types { @@ -1899,14 +1900,14 @@ fn call_to_fsc_rules<'a>( fs_name: fs_name.clone(), path: Some(regex_string.clone()), file_type: Some(file_type), - context: fs_context.clone(), - file: file.clone(), file_type_range: file_types_arg .get_range() .unwrap_or_else(|| c.get_name_range().unwrap_or_default()), + context: fs_context.clone(), context_range: context_str_arg .get_range() .ok_or_else(|| CascadeErrors::from(InternalError::new()))?, + file: file.clone(), }); } } @@ -3963,16 +3964,9 @@ mod tests { let mut renames = BTreeMap::new(); renames.insert("old_name".to_string(), "new_name".to_string()); let renamed_statement = statement.get_renamed_statement(&renames); - match Sexp::try_from(&renamed_statement) { - Ok(sexp) => { - assert!(sexp.to_string().contains("new_name")); - assert!(!sexp.to_string().contains("old_name")); - } - Err(_) => { - // We should never get here in testing, but if we do panic - panic!(); - } - } + let sexp = Sexp::try_from(&renamed_statement).unwrap(); + assert!(sexp.to_string().contains("new_name")); + assert!(!sexp.to_string().contains("old_name")); } } }