[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 descr

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/o…
==============================================================================
--- 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=…
==============================================================================
--- 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=2…
==============================================================================
--- 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=…
==============================================================================
--- 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))
     {