[Cry for help by gitoxide]: Why can this code not determine that the file it just created is indeed owned by it?

[Byron]

As part of my work on gitoxide to incorporate the additional git security protocols I am trying to implement an ownership check for windows to assure that gitoxide will not fully trust repositories that aren't owned by the user executing the process.

This is the idea:

        pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result<bool> {
            fn from_path(path: Cow<'_, Path>) -> std::io::Result<u32> {
                use std::os::unix::fs::MetadataExt;
                let meta = std::fs::symlink_metadata(path)?;
                Ok(meta.uid())
            }

            fn from_process() -> std::io::Result<u32> {
                // SAFETY: there is no documented possibility for failure
                #[allow(unsafe_code)]
                let uid = unsafe { libc::geteuid() };
                Ok(uid)
            }

            Ok(from_path(path)? == from_process()?)
        }

However, when trying to implement the same for windows very much similarly as git itself the following test does not succeed despite hours of trying.

let dir = tempfile::tempdir()?;
assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?);

I am reaching out in the hopes that someone can point me to the issue with the code causing the failure. It's terrible, but maybe there is a way to get it to work nonetheless.

Thanks for your consideration.

[riverar]

No more crying @Byron! Here's one way you can check if the current process effective token contains the SID matching the owner of the demo directory on disk.

Thanks for asking, as we identified a metadata bug along the way. (microsoft/win32metadata#884)

[package]
name = "app"
version = "0.0.0"
edition = "2021"
publish = false

[dependencies.windows]
version = "0.35"
features = [
    "alloc",
    "Win32_Foundation",
    "Win32_Security_Authorization",
    "Win32_Storage_FileSystem",
    "Win32_System_Memory",
    "Win32_System_Threading",
]

use windows::{
    core::{Error, PCSTR},
    Win32::{
        Foundation::{BOOL, ERROR_SUCCESS, HANDLE, PSID},
        Security::{
            Authorization::{GetNamedSecurityInfoA, SE_FILE_OBJECT},
            CheckTokenMembershipEx, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR,
        },
        System::Memory::LocalFree,
    },
};

fn main() -> windows::core::Result<()> {
    unsafe {
        let demo_path = std::path::Path::new("demo");
        if !demo_path.exists() {
            std::fs::create_dir(demo_path).unwrap();
        }

        let mut psid = PSID::default();
        let mut pdescriptor = PSECURITY_DESCRIPTOR::default();
        let result = GetNamedSecurityInfoA(
            PCSTR(b".\\demo\0".as_ptr()),
            SE_FILE_OBJECT,
            OWNER_SECURITY_INFORMATION,
            &mut psid,
            std::ptr::null_mut(),
            std::ptr::null_mut(),
            std::ptr::null_mut(),
            &mut pdescriptor,
        );

        // Workaround for https://github.com/microsoft/win32metadata/issues/884
        if result != ERROR_SUCCESS.0 {
            return Err(Error::from_win32());
        }

        let mut is_member = BOOL(0);
        CheckTokenMembershipEx(HANDLE::default(), psid, 0, &mut is_member).ok()?;
        if is_member.as_bool() {
            println!(r"User owns \demo directory.");
        } else {
            println!(r"User does not own \demo directory.");
        }

        LocalFree(pdescriptor.0 as isize);
        Ok(())
    }
}

[kennykerr]

@danielframpton / @sivadeilra may know if there's a reliable (e.g. works on Windows) way to do this with the Rust standard library. @jonwis may have additional API suggestions.

[jonwis]

@riverar 's answer is for the question "is the current token a member of a group that is the owner of the file." So if the ownership of a file was assigned to YourDomain\MuffinMakers (a group containing all the principals that make muffins for the domain) and was being run by YourDomain\Sally, the method returns "true" as long as Sally is a muffin maker.

If you were looking for a more precise answer, like "is this directory owned by the owner of this process", then you'd want to GetTokenInformation for TokenOwner and look at the TOKEN_OWNER structure's Owner field.

On rereading the motivation - should this user trust the files in this directory - the group membership check is probably right. A domain could have a rule saying that cloned repos must be owned by YourDomain\DevGroup5 and as long as Sally is also a member of DevGroup5, she should trust it.

(Note that "trust" is a word with a lot of baggage... what specific kinds of "trust" did you mean?)

[riverar]

@riverar 's answer is for the question "is the current token a member of a group that is the owner of the file." So if the ownership of a file was assigned to YourDomain\MuffinMakers (a group containing all the principals that make muffins for the domain) and was being run by YourDomain\Sally, the method returns "true" as long as Sally is a muffin maker.

I'm not seeing that behavior.

> dir /q
...
04/15/2022  10:33 AM    <DIR>          BUILTIN\Administrators demo
...

> cargo run
User does not own \demo directory.

(Probably a bad test as I'm using an unelevated token.)

[jonwis]

Is cargo run running with an elevated token? An un-elevated token has the Administrator group present, but disabled for purposes of "are you a muffin maker"

[riverar]

@jonwis Just realized my test was bad for that reason. Thanks!

[Byron]

wiping away the tears Thanks a lot for your swift help, @riverar, that does indeed work and CI is now green. The code is also much more readable what I had before (source code link for excerpt below)

pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result<bool> {
            use windows::Win32::{
                Foundation::{CloseHandle, ERROR_SUCCESS, HANDLE, PSID},
                Security,
                Security::Authorization::SE_FILE_OBJECT,
                System::Threading,
            };
            let mut handle = HANDLE::default();
            let mut descriptor = Security::PSECURITY_DESCRIPTOR::default();
            let mut err_msg = None;
            let mut is_owned = false;

            #[allow(unsafe_code)]
            unsafe {
                Threading::OpenProcessToken(Threading::GetCurrentProcess(), Security::TOKEN_QUERY, &mut handle)
                    .ok()
                    .map_err(|_| err("Failed to open process token"))?;

                let mut len = 0_u32;
                if !Security::GetTokenInformation(handle, Security::TokenUser, std::ptr::null_mut(), 0, &mut len)
                    .as_bool()
                {
                    let mut info_buf = Vec::<u8>::new();
                    info_buf.reserve_exact(len as usize);
                    if Security::GetTokenInformation(
                        handle,
                        Security::TokenUser,
                        info_buf.as_mut_ptr() as *mut std::ffi::c_void,
                        len,
                        &mut len,
                    )
                    .as_bool()
                    {
                        // NOTE: we avoid to copy the sid or cache it in any way for now, even though it should be possible
                        //       with a custom allocation/vec/box and it's just very raw. Can the `windows` crate do better?
                        //       When/If yes, then let's improve this.
                        //       It should however be possible to create strings from SIDs, check this once more.
                        let info: *const Security::TOKEN_USER = std::mem::transmute(info_buf.as_ptr());
                        if Security::IsValidSid((*info).User.Sid).as_bool() {
                            let wide_path = to_wide_path(&path);
                            let mut path_sid = PSID::default();
                            let res = Security::Authorization::GetNamedSecurityInfoW(
                                windows::core::PCWSTR(wide_path.as_ptr()),
                                SE_FILE_OBJECT,
                                Security::OWNER_SECURITY_INFORMATION | Security::DACL_SECURITY_INFORMATION,
                                &mut path_sid as *mut _,
                                std::ptr::null_mut(),
                                std::ptr::null_mut(),
                                std::ptr::null_mut(),
                                &mut descriptor as *mut _,
                            );

                            if res == ERROR_SUCCESS.0 && Security::IsValidSid(path_sid).as_bool() {
                                is_owned = Security::EqualSid(path_sid, (*info).User.Sid).as_bool();
                                dbg!(is_owned, path.as_ref());
                            } else {
                                err_msg = format!("couldn't get owner for path or it wasn't valid: {}", res).into();
                            }
                        } else {
                            err_msg = String::from("owner id of current process wasn't set or valid").into();
                        }
                    } else {
                        err_msg = String::from("Could not get information about the token user").into();
                    }
                } else {
                    err_msg = String::from("Could not get token information for length of token user").into();
                }
                CloseHandle(handle);
                if !descriptor.is_invalid() {
                    windows::core::heap_free(descriptor.0);
                }
            }
            err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned))
        }

