Fixed not passing file open mode on bypass (Go+Linux) (#2865)

* Passing mode on bypass

* Added integration test

* Changelog

* Style fixed in test app

* Updated test doc

* Fix lints in test

* Use va_list in openat_detour

* Fixed param type

* pass mode as c_int to fix macos x64

changelog.d/2614.fixed.md

@@ -0,0 +1 @@
+Fixed a bug where file mode was ignored when Go applications were creating local files.

mirrord/layer/src/file/hooks.rs

@@ -303,18 +303,26 @@ pub(crate) unsafe extern "C" fn dirfd_detour(dirp: *mut DIR) -> c_int {
 /// If `fd == AT_FDCWD`, the current working directory is used, and the behavior is the same as
 /// `open_detour`.
 /// `fd` for a file descriptor with the `O_DIRECTORY` flag.
-#[hook_guard_fn]
+#[hook_fn]
 pub(crate) unsafe extern "C" fn openat_detour(
     fd: RawFd,
     raw_path: *const c_char,
     open_flags: c_int,
+    mut args: ...
 ) -> RawFd {
-    let open_options = OpenOptionsInternalExt::from_flags(open_flags);
+    let mode: c_int = args.arg();
 
-    openat(fd, raw_path.checked_into(), open_options).unwrap_or_bypass_with(|bypass| {
-        let raw_path = update_ptr_from_bypass(raw_path, &bypass);
-        FN_OPENAT(fd, raw_path, open_flags)
-    })
+    let guard = DetourGuard::new();
+    if guard.is_none() {
+        FN_OPENAT(fd, raw_path, open_flags, mode)
+    } else {
+        let open_options = OpenOptionsInternalExt::from_flags(open_flags);
+
+        openat(fd, raw_path.checked_into(), open_options).unwrap_or_bypass_with(|bypass| {
+            let raw_path = update_ptr_from_bypass(raw_path, &bypass);
+            FN_OPENAT(fd, raw_path, open_flags, mode)
+        })
+    }
 }
 
 /// Equivalent to `open_detour`, **except** when `raw_path` specifies a relative path.

mirrord/layer/src/go/mod.rs

@@ -103,7 +103,10 @@ unsafe extern "C" fn c_abi_syscall6_handler(
                 libc::SYS_fstat => fstat_detour(param1 as _, param2 as _) as i64,
                 libc::SYS_fsync => fsync_detour(param1 as _) as i64,
                 libc::SYS_fdatasync => fsync_detour(param1 as _) as i64,
-                libc::SYS_openat => openat_detour(param1 as _, param2 as _, param3 as _) as i64,
+                libc::SYS_openat => {
+                    openat_detour(param1 as _, param2 as _, param3 as _, param4 as libc::c_int)
+                        as i64
+                }
                 libc::SYS_getdents64 => {
                     getdents64_detour(param1 as _, param2 as _, param3 as _) as i64
                 }

mirrord/layer/tests/apps/open_go/go.mod

@@ -0,0 +1,3 @@
+module open_go
+
+go 1.23

mirrord/layer/tests/apps/open_go/main.go

@@ -0,0 +1,25 @@
+package main
+
+import (
+	"flag"
+	"io/fs"
+	_ "net" // to assert dynamic linking
+	"os"
+)
+
+func main() {
+	var path string
+	var flags int
+	var mode uint
+
+	flag.StringVar(&path, "p", "/tmp/testfile", "Path to the file")
+	flag.IntVar(&flags, "f", os.O_RDONLY, "Flags to use when opening the file (as raw integer)")
+	flag.UintVar(&mode, "m", 0444, "Mode to use when opening the file (as raw integer)")
+	flag.Parse()
+
+	fd, err := os.OpenFile(path, flags, fs.FileMode(mode))
+	if err != nil {
+		panic(err)
+	}
+	fd.Close()
+}

mirrord/layer/tests/common/mod.rs

@@ -752,6 +752,15 @@ pub enum Application {
     RustIssue2438,
     NodeIssue2807,
     RustRebind0,
+    /// Go application that simply opens a file.
+    Go23Open {
+        /// Path to the file, accepted as `-p` param.
+        path: String,
+        /// Flags to use when opening the file, accepted as `-f` param.
+        flags: i32,
+        /// Mode to use when opening the file, accepted as `-m` param.
+        mode: u32,
+    },
     // For running applications with the executable and arguments determined at runtime.
     DynamicApp(String, Vec<String>),
 }
@@ -923,6 +932,7 @@ impl Application {
             ),
             Application::RustIssue2058 => String::from("tests/apps/issue2058/target/issue2058"),
             Application::RustIssue2204 => String::from("tests/apps/issue2204/target/issue2204"),
+            Application::Go23Open { .. } => String::from("tests/apps/open_go/23.go_test_app"),
             Application::DynamicApp(exe, _) => exe.clone(),
         }
     }
@@ -1051,6 +1061,16 @@ impl Application {
                 .into_iter()
                 .map(Into::into)
                 .collect(),
+            Application::Go23Open { path, flags, mode } => {
+                vec![
+                    "-p".to_string(),
+                    path.clone(),
+                    "-f".to_string(),
+                    flags.to_string(),
+                    "-m".to_string(),
+                    mode.to_string(),
+                ]
+            }
             Application::DynamicApp(_, args) => args.to_owned(),
         }
     }
@@ -1123,6 +1143,7 @@ impl Application {
             | Application::RustIssue2438
             | Application::NodeIssue2807
             | Application::RustRebind0
+            | Application::Go23Open { .. }
             | Application::DynamicApp(..) => unimplemented!("shouldn't get here"),
             Application::PythonSelfConnect => 1337,
             Application::RustIssue2058 => 1234,

mirrord/layer/tests/issue2614.rs

@@ -0,0 +1,46 @@
+#![cfg(target_os = "linux")]
+#![feature(assert_matches)]
+#![warn(clippy::indexing_slicing)]
+
+use std::{os::unix::fs::PermissionsExt, path::Path, time::Duration};
+
+use rand::{thread_rng, Rng};
+use rstest::rstest;
+
+mod common;
+
+pub use common::*;
+
+/// Verify that issue [#2614](https://github.com/metalbear-co/mirrord/issues/2614) is fixed
+/// and the file open mode is honoured on bypass.
+#[rstest]
+#[tokio::test]
+#[timeout(Duration::from_secs(60))]
+async fn test_issue2614(dylib_path: &Path) {
+    let tmpdir = tempfile::tempdir().unwrap();
+    let file_path = tmpdir
+        .path()
+        .join(format!("testfile-{}", thread_rng().gen::<u64>(),));
+    let application = Application::Go23Open {
+        path: file_path.to_str().unwrap().into(),
+        flags: libc::O_CREAT | libc::O_RDWR,
+        mode: libc::S_IRUSR | libc::S_IRGRP | libc::S_IROTH,
+    };
+    let (mut test_process, mut intproxy) = application
+        .start_process_with_layer(dylib_path, vec![("MIRRORD_FILE_MODE", "local")], None)
+        .await;
+
+    let message = intproxy.try_recv().await;
+    assert!(
+        message.is_none(),
+        "received an unexpected message: {message:?}"
+    );
+    test_process.wait_assert_success().await;
+
+    let permissions = tokio::fs::metadata(file_path).await.unwrap().permissions();
+    assert_eq!(
+        permissions.mode() & 0b111111111,
+        libc::S_IRUSR | libc::S_IRGRP | libc::S_IROTH,
+        "test app created file with unexpected permissions"
+    )
+}