Re: [ADVAPI] SetSecurityInfo

[Robert Shearman]

Raphael wrote:
> Hi,
>
> Changelog:
> - simple implementation of SetSecurityInfo
>
> (seems to be needed by War3 frozen throne, cf 
> http://forums.gentoo.org/viewtopic-t-303306.html?sid=3f962c702ae58ad8902ca83...)

I have made a few comments on the implementation below:
> SRCDIR    = @srcdir@
> VPATH     = @srcdir@
> MODULE    = advapi32.dll
> -IMPORTS   = kernel32 ntdll
> +IMPORTS   = winspool kernel32 ntdll

Should be in DELAYIMPORTS. We don't want to slow down the loading of all 
applications because of this one uncommon case.
> @@ -1418,6 +1422,87 @@
> {
>   FIXME("(%s) : stub\n", debugstr_w(lpFileName) );
>   return TRUE;
> +}
> -
>
> +/******************************************************************************
> -
> - SetSecurityInfo [ADVAPI32.@]
> - */
>
> +BOOL WINAPI SetSecurityInfo(HANDLE handle,
> -
> 	    SE_OBJECT_TYPE ObjectType,
> -
> 	    SECURITY_INFORMATION SecurityInfo,
> -
> 	    PSID psidOwner,
> -
> 	    PSID psidGroup,
> -
> 	    PACL pDacl,
> -
> 	    PACL pSacl)
>
> +{
> - SECURITY_DESCRIPTOR sd;
> - BOOL test;
> -
> - TRACE("(%p, %x, %x, %p, %p, %p, %p) : alomst stub\n", handle, ObjectType, SecurityInfo, psidOwner, psidGroup, pDacl, pSacl);

Remove the "almost stub" part of it and just print FIXME's for the parts 
that aren't implemented, otherwise the next person that comes along to 
this function will spend hours trying to work out what isn't 
implemented, if they even realise this function is to blame.
> -
> - test = InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION);
> - if (!test) return FALSE;
> -
> - if (SecurityInfo & DACL_SECURITY_INFORMATION) {
> - test = SetSecurityDescriptorDacl(&sd, TRUE, pDacl, TRUE);
> - }
> - if (SecurityInfo & SACL_SECURITY_INFORMATION) {
> - test = SetSecurityDescriptorSacl(&sd, TRUE, pSacl, TRUE);
> - }
> - if (SecurityInfo & GROUP_SECURITY_INFORMATION) {
> - if (NULL == psidGroup) {
> -
>  SetLastError( ERROR_INVALID_PARAMETER );
> -
>  return FALSE;
> - }
> - test = SetSecurityDescriptorGroup(&sd, psidGroup, TRUE);
> - }
> - if (SecurityInfo & OWNER_SECURITY_INFORMATION) {
> - if (NULL == psidOwner) {
> -
>  SetLastError( ERROR_INVALID_PARAMETER );
> -
>  return FALSE;
> - }
> - test = SetSecurityDescriptorOwner(&sd, psidOwner, TRUE);
> - }
> -
> - switch (ObjectType) {
> - case SE_KERNEL_OBJECT:
> - test = SetKernelObjectSecurity(handle, SecurityInfo, &sd);
> - break;
> - case SE_SERVICE:
> - test = SetServiceObjectSecurity(handle, SecurityInfo, &sd);
> - break;
> - case SE_REGISTRY_WOW64_32KEY:
> - case SE_REGISTRY_KEY:
> - test = RegSetKeySecurity(handle, SecurityInfo, &sd);
> - break;
> - case SE_LMSHARE:
> - /*test = NetShareSetInfo(handle, SecurityInfo, &sd); TODO: don't exit on netapi */
> - break;
> - case SE_PRINTER:
> - {
> -
>  PRINTER_INFO_2A pr_info;
> -
>  memset(&pr_info, 0, sizeof(pr_info));
> -
>  /** maybe we should do GetPrinter before SetSecurityDesc* ? */
> -
>  test = GetPrinterA(handle, 2, (LPBYTE) &pr_info, 0, NULL);
> -
>  pr_info.pSecurityDescriptor = &sd;
> -
>  test = SetPrinterA(handle, 2, (LPBYTE) &pr_info, PRINTER_CONTROL_SET_STATUS);
> -
>  break;
> - }
> -
> - case SE_FILE_OBJECT:
> - /*SetFileSecurity ? */
> - case SE_WINDOW_OBJECT:

Call SetUserObjectSecurity.
> - case SE_DS_OBJECT:
> - case SE_DS_OBJECT_ALL:
> - case SE_PROVIDER_DEFINED_OBJECT:
> - case SE_WMIGUID_OBJECT:
> - case SE_UNKNOWN_OBJECT_TYPE:
> - default:
> - WARN("unsupported ObjectType %d\n", ObjectType);

Change this to a FIXME.
> - test = TRUE;
> - }
> -
> - return test;

This function returns a DWORD error code, not a BOOL.
> }
>
> /******************************************************************************

Rob