feat(jailer): check if executable has exec permissions

The executable passed to the jailer must have exec
permissions set.
Update the jailer executable check unit tests with `match!`.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/jailer/src/env.rs

@@ -289,7 +289,14 @@ impl Env {
         let exec_file_path = canonicalize(exec_file)
             .map_err(|err| JailerError::Canonicalize(PathBuf::from(exec_file), err))?;
 
-        if !exec_file_path.is_file() {
+        // Safety: the `canonicalize` call above ensured the existance of the path.
+        let metadata = std::fs::metadata(&exec_file_path).unwrap();
+
+        // Check that it is executable
+        if metadata.permissions().mode() & 0o111 == 0 {
+            return Err(JailerError::NotExecutable(exec_file_path));
+        }
+        if !metadata.is_file() {
             return Err(JailerError::NotAFile(exec_file_path));
         }
 
@@ -782,7 +789,8 @@ mod tests {
         pub fn new(pseudo_exec_file_path: &'a str) -> ArgVals<'a> {
             let pseudo_exec_file_dir = Path::new(&pseudo_exec_file_path).parent().unwrap();
             fs::create_dir_all(pseudo_exec_file_dir).unwrap();
-            File::create(pseudo_exec_file_path).unwrap();
+            create_exec_file(pseudo_exec_file_path);
+
             ArgVals {
                 id: "bd65600d-8669-4903-8a14-af88203add38",
                 exec_file: pseudo_exec_file_path,
@@ -799,6 +807,13 @@ mod tests {
         }
     }
 
+    fn create_exec_file(pseudo_exec_file_path: &str) {
+        let exec = File::create(pseudo_exec_file_path).unwrap();
+        let mut perms = exec.metadata().unwrap().permissions();
+        perms.set_mode(0o111);
+        exec.set_permissions(perms).unwrap();
+    }
+
     fn make_args(arg_vals: &ArgVals) -> Vec<String> {
         let mut arg_vec = vec![
             "--binary-name",
@@ -1018,31 +1033,30 @@ mod tests {
         let pseudo_exec_file_path = get_pseudo_exec_file_path();
         let pseudo_exec_file_dir = Path::new(&pseudo_exec_file_path).parent().unwrap();
         create_dir_all(pseudo_exec_file_dir).unwrap();
-        File::create(&pseudo_exec_file_path).unwrap();
+        create_exec_file(&pseudo_exec_file_path);
         Env::validate_exec_file(&pseudo_exec_file_path).unwrap();
 
         // Error case 1: No such file exists
         std::fs::remove_file(&pseudo_exec_file_path).unwrap();
-        assert_eq!(
-            format!(
-                "{}",
-                Env::validate_exec_file(&pseudo_exec_file_path).unwrap_err()
-            ),
-            format!(
-                "Failed to canonicalize path {}: No such file or directory (os error 2)",
-                pseudo_exec_file_path
-            )
-        );
+        assert!(matches!(
+            Env::validate_exec_file("/tmp/firecracker_test_dir/foobarbaz"),
+            Err(JailerError::Canonicalize(_, _))
+        ));
 
         // Error case 2: Not a file
         std::fs::create_dir_all("/tmp/firecracker_test_dir").unwrap();
-        assert_eq!(
-            format!(
-                "{}",
-                Env::validate_exec_file("/tmp/firecracker_test_dir").unwrap_err()
-            ),
-            "/tmp/firecracker_test_dir is not a file"
-        );
+        assert!(matches!(
+            Env::validate_exec_file("/tmp/firecracker_test_dir"),
+            Err(JailerError::NotAFile(_))
+        ));
+
+        // Error case 3: Not an executable
+        File::create("/tmp/firecracker_test_dir/foobarbaz").unwrap();
+        assert!(matches!(
+            Env::validate_exec_file("/tmp/firecracker_test_dir/foobarbaz"),
+            Err(JailerError::NotExecutable(_))
+        ));
+        std::fs::remove_file("/tmp/firecracker_test_dir/foobarbaz").unwrap();
 
         std::fs::remove_dir_all("/tmp/firecracker_test_dir").unwrap();
     }
@@ -1181,7 +1195,7 @@ mod tests {
         let exec_file_path = get_pseudo_exec_file_path();
         let exec_file_dir = Path::new(&exec_file_path).parent().unwrap();
         fs::create_dir_all(exec_file_dir).unwrap();
-        File::create(&exec_file_path).unwrap();
+        create_exec_file(&exec_file_path);
         let some_dir = TempDir::new().unwrap();
         let some_dir_path = some_dir.as_path().to_str().unwrap();
 

src/jailer/src/main.rs

@@ -99,6 +99,8 @@ pub enum JailerError {
     MountBind(io::Error),
     #[error("Failed to change the propagation type to slave: {0}")]
     MountPropagationSlave(io::Error),
+    #[error("{}", format!("{:?} is not executable", .0).replace('\"', ""))]
+    NotExecutable(PathBuf),
     #[error("{}", format!("{:?} is not a file", .0).replace('\"', ""))]
     NotAFile(PathBuf),
     #[error("{}", format!("{:?} is not a directory", .0).replace('\"', ""))]

tests/integration_tests/security/test_jail.py

@@ -106,7 +106,7 @@ def test_exec_file_not_exist(uvm_plain, tmp_path):
     ):
         test_microvm.spawn()
 
-    # Error case 3: Filename without "firecracker"
+    # Error case 3: Not an executable
     pseudo_exec_file_path = tmp_path / "foobarbaz"
     pseudo_exec_file_path.touch()
     fc_dir = Path("/srv/jailer") / pseudo_exec_file_path.name / test_microvm.id
@@ -115,8 +115,7 @@ def test_exec_file_not_exist(uvm_plain, tmp_path):
 
     with pytest.raises(
         Exception,
-        match=r"Jailer error: Invalid filename. The filename of `--exec-file` option"
-        r' must contain "firecracker": foobarbaz',
+        match=rf"Jailer error: {pseudo_exec_file_path} is not executable",
     ):
         test_microvm.spawn()