The intend of the above is indeed to compare actual ownership, which is how git does it. That's why ultimately I'd love to do it with the same semantics even though I certainly don't care how.

@jonwis already mentioned that the above would be more inline with the semantics that were originally intended, and I'd love to get that to work even though for some reason it didn't.

Now I am with this implementation which serves as a stepping stone towards the ultimate goal of being 'git-compliant' :D.

The question remains, is there a way to check for actual ownership and can this be made to work?

PS: Right now testing for me is a bit painful as it has to happen on CI, but I am trying to get a Windows for ARM instance running within Parallels which should speed up this kind of development for me.
PS2: In case you are interested, my initial experience with the windows crate was positive especially compared to winapi , but I'd love to see more documentation on the basics. Right now most of what I found in the README was TODO.

[riverar]

Do consider replacing my sample above with a more precise version soon then! We can get you a sample using GetTokenInformation but first we need to fix the metadata describing this function. It's a bit busted and makes working with it very difficult. Standby.

[Byron]

By the way, I just successfully ran the test on windows with the GNU toolchain even, neat! I highlight this because I was surprised that it was 'so easy', at least after the msvc Rust toolchain failed due to MSVC not being installed.

Edit: I just realized that it installed the x86_64 toolchain despite being on ARM, and it worked nonetheless. This must be an issue with Rustup which should detect it's an ARM environment.
Edit2: It seems like rustup.exe is an x86 application which is why it defaults to these architectures. There is an aarch64-pc-windows-msvc target, but no GNU version of it. All of this is tangential, maybe it's interesting to some.

[kennykerr]

Glad you got it working!