rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

Add security_attributes() to windows::OpenOptionsExt #314

Open Adrien-Bodineau opened 7 months ago

Adrien-Bodineau commented 7 months ago

Proposal

Problem statement

There is currently no means, using std::fs, to provide SECURITY_ATTRIBUTES when opening a file. A private implementation of OpenOptions do have a security_attributes function but it is not expose through the OpenOptionExt trait. In order to provide a SECURITY_ATTRIBUTES, one must use the windows crate directly calling C ffi bindings.

Motivating examples or use cases

let file = std::fs::File::options()
    .access_mode(1179785u32) // FILE_GENERIC_READ
    .share_mode(0) // FILE_SHARE_NONE
    // no means to pass security attributes to add more security.
    .open("foo.txt").unwrap();

Solution sketch

Extends OpenOptionsExt for windows

pub trait OpenOptionsExt {
    // Required methods
    fn [access_mode](https://doc.rust-lang.org/std/os/windows/fs/trait.OpenOptionsExt.html#tymethod.access_mode)(&mut self, access: [u32](https://doc.rust-lang.org/std/primitive.u32.html)) -> [&mut Self](https://doc.rust-lang.org/std/primitive.reference.html);
    // ...

   fn security_attributes(&mut self, attributes: SECURITY_ATTRIBUTES) -> &mut Self;
}
let attributes = // ...;

let file = std::fs::File::options()
    .access_mode(1179785u32) // FILE_GENERIC_READ
    .share_mode(0) // FILE_SHARE_NONE
    .security_attributes(attributes)
    .open("foo.txt").unwrap();

Links and related work

windows_security.go

Design issue

The main design issue I see is how to provide a safe way to create security attributes.

the8472 commented 6 months ago

And how would we define a stable SECURITY_ATTRIBUTES type? Is that just a pointer?

ChrisDenton commented 6 months ago

I think the easiest thing to do would be to define it as a pointer and let the ecosystem decide on how to wrap it in a nicer API.

However, the problem with that is it would make OpenOptions::open unsafe to use. Callers of security_attributes would need to ensure that the memory pointed to (either directly or indirectly) outlasts the OpenOptions instance, which they can only do if they actually have some control over how long OpenOptions lives (or else they leak memory).

An alternative might be to have OpenOptions contain something like a Vec<u8> (except malloc-aligned) which can hold arbitrary data for the lifetime of OpenOptions. It would still be unsafe to construct but at least it could be safe thereafter as OpenOptions controls when it gets deallocated. The downside of that it requires allocating memory that could just as well live on the stack or somewhere else.

Another alternative would be for security_attributes to consume self and return a new struct, say WindowsOpenOptions, that wraps OpenOptions but adds a lifetime that can be used to assert the SECURITY_ATTRIBUTES struct (and all associated data) lives long enough.

Tbh, my thought is to just use a pointer on nightly and let the ecosystem figure out the hard questions. Before stabilizing we can revisit the problem and look at how people are using it.

Adrien-Bodineau commented 6 months ago

A solution could be to use the capability of a SecurityDescriptor to be serialized as a String to provide a safe interface using the documentation for the SDDL then internally create the pointer. Thus it would be possible to provide a safe interface over it.