Add lints for exe subjects (#854)

Additional lints for exe subjects.

- Check for directory used as exe
- Check for non-executable file used as exe

Closes #853

crates/rules/Cargo.toml

@@ -2,7 +2,7 @@
 name = "fapolicy-rules"
 description = "Rule support for fapolicyd"
 license = "MPL-2.0"
-version = "0.4.4"
+version = "0.4.5"
 edition = "2018"
 
 [lib]
@@ -17,3 +17,4 @@ nom = "7.1"
 serde = { version = "1.0", features = ["derive"] }
 thiserror = "1.0"
 log = "0.4"
+is_executable = "1"

crates/rules/src/linter/findings.rs

@@ -7,8 +7,8 @@
  */
 
 use crate::db::{Entry, DB};
-use crate::object::Part;
 use crate::Rule;
+use is_executable::IsExecutable;
 use std::path::PathBuf;
 
 pub(crate) const L001_MESSAGE: &str = "Using any+all+all here will short-circuit all other rules";
@@ -17,6 +17,8 @@ pub(crate) const L003_MESSAGE_A: &str = "object does not exist at";
 pub(crate) const L003_MESSAGE_B: &str = "The object should be a";
 pub(crate) const L004_MESSAGE: &str = "Duplicate of rule";
 pub(crate) const L005_MESSAGE: &str = "Directory should have trailing slash";
+pub(crate) const L006_MESSAGE: &str = "The subject exe is a directory";
+pub(crate) const L007_MESSAGE: &str = "The subject exe is not executable";
 
 pub fn l001(fk: usize, r: &Rule, db: &DB) -> Option<String> {
     let id = db.rule_rev(fk).map(|e| e.id).unwrap();
@@ -55,15 +57,29 @@ fn is_file(p: &str) -> bool {
     PathBuf::from(p).is_file()
 }
 
+fn is_executable(p: &str) -> bool {
+    PathBuf::from(p).is_executable()
+}
+
 fn path_does_not_exist_message(t: &str, p: &str) -> String {
     format!("{} {} {}", t, L003_MESSAGE_A, p)
 }
 
+fn exe_is_a_directory(p: &str) -> String {
+    format!("{} at {}", L006_MESSAGE, p)
+}
+
+fn exe_is_not_executable(p: &str) -> String {
+    format!("{} at {}", L007_MESSAGE, p)
+}
+
 fn wrong_type_message(t: &str) -> String {
     format!("{} {}", L003_MESSAGE_B, t)
 }
 
 pub fn l003_object_path_missing(_: usize, r: &Rule, _db: &DB) -> Option<String> {
+    use crate::object::Part;
+
     r.obj
         .parts
         .iter()
@@ -95,6 +111,8 @@ pub fn l004_duplicate_rule(fk: usize, r: &Rule, db: &DB) -> Option<String> {
 }
 
 pub fn l005_object_dir_missing_trailing_slash(_: usize, r: &Rule, _db: &DB) -> Option<String> {
+    use crate::object::Part;
+
     r.obj
         .parts
         .iter()
@@ -107,11 +125,30 @@ pub fn l005_object_dir_missing_trailing_slash(_: usize, r: &Rule, _db: &DB) -> O
         .cloned()
 }
 
+pub fn l006_l007_subject_exe(_: usize, r: &Rule, _db: &DB) -> Option<String> {
+    use crate::subject::Part;
+
+    r.subj
+        .parts
+        .iter()
+        .filter_map(|p| match p {
+            Part::Exe(p) if is_dir(p) => Some(exe_is_a_directory(p)),
+            Part::Exe(p) if !is_executable(p) => Some(exe_is_not_executable(p)),
+            _ => None,
+        })
+        .collect::<Vec<String>>()
+        .first()
+        .cloned()
+}
+
 #[cfg(test)]
 mod tests {
-    use crate::linter::findings::{path_does_not_exist_message, L004_MESSAGE};
+    use crate::linter::findings::{
+        exe_is_a_directory, exe_is_not_executable, path_does_not_exist_message, L004_MESSAGE,
+    };
     use crate::read::deserialize_rules_db;
     use std::error::Error;
+    use tempfile::NamedTempFile;
 
     #[test]
     fn lint_duplicates() -> Result<(), Box<dyn Error>> {
@@ -214,4 +251,33 @@ mod tests {
         );
         Ok(())
     }
+
+    #[test]
+    fn lint_exe_is_a_dir() -> Result<(), Box<dyn Error>> {
+        let db = deserialize_rules_db("allow perm=any exe=/ : all")?;
+        let r = db.rule(1).unwrap();
+
+        assert!(r.msg.is_some());
+        println!("{}", r.msg.as_ref().unwrap());
+        assert_eq!(r.msg.as_ref().unwrap(), &exe_is_a_directory("/"));
+        Ok(())
+    }
+
+    #[test]
+    fn lint_exe_is_not_executable() -> Result<(), Box<dyn Error>> {
+        let file = NamedTempFile::new()?;
+        let file_path_str = file.path().display().to_string();
+        let rule_txt = format!("allow perm=any exe={file_path_str} : all");
+        println!("{rule_txt}");
+        let db = deserialize_rules_db(&rule_txt)?;
+        let r = db.rule(1).unwrap();
+
+        assert!(r.msg.is_some());
+        println!("{}", r.msg.as_ref().unwrap());
+        assert_eq!(
+            r.msg.as_ref().unwrap(),
+            &exe_is_not_executable(&file_path_str)
+        );
+        Ok(())
+    }
 }

crates/rules/src/linter/lint.rs

@@ -19,6 +19,7 @@ pub fn lint_db(db: DB) -> DB {
         l003_object_path_missing,
         l004_duplicate_rule,
         l005_object_dir_missing_trailing_slash,
+        l006_l007_subject_exe,
     ];
 
     db.iter()

fapolicy-analyzer.spec

@@ -43,6 +43,7 @@ BuildRequires: rust-either-devel
 BuildRequires: rust-fastrand-devel
 BuildRequires: rust-getrandom-devel
 BuildRequires: rust-iana-time-zone-devel
+BuildRequires: rust-is_executable-devel
 BuildRequires: rust-instant-devel
 BuildRequires: rust-lazy_static-devel
 BuildRequires: rust-libc-devel