dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

Kernel32.ACCESS_MASK could be more "open" to being extended #271

Closed arlm closed 1 year ago

arlm commented 8 years ago

For the first time in the project we have hit and seen a specialization of the ACCESS_MASK on the wild. It was introduced for Desktops and Window Stations. We might expect this to happen more frequently as we start porting more security and access related functions and APIs.

I have created a second and third, specialized, ACCESS_MASK under the names of DESKTOP_ACCESS_MASK and WINDOW_STATION_ACCESS_MASK on the User32 class. They have the ability to be compared and converted to ACCESS_MASK using implicit and explicit casts.

Here is the place for us to discuss a better approach for this ACCESS_MASK extending problem we have hit.

Citing @vbfox on PR #268 comments here (quote 1) and here (quote 2) to aggregate relevant discussion topics on this issue.

quote 1:

As each subsystem will have it's own access mask, maybe we should decide on 2 things: How can we add values so that it's easy to use for 1.x and maybe how show we refactor it for 2.x (And is it even necessary)

quote 2:

I don't know if there is a perfect solution here, the classical solution is to use classes instead of enums (Like old school java) but we need a solution that works with the marshaler (Or it could involve one more code transformation like FriendlyAttribute).

arlm commented 8 years ago

A little more quotes, for reference:

from @AArnott here:

You seem to have done a careful job at making this type interact well with ACCESS_MASK. But I'm curious why we needed an extra type here rather than just adding more to ACCESS_MASK. In particular, I see GENERIC_WRITE is defined in winnt.h, so I'd initially say this should go in Windows.Core. But it certainly seems compatible with putting into the Kernel32+ACCESS_MASK type. Although looking more closely I see you're defining it in terms of new DESKTOP_* constants, which is interesting. Can you tell me more about these? Is ACCESS_MASK something where different domains will define values differently?

from me here:

Well as far as I understood ACCESS_MASK has some levels of "flagging" things and there is a specific part that other APIs can extend. This semantics is really interesting and the guy that inputted the Kernel32 port placed a really nice link that enlightened things for me.

On the Window Station API documentation, as well as on the Desktop API documentation, the specific flags semantics for these kinds of objects are explained in detail. I would just extend the Kernel32.ACCESS_MASK or the Kernel32.ACCESS_MASK.SpecificRight if I could without much refactoring but Kernel32.ACCESS_MASK.SpecificRight is an enum and cant be extended, also Kernel32.ACCESS_MASK would need some refactoring to be able to have extensions coming from other packages. As far as I understood we will see this kind of things happen more and more when we start mapping the security and access functions more aggressively.

What I tried to do instead is make the least possible to make it compatible with the Kernel32.ACCESS_MASK struct, being convertible and comparable with it so that we could use any one. The main difference is on the SpecificRight flags and the fact the the GENERIC_* flags change semantics and meaning when applied on these kind of objects.