Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add security_attributes() to windows::OpenOptionsExt #314

Open
Adrien-Bodineau opened this issue Dec 14, 2023 · 3 comments
Open

Add security_attributes() to windows::OpenOptionsExt #314

Adrien-Bodineau opened this issue Dec 14, 2023 · 3 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@Adrien-Bodineau
Copy link

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.

@Adrien-Bodineau Adrien-Bodineau added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 14, 2023
@the8472
Copy link
Member

the8472 commented Jan 5, 2024

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

@ChrisDenton
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants