[PATCH 3/4] advapi32: Add DACL inheritance support in SetSecurityInfo

Piotr Caban piotr.caban at gmail.com
Fri Mar 27 04:43:58 CDT 2015


> On Thu, Mar 26, 2015 at 2:59 PM, Erich E. Hoover
> <erich.e.hoover at wine-staging.com> wrote:
>> On Thu, Mar 26, 2015 at 12:10 PM, Piotr Caban <piotr.caban at gmail.com> wrote:
>>> ...
>>> The inheritance doesn't occur when function from ntdll are used for setting
>>> DACL. This makes your code inside server incorrect. Also your code is not
>>> checking handle permissions correctly. If you try to test it keep in mind
>>> that if correct inheritance flags are set in security descriptor control
>>> field the permissions will be filled when you right-click on file and
>>> display security properties on windows.
>>
>> I don’t have the time to write a test at the moment, but I find it
>> highly unlikely that MS would implement security descriptors in a
>> high-level DLL.  If that were the case then an application could
>> easily hijack the API in order to permit access to files that would
>> otherwise be inaccessible.  I would guess that
>> UNPROTECTED_DACL_SECURITY_INFORMATION and SE_DACL_AUTO_INHERITED
>> probably enter into this somehow.
I'll send a test that shows the lack of inheritance in 
NtSetSecurityObject. Of course it's not a problem in the API, if we have 
rights to change the permissions we can do whatever we want anyway.

>> Besides that I think we should start implementing stored ACLs first,
>> before even thinking about implementing suitable inheritance
>> mechanisms.  I submitted a bunch of patches for this some time ago,
>> but Alexandre wasn’t happy with using extended user attributes to
>> store this information.  I’m just not sure that it’s useful to
>> implement inheritance without storing the ACLs, it’ll certainly fix
>> some apps but it will likely break a bunch of others.
I don't think complete support for storing security descriptors needs to 
be done first. This is the step in correct direction.

Thanks,
Piotr



More information about the wine-devel mailing list