Added patch for initial implementation of advapi32.BuildSecurityDescriptorW.

This commit is contained in:
Sebastian Lackner
2017-05-05 00:25:14 +02:00
parent 9985fe9035
commit a08b6b2b62
4 changed files with 536 additions and 168 deletions

View File

@@ -0,0 +1,268 @@
From 994fe46f1b68d851d285a29cce904bd9f22540ea Mon Sep 17 00:00:00 2001
From: Andrew Wesie <awesie@gmail.com>
Date: Tue, 2 May 2017 00:59:49 -0500
Subject: advapi32: Implement BuildSecurityDescriptorW.
---
dlls/advapi32/security.c | 218 +++++++++++++++++++++++++++++++++++------------
1 file changed, 164 insertions(+), 54 deletions(-)
diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c
index 24ec3099713..82bb6689d43 100644
--- a/dlls/advapi32/security.c
+++ b/dlls/advapi32/security.c
@@ -58,6 +58,7 @@ static BOOL ParseStringSecurityDescriptorToSecurityDescriptor(
SECURITY_DESCRIPTOR_RELATIVE* SecurityDescriptor,
LPDWORD cBytes);
static DWORD ParseAclStringFlags(LPCWSTR* StringAcl);
+static DWORD trustee_to_sid(DWORD nDestinationSidLength, PSID pDestinationSid, PTRUSTEEW pTrustee);
typedef struct _ACEFLAG
{
@@ -1264,16 +1265,122 @@ DWORD WINAPI BuildSecurityDescriptorW(
IN ULONG cCountOfAccessEntries,
IN PEXPLICIT_ACCESSW pListOfAccessEntries,
IN ULONG cCountOfAuditEntries,
- IN PEXPLICIT_ACCESSW pListofAuditEntries,
+ IN PEXPLICIT_ACCESSW pListOfAuditEntries,
IN PSECURITY_DESCRIPTOR pOldSD,
IN OUT PULONG lpdwBufferLength,
OUT PSECURITY_DESCRIPTOR* pNewSD)
{
- FIXME("(%p,%p,%d,%p,%d,%p,%p,%p,%p) stub!\n",pOwner,pGroup,
- cCountOfAccessEntries,pListOfAccessEntries,cCountOfAuditEntries,
- pListofAuditEntries,pOldSD,lpdwBufferLength,pNewSD);
+ SECURITY_DESCRIPTOR desc;
+ NTSTATUS status;
+ DWORD ret = ERROR_SUCCESS;
+
+ TRACE("(%p,%p,%d,%p,%d,%p,%p,%p,%p)\n", pOwner, pGroup,
+ cCountOfAccessEntries, pListOfAccessEntries, cCountOfAuditEntries,
+ pListOfAuditEntries, pOldSD, lpdwBufferLength, pNewSD);
- return ERROR_CALL_NOT_IMPLEMENTED;
+ if (pOldSD)
+ {
+ SECURITY_DESCRIPTOR_CONTROL control;
+ DWORD desc_size, dacl_size = 0, sacl_size = 0, owner_size = 0, group_size = 0;
+ PACL dacl = NULL, sacl = NULL;
+ PSID owner = NULL, group = NULL;
+ DWORD revision;
+
+ if ((status = RtlGetControlSecurityDescriptor( pOldSD, &control, &revision )) != STATUS_SUCCESS)
+ return RtlNtStatusToDosError( status );
+ if (!(control & SE_SELF_RELATIVE))
+ return ERROR_INVALID_SECURITY_DESCR;
+
+ desc_size = sizeof(desc);
+ status = RtlSelfRelativeToAbsoluteSD( pOldSD, &desc, &desc_size, dacl, &dacl_size, sacl, &sacl_size,
+ owner, &owner_size, group, &group_size );
+ if (status == STATUS_BUFFER_TOO_SMALL)
+ {
+ if (dacl_size)
+ dacl = LocalAlloc( LMEM_FIXED, dacl_size );
+ if (sacl_size)
+ sacl = LocalAlloc( LMEM_FIXED, sacl_size );
+ if (owner_size)
+ owner = LocalAlloc( LMEM_FIXED, owner_size );
+ if (group_size)
+ group = LocalAlloc( LMEM_FIXED, group_size );
+
+ desc_size = sizeof(desc);
+ status = RtlSelfRelativeToAbsoluteSD( pOldSD, &desc, &desc_size, dacl, &dacl_size, sacl, &sacl_size,
+ owner, &owner_size, group, &group_size );
+ }
+ if (status != STATUS_SUCCESS)
+ {
+ LocalFree( dacl );
+ LocalFree( sacl );
+ LocalFree( owner );
+ LocalFree( group );
+ return RtlNtStatusToDosError( status );
+ }
+ }
+ else
+ {
+ if ((status = RtlCreateSecurityDescriptor( &desc, SECURITY_DESCRIPTOR_REVISION )) != STATUS_SUCCESS)
+ return RtlNtStatusToDosError( status );
+ }
+
+ if (pOwner)
+ {
+ LocalFree( desc.Owner );
+ desc.Owner = LocalAlloc( LMEM_FIXED, sizeof(MAX_SID) );
+ if ((ret = trustee_to_sid( sizeof(MAX_SID), desc.Owner, pOwner )))
+ goto done;
+ }
+
+ if (pGroup)
+ {
+ LocalFree( desc.Group );
+ desc.Group = LocalAlloc( LMEM_FIXED, sizeof(MAX_SID) );
+ if ((ret = trustee_to_sid( sizeof(MAX_SID), desc.Group, pGroup )))
+ goto done;
+ }
+
+ if (pListOfAccessEntries)
+ {
+ PACL new_dacl;
+
+ if ((ret = SetEntriesInAclW( cCountOfAccessEntries, pListOfAccessEntries, desc.Dacl, &new_dacl )))
+ goto done;
+
+ LocalFree( desc.Dacl );
+ desc.Dacl = new_dacl;
+ desc.Control |= SE_DACL_PRESENT;
+ }
+
+ if (pListOfAuditEntries)
+ {
+ PACL new_sacl;
+
+ if ((ret = SetEntriesInAclW( cCountOfAuditEntries, pListOfAuditEntries, desc.Sacl, &new_sacl )))
+ goto done;
+
+ LocalFree( desc.Sacl );
+ desc.Sacl = new_sacl;
+ desc.Control |= SE_SACL_PRESENT;
+ }
+
+ *lpdwBufferLength = RtlLengthSecurityDescriptor( &desc );
+ *pNewSD = LocalAlloc( LMEM_FIXED, *lpdwBufferLength );
+
+ if ((status = RtlMakeSelfRelativeSD( &desc, *pNewSD, lpdwBufferLength )) != STATUS_SUCCESS)
+ {
+ ret = RtlNtStatusToDosError( status );
+ LocalFree( *pNewSD );
+ *pNewSD = NULL;
+ }
+
+done:
+ /* free absolute descriptor */
+ LocalFree( desc.Owner );
+ LocalFree( desc.Group );
+ LocalFree( desc.Sacl );
+ LocalFree( desc.Dacl );
+ return ret;
}
/******************************************************************************
@@ -3766,6 +3873,56 @@ static void free_trustee_name(TRUSTEE_FORM form, WCHAR *trustee_nameW)
}
}
+static DWORD trustee_to_sid( DWORD nDestinationSidLength, PSID pDestinationSid, PTRUSTEEW pTrustee )
+{
+ if (pTrustee->MultipleTrusteeOperation == TRUSTEE_IS_IMPERSONATE)
+ {
+ WARN("bad multiple trustee operation %d\n", pTrustee->MultipleTrusteeOperation);
+ return ERROR_INVALID_PARAMETER;
+ }
+
+ switch (pTrustee->TrusteeForm)
+ {
+ case TRUSTEE_IS_SID:
+ if (!CopySid(nDestinationSidLength, pDestinationSid, pTrustee->ptstrName))
+ {
+ WARN("bad sid %p\n", pTrustee->ptstrName);
+ return ERROR_INVALID_PARAMETER;
+ }
+ break;
+ case TRUSTEE_IS_NAME:
+ {
+ DWORD sid_size = nDestinationSidLength;
+ DWORD domain_size = MAX_COMPUTERNAME_LENGTH + 1;
+ SID_NAME_USE use;
+ if (!strcmpW( pTrustee->ptstrName, CURRENT_USER ))
+ {
+ if (!lookup_user_account_name( pDestinationSid, &sid_size, NULL, &domain_size, &use ))
+ {
+ return GetLastError();
+ }
+ }
+ else if (!LookupAccountNameW(NULL, pTrustee->ptstrName, pDestinationSid, &sid_size, NULL, &domain_size, &use))
+ {
+ WARN("bad user name %s\n", debugstr_w(pTrustee->ptstrName));
+ return ERROR_INVALID_PARAMETER;
+ }
+ break;
+ }
+ case TRUSTEE_IS_OBJECTS_AND_SID:
+ FIXME("TRUSTEE_IS_OBJECTS_AND_SID unimplemented\n");
+ break;
+ case TRUSTEE_IS_OBJECTS_AND_NAME:
+ FIXME("TRUSTEE_IS_OBJECTS_AND_NAME unimplemented\n");
+ break;
+ default:
+ WARN("bad trustee form %d\n", pTrustee->TrusteeForm);
+ return ERROR_INVALID_PARAMETER;
+ }
+
+ return ERROR_SUCCESS;
+}
+
/******************************************************************************
* SetEntriesInAclA [ADVAPI32.@]
*/
@@ -3861,56 +4018,9 @@ DWORD WINAPI SetEntriesInAclW( ULONG count, PEXPLICIT_ACCESSW pEntries,
pEntries[i].Trustee.TrusteeForm, pEntries[i].Trustee.TrusteeType,
pEntries[i].Trustee.ptstrName);
- if (pEntries[i].Trustee.MultipleTrusteeOperation == TRUSTEE_IS_IMPERSONATE)
- {
- WARN("bad multiple trustee operation %d for trustee %d\n", pEntries[i].Trustee.MultipleTrusteeOperation, i);
- ret = ERROR_INVALID_PARAMETER;
- goto exit;
- }
-
- switch (pEntries[i].Trustee.TrusteeForm)
- {
- case TRUSTEE_IS_SID:
- if (!CopySid(FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]),
- ppsid[i], pEntries[i].Trustee.ptstrName))
- {
- WARN("bad sid %p for trustee %d\n", pEntries[i].Trustee.ptstrName, i);
- ret = ERROR_INVALID_PARAMETER;
- goto exit;
- }
- break;
- case TRUSTEE_IS_NAME:
- {
- DWORD sid_size = FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]);
- DWORD domain_size = MAX_COMPUTERNAME_LENGTH + 1;
- SID_NAME_USE use;
- if (!strcmpW( pEntries[i].Trustee.ptstrName, CURRENT_USER ))
- {
- if (!lookup_user_account_name( ppsid[i], &sid_size, NULL, &domain_size, &use ))
- {
- ret = GetLastError();
- goto exit;
- }
- }
- else if (!LookupAccountNameW(NULL, pEntries[i].Trustee.ptstrName, ppsid[i], &sid_size, NULL, &domain_size, &use))
- {
- WARN("bad user name %s for trustee %d\n", debugstr_w(pEntries[i].Trustee.ptstrName), i);
- ret = ERROR_INVALID_PARAMETER;
- goto exit;
- }
- break;
- }
- case TRUSTEE_IS_OBJECTS_AND_SID:
- FIXME("TRUSTEE_IS_OBJECTS_AND_SID unimplemented\n");
- break;
- case TRUSTEE_IS_OBJECTS_AND_NAME:
- FIXME("TRUSTEE_IS_OBJECTS_AND_NAME unimplemented\n");
- break;
- default:
- WARN("bad trustee form %d for trustee %d\n", pEntries[i].Trustee.TrusteeForm, i);
- ret = ERROR_INVALID_PARAMETER;
+ ret = trustee_to_sid( FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]), ppsid[i], &pEntries[i].Trustee);
+ if (ret)
goto exit;
- }
/* Note: we overestimate the ACL size here as a tradeoff between
* instructions (simplicity) and memory */
--
2.12.2

