[ros-diffs] [ion] 22279: - Fix ObGetObjectSecurity to use the object type's pool type instead of assuming PagedPool. - Re-wrote the way ObInsertObject handles security as described in Chapters 6 and 9 of Gl00my (ie: made it use ObGetObjectSecurity and ObAssignObjectSecurity; ironically, these functions already existed/are exported and could've been used since the start instead of duplicating code). - Fix ObpReferenceCachedSecurityDescriptor only to touch the cached entry if it actually gets a non-NULL descriptor. Also improved it to return the referenced SD, instead of requiring the caller to do it manually.
ion at svn.reactos.org
ion at svn.reactos.org
Thu Jun 8 07:41:39 CEST 2006
- Previous message: [ros-diffs] [ion] 22278: - I just noticed that ObInsertObject never got updated to deal with the improvements in ObFindObject and ACCESS_STATE usage, so made the following fixes: * Create the ACCESS_STATE structure much earlier. * Actually send the access state and parse context to ObFindObject, when called from ObInsertObject (this should fix some hidden regressions, since they finally get an access state with access masks now). * Remove some deprecated hacks. * If inserting the handle failed, cleanup the name and remove it from the directory entry. * Fix a memory leak because we weren't deleting the access state.
- Next message: [ros-diffs] [ion] 22280: - ObFindObject should return the actual object being inserted, in the insert case (otherwise it always returns the root-directory instead...) - Add check for optimized case for objects with no name and no security, but not implemented (ObpIncrementUnnamedHandleCount in Gl00my docs), since I need a better translation then babelfish's. - Fix ObInsertObject to save the Security Descriptor in the Access State structure. Gl00my mentions this isn't absorbed by SeCreateAccessCheck and I just noticed that too. - We only need to perform security checks for a new object, in ObInsertObject, not if the object already existed. - Added proper backout+failure code in ObInsertObject if lookup failed, and also look out for mismatch/exists/collision cases (implemented using simple logic).
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
Author: ion
Date: Thu Jun 8 09:41:39 2006
New Revision: 22279
URL: http://svn.reactos.ru/svn/reactos?rev=22279&view=rev
Log:
- Fix ObGetObjectSecurity to use the object type's pool type instead of assuming PagedPool.
- Re-wrote the way ObInsertObject handles security as described in Chapters 6 and 9 of Gl00my (ie: made it use ObGetObjectSecurity and ObAssignObjectSecurity; ironically, these functions already existed/are exported and could've been used since the start instead of duplicating code).
- Fix ObpReferenceCachedSecurityDescriptor only to touch the cached entry if it actually gets a non-NULL descriptor. Also improved it to return the referenced SD, instead of requiring the caller to do it manually.
Modified:
trunk/reactos/ntoskrnl/include/internal/ob.h
trunk/reactos/ntoskrnl/ob/obhandle.c
trunk/reactos/ntoskrnl/ob/sdcache.c
trunk/reactos/ntoskrnl/ob/security.c
Modified: trunk/reactos/ntoskrnl/include/internal/ob.h
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/include/internal/ob.h?rev=22279&r1=22278&r2=22279&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ob.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/ob.h Thu Jun 8 09:41:39 2006
@@ -172,9 +172,11 @@
NTAPI
ObpRemoveSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor);
-VOID
-NTAPI
-ObpReferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor);
+PSECURITY_DESCRIPTOR
+NTAPI
+ObpReferenceCachedSecurityDescriptor(
+ IN PSECURITY_DESCRIPTOR SecurityDescriptor
+);
VOID
NTAPI
Modified: trunk/reactos/ntoskrnl/ob/obhandle.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/obhandle.c?rev=22279&r1=22278&r2=22279&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/obhandle.c (original)
+++ trunk/reactos/ntoskrnl/ob/obhandle.c Thu Jun 8 09:41:39 2006
@@ -1167,8 +1167,8 @@
PVOID FoundObject = NULL;
POBJECT_HEADER FoundHeader = NULL;
NTSTATUS Status = STATUS_SUCCESS;
- PSECURITY_DESCRIPTOR NewSecurityDescriptor = NULL;
- SECURITY_SUBJECT_CONTEXT SubjectContext;
+ PSECURITY_DESCRIPTOR DirectorySd = NULL;
+ BOOLEAN SdAllocated;
OBP_LOOKUP_CONTEXT Context;
POBJECT_HEADER_NAME_INFO ObjectNameInfo;
ACCESS_STATE AccessState;
@@ -1252,48 +1252,58 @@
}
}
- DPRINT("Security Assignment in progress\n");
- SeCaptureSubjectContext(&SubjectContext);
-
- /* Build the new security descriptor */
- Status = SeAssignSecurity((FoundHeader != NULL) ? FoundHeader->SecurityDescriptor : NULL,
- (ObjectCreateInfo != NULL) ? ObjectCreateInfo->SecurityDescriptor : NULL,
- &NewSecurityDescriptor,
- (Header->Type == ObDirectoryType),
- &SubjectContext,
- &Header->Type->TypeInfo.GenericMapping,
- PagedPool);
-
- if (NT_SUCCESS(Status))
- {
- DPRINT("NewSecurityDescriptor %p\n", NewSecurityDescriptor);
-
- if (Header->Type->TypeInfo.SecurityProcedure != NULL)
- {
- /* Call the security method */
- Status = Header->Type->TypeInfo.SecurityProcedure(&Header->Body,
- AssignSecurityDescriptor,
- 0,
- NewSecurityDescriptor,
- NULL,
- NULL,
- NonPagedPool,
- NULL);
- }
- else
- {
- /* Assign the security descriptor to the object header */
- Status = ObpAddSecurityDescriptor(NewSecurityDescriptor,
- &Header->SecurityDescriptor);
- DPRINT("Object security descriptor %p\n", Header->SecurityDescriptor);
- }
-
- /* Release the new security descriptor */
- SeDeassignSecurity(&NewSecurityDescriptor);
- }
-
- DPRINT("Security Complete\n");
- SeReleaseSubjectContext(&SubjectContext);
+ /* Check if it's named or forces security */
+ if ((IsNamed) || (ObjectType->TypeInfo.SecurityRequired))
+ {
+ /* Make sure it's inserted into an object directory */
+ if ((ObjectNameInfo) && (ObjectNameInfo->Directory))
+ {
+ /* Get the current descriptor */
+ ObGetObjectSecurity(ObjectNameInfo->Directory,
+ &DirectorySd,
+ &SdAllocated);
+ }
+
+ /* Now assign it */
+ Status = ObAssignSecurity(PassedAccessState,
+ DirectorySd,
+ Object,
+ ObjectType);
+
+ /* Check if we captured one */
+ if (DirectorySd)
+ {
+ /* We did, release it */
+ DPRINT1("Here\n");
+ ObReleaseObjectSecurity(DirectorySd, SdAllocated);
+ }
+ else if (NT_SUCCESS(Status))
+ {
+ /* Other we didn't, but we were able to use the current SD */
+ SeReleaseSecurityDescriptor(ObjectCreateInfo->SecurityDescriptor,
+ ObjectCreateInfo->ProbeMode,
+ TRUE);
+
+ /* Clear the current one */
+ PassedAccessState->SecurityDescriptor =
+ ObjectCreateInfo->SecurityDescriptor = NULL;
+ }
+ }
+
+ /* Check if anything until now failed */
+ if (!NT_SUCCESS(Status))
+ {
+ /* We failed, dereference the object and delete the access state */
+ ObDereferenceObject(Object);
+ if (PassedAccessState == &AccessState)
+ {
+ /* We used a local one; delete it */
+ SeDeleteAccessState(PassedAccessState);
+ }
+
+ /* Return failure code */
+ return Status;
+ }
/* HACKHACK: Because of ROS's incorrect startup, this can be called
* without a valid Process until I finalize the startup patch,
@@ -1301,8 +1311,7 @@
* a handle if Handle is NULL when the Registry Code calls it, because
* the registry code totally bastardizes the Ob and needs to be fixed
*/
- DPRINT("Creating handle\n");
- if (Handle != NULL)
+ if (Handle)
{
/* Create the handle */
Status = ObpCreateHandle(OpenReason,
@@ -1338,7 +1347,6 @@
}
/* Return failure code */
- DPRINT("Status %x\n", Status);
return Status;
}
Modified: trunk/reactos/ntoskrnl/ob/sdcache.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/sdcache.c?rev=22279&r1=22278&r2=22279&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/sdcache.c (original)
+++ trunk/reactos/ntoskrnl/ob/sdcache.c Thu Jun 8 09:41:39 2006
@@ -306,30 +306,34 @@
return STATUS_SUCCESS;
}
+PSECURITY_DESCRIPTOR
+NTAPI
+ObpReferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)
+{
+ PSD_CACHE_ENTRY CacheEntry;
+
+ /* Lock the cache */
+ ObpSdCacheLock();
+
+ /* Make sure we got a descriptor */
+ if (SecurityDescriptor)
+ {
+ /* Get the entry */
+ CacheEntry = (PSD_CACHE_ENTRY)((ULONG_PTR)SecurityDescriptor -
+ sizeof(SD_CACHE_ENTRY));
+
+ /* Reference it */
+ CacheEntry->RefCount++;
+ DPRINT("RefCount %lu\n", CacheEntry->RefCount);
+ }
+
+ /* Unlock the cache and return the descriptor */
+ ObpSdCacheUnlock();
+ return SecurityDescriptor;
+}
VOID
NTAPI
-ObpReferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)
-{
- PSD_CACHE_ENTRY CacheEntry;
-
- DPRINT("ObpReferenceCachedSecurityDescriptor() called\n");
-
- ObpSdCacheLock();
-
- CacheEntry = (PSD_CACHE_ENTRY)((ULONG_PTR)SecurityDescriptor - sizeof(SD_CACHE_ENTRY));
-
- CacheEntry->RefCount++;
- DPRINT("RefCount %lu\n", CacheEntry->RefCount);
-
- ObpSdCacheUnlock();
-
- DPRINT("ObpReferenceCachedSecurityDescriptor() done\n");
-}
-
-
-VOID
-NTAPI
ObpDereferenceCachedSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor)
{
DPRINT("ObpDereferenceCachedSecurityDescriptor() called\n");
Modified: trunk/reactos/ntoskrnl/ob/security.c
URL: http://svn.reactos.ru/svn/reactos/trunk/reactos/ntoskrnl/ob/security.c?rev=22279&r1=22278&r2=22279&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ob/security.c (original)
+++ trunk/reactos/ntoskrnl/ob/security.c Thu Jun 8 09:41:39 2006
@@ -117,9 +117,9 @@
/* Check if the object uses default security */
if (Type->TypeInfo.SecurityProcedure == SeDefaultObjectMethod)
{
- /* Reference the descriptor and return it */
- ObpReferenceCachedSecurityDescriptor(Header->SecurityDescriptor);
- *SecurityDescriptor = Header->SecurityDescriptor;
+ /* Reference the descriptor */
+ *SecurityDescriptor =
+ ObpReferenceCachedSecurityDescriptor(Header->SecurityDescriptor);
/* Tell the caller that we didn't have to allocate anything */
*MemoryAllocated = FALSE;
@@ -134,10 +134,10 @@
GROUP_SECURITY_INFORMATION |
DACL_SECURITY_INFORMATION |
SACL_SECURITY_INFORMATION,
- NULL,
+ *SecurityDescriptor,
&Length,
&Header->SecurityDescriptor,
- PagedPool,
+ Type->TypeInfo.PoolType,
&Type->TypeInfo.GenericMapping);
if (Status != STATUS_BUFFER_TOO_SMALL) return Status;
@@ -158,7 +158,7 @@
*SecurityDescriptor,
&Length,
&Header->SecurityDescriptor,
- PagedPool,
+ Type->TypeInfo.PoolType,
&Type->TypeInfo.GenericMapping);
if (!NT_SUCCESS(Status))
{
- Previous message: [ros-diffs] [ion] 22278: - I just noticed that ObInsertObject never got updated to deal with the improvements in ObFindObject and ACCESS_STATE usage, so made the following fixes: * Create the ACCESS_STATE structure much earlier. * Actually send the access state and parse context to ObFindObject, when called from ObInsertObject (this should fix some hidden regressions, since they finally get an access state with access masks now). * Remove some deprecated hacks. * If inserting the handle failed, cleanup the name and remove it from the directory entry. * Fix a memory leak because we weren't deleting the access state.
- Next message: [ros-diffs] [ion] 22280: - ObFindObject should return the actual object being inserted, in the insert case (otherwise it always returns the root-directory instead...) - Add check for optimized case for objects with no name and no security, but not implemented (ObpIncrementUnnamedHandleCount in Gl00my docs), since I need a better translation then babelfish's. - Fix ObInsertObject to save the Security Descriptor in the Access State structure. Gl00my mentions this isn't absorbed by SeCreateAccessCheck and I just noticed that too. - We only need to perform security checks for a new object, in ObInsertObject, not if the object already existed. - Added proper backout+failure code in ObInsertObject if lookup failed, and also look out for mismatch/exists/collision cases (implemented using simple logic).
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
More information about the Ros-diffs
mailing list