View File

@@ -0,0 +1,69 @@
From 63082c3863d8be466ed14f532653ddf35e40328a Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Fri, 5 May 2017 00:18:50 +0200
Subject: advapi32/tests: Add basic tests for BuildSecurityDescriptor.
---
dlls/advapi32/tests/security.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index d6ea3a19fad..c591f7b6e5f 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -7489,6 +7489,44 @@ static void test_child_token_sd(void)
HeapFree(GetProcessHeap(), 0, sd);
}
+static void test_BuildSecurityDescriptorW(void)
+{
+ SECURITY_DESCRIPTOR old_sd, *new_sd, *rel_sd;
+ ULONG new_sd_size;
+ DWORD buf_size;
+ char buf[1024];
+ BOOL success;
+ DWORD ret;
+
+ InitializeSecurityDescriptor(&old_sd, SECURITY_DESCRIPTOR_REVISION);
+
+ buf_size = sizeof(buf);
+ rel_sd = (SECURITY_DESCRIPTOR *)buf;
+ success = MakeSelfRelativeSD(&old_sd, rel_sd, &buf_size);
+ ok(success, "MakeSelfRelativeSD failed with %u\n", GetLastError());
+
+ new_sd = NULL;
+ new_sd_size = 0;
+ ret = BuildSecurityDescriptorW(NULL, NULL, 0, NULL, 0, NULL, NULL, &new_sd_size, (void **)&new_sd);
+ ok(ret == ERROR_SUCCESS, "BuildSecurityDescriptor failed with %u\n", ret);
+ ok(new_sd != NULL, "expected new_sd != NULL\n");
+ ok(new_sd_size == sizeof(old_sd), "expected new_sd_size == sizeof(old_sd), got %u\n", new_sd_size);
+ LocalFree(new_sd);
+
+ new_sd = (void *)0xdeadbeef;
+ ret = BuildSecurityDescriptorW(NULL, NULL, 0, NULL, 0, NULL, &old_sd, &new_sd_size, (void **)&new_sd);
+ ok(ret == ERROR_INVALID_SECURITY_DESCR, "expected ERROR_INVALID_SECURITY_DESCR, got %u\n", ret);
+ ok(new_sd == (void *)0xdeadbeef, "expected new_sd == 0xdeadbeef, got %p\n", new_sd);
+
+ new_sd = NULL;
+ new_sd_size = 0;
+ ret = BuildSecurityDescriptorW(NULL, NULL, 0, NULL, 0, NULL, rel_sd, &new_sd_size, (void **)&new_sd);
+ ok(ret == ERROR_SUCCESS, "BuildSecurityDescriptor failed with %u\n", ret);
+ ok(new_sd != NULL, "expected new_sd != NULL\n");
+ ok(new_sd_size == sizeof(old_sd), "expected new_sd_size == sizeof(old_sd), got %u\n", new_sd_size);
+ LocalFree(new_sd);
+}
+
START_TEST(security)
{
init();
@@ -7542,6 +7580,7 @@ START_TEST(security)
test_pseudo_tokens();
test_maximum_allowed();
test_GetExplicitEntriesFromAclW();
+ test_BuildSecurityDescriptorW();
/* must be the last test, modifies process token */
test_token_security_descriptor();
--
2.12.2

View File

@@ -0,0 +1,2 @@
Fixes: Initial implementation of advapi32.BuildSecurityDescriptorW
Depends: server-LABEL_SECURITY_INFORMATION