Windows: Police Library IOCTLs

Ensure that the callers of the various library ioctls have
the correct identity or privs.  All this policing is done in
the fs (non unloadable) layer, and to ensure that the library
layer cannot receive these calls directly we forbid non
create Opens of the library control device.

Change-Id: I2342fe10047642082adfbd1cc6aaee09cc91b520
Reviewed-on: http://gerrit.openafs.org/8893
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/WINNT/afsrdr/kernel/fs/AFSCommSupport.cpp b/src/WINNT/afsrdr/kernel/fs/AFSCommSupport.cpp
index cb3cde7..2905dc9 100644 (file)
--- a/src/WINNT/afsrdr/kernel/fs/AFSCommSupport.cpp
+++ b/src/WINNT/afsrdr/kernel/fs/AFSCommSupport.cpp
@@ -385,6 +385,157 @@ try_exit:
     return ntStatus;
 }
 
+static
+NTSTATUS
+AFSCheckIoctlPermissions( IN ULONG ControlCode)
+{
+    switch ( ControlCode)
+    {
+        //
+        // First the FS ioctls
+        //
+
+        case IOCTL_AFS_INITIALIZE_CONTROL_DEVICE:
+
+            //
+            // Only a System Service can run this (unless we are compiled
+            // for debug with the correct flags set.
+            //
+
+            if ( !AFSIsUser( SeExports->SeLocalSystemSid)
+#if DBG
+                && !BooleanFlagOn( AFSDebugFlags, AFS_DBG_DISABLE_SYSTEM_SID_CHECK)
+#endif
+                )
+            {
+
+                return STATUS_ACCESS_DENIED;
+            }
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_INITIALIZE_REDIRECTOR_DEVICE:
+        case IOCTL_AFS_PROCESS_IRP_REQUEST:
+        case IOCTL_AFS_PROCESS_IRP_RESULT:
+        case IOCTL_AFS_SYSNAME_NOTIFICATION:
+        case IOCTL_AFS_SHUTDOWN:
+
+            //
+            // Once initialized, only the service can call these
+            //
+
+            if ( !AFSIsService())
+            {
+
+                return STATUS_ACCESS_DENIED;
+            }
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_CONFIGURE_DEBUG_TRACE:
+        case IOCTL_AFS_GET_TRACE_BUFFER:
+        case IOCTL_AFS_FORCE_CRASH:
+
+            //
+            // Any admin can call these
+            //
+
+            if ( !AFSIsInGroup( SeExports->SeAliasAdminsSid))
+            {
+
+                return STATUS_ACCESS_DENIED;
+            }
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_AUTHGROUP_CREATE_AND_SET:
+        case IOCTL_AFS_AUTHGROUP_QUERY:
+        case IOCTL_AFS_AUTHGROUP_SET:
+        case IOCTL_AFS_AUTHGROUP_RESET:
+        case IOCTL_AFS_AUTHGROUP_LOGON_CREATE:
+        case IOCTL_AFS_AUTHGROUP_SID_CREATE:
+        case IOCTL_AFS_AUTHGROUP_SID_QUERY:
+
+            //
+            // Anyone can call these.
+            //
+
+            return STATUS_SUCCESS;
+
+        //
+        // And now the LIB ioctls
+        //
+
+        case IOCTL_AFS_INITIALIZE_LIBRARY_DEVICE:
+
+            //
+            // Only the kernel can issue this
+            //
+
+            return STATUS_ACCESS_DENIED;
+
+        case IOCTL_AFS_STATUS_REQUEST:
+        case 0x140390:      // IOCTL_LMR_DISABLE_LOCAL_BUFFERING
+
+            //
+            // Anyone can call these.
+            //
+
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_ADD_CONNECTION:
+        case IOCTL_AFS_CANCEL_CONNECTION:
+        case IOCTL_AFS_GET_CONNECTION:
+        case IOCTL_AFS_LIST_CONNECTIONS:
+        case IOCTL_AFS_GET_CONNECTION_INFORMATION:
+
+            //
+            // These must only be called by the network provider but we
+            // don't have a method of enforcing that at the moment.
+            //
+
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_SET_FILE_EXTENTS:
+        case IOCTL_AFS_RELEASE_FILE_EXTENTS:
+        case IOCTL_AFS_SET_FILE_EXTENT_FAILURE:
+        case IOCTL_AFS_INVALIDATE_CACHE:
+        case IOCTL_AFS_NETWORK_STATUS:
+        case IOCTL_AFS_VOLUME_STATUS:
+
+            //
+            // Again, service only
+            //
+
+            if ( !AFSIsService())
+            {
+
+                return STATUS_ACCESS_DENIED;
+            }
+            return STATUS_SUCCESS;
+
+        case IOCTL_AFS_GET_OBJECT_INFORMATION:
+
+            //
+            // Admins only
+            //
+
+            if ( !AFSIsInGroup( SeExports->SeAliasAdminsSid))
+            {
+
+                return STATUS_ACCESS_DENIED;
+            }
+            return STATUS_SUCCESS;
+
+        default:
+
+            //
+            // NOTE that for security we police all known functions here
+            // and return STATUS_NOT_IMPLEMENTED.  So new ioctls need to
+            // be added both here and either below or in
+            // ..\lib\AFSDevControl.cpp
+            //
+
+            return STATUS_NOT_IMPLEMENTED;
+    }
+}
 NTSTATUS
 AFSProcessControlRequest( IN PIRP Irp)
 {
@@ -402,21 +553,18 @@ AFSProcessControlRequest( IN PIRP Irp)
 
         ulIoControlCode = pIrpSp->Parameters.DeviceIoControl.IoControlCode;
 
+        ntStatus = AFSCheckIoctlPermissions( ulIoControlCode);
+
+        if ( !NT_SUCCESS( ntStatus))
+        {
+            try_return( ntStatus);
+        }
+
         switch( ulIoControlCode)
         {
 
             case IOCTL_AFS_INITIALIZE_CONTROL_DEVICE:
             {
-                if ( !AFSIsUser( SeExports->SeLocalSystemSid)
-#if DBG
-                    && !BooleanFlagOn( AFSDebugFlags, AFS_DBG_DISABLE_SYSTEM_SID_CHECK)
-#endif
-                    )
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-                    break;
-                }
                 //
                 // Go intialize the pool
                 //
@@ -448,14 +596,6 @@ AFSProcessControlRequest( IN PIRP Irp)
 
                 AFSRedirectorInitInfo *pRedirInitInfo = (AFSRedirectorInitInfo *)Irp->AssociatedIrp.SystemBuffer;
 
-                if ( !AFSIsService())
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-
-                    break;
-                }
-
                 //
                 // Extract off the passed in information which contains the
                 // cache file parameters
@@ -495,14 +635,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             case IOCTL_AFS_PROCESS_IRP_REQUEST:
             {
 
-                if ( !AFSIsService())
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-
-                    break;
-                }
-
                 ntStatus = AFSProcessIrpRequest( Irp);
 
                 break;
@@ -511,14 +643,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             case IOCTL_AFS_PROCESS_IRP_RESULT:
             {
 
-                if ( !AFSIsService())
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-
-                    break;
-                }
-
                 ntStatus = AFSProcessIrpResult( Irp);
 
                 break;
@@ -529,14 +653,6 @@ AFSProcessControlRequest( IN PIRP Irp)
 
                 AFSSysNameNotificationCB *pSysNameInfo = (AFSSysNameNotificationCB *)Irp->AssociatedIrp.SystemBuffer;
 
-                if ( !AFSIsService())
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-
-                    break;
-                }
-
                 if( pSysNameInfo == NULL ||
                     pIrpSp->Parameters.DeviceIoControl.InputBufferLength < sizeof( AFSSysNameNotificationCB))
                 {
@@ -557,13 +673,6 @@ AFSProcessControlRequest( IN PIRP Irp)
 
                 AFSTraceConfigCB *pTraceInfo = (AFSTraceConfigCB *)Irp->AssociatedIrp.SystemBuffer;
 
-                if ( !AFSIsInGroup( SeExports->SeAliasAdminsSid))
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-                    break;
-                }
-
                 if( pTraceInfo == NULL ||
                     pIrpSp->Parameters.DeviceIoControl.InputBufferLength < sizeof( AFSTraceConfigCB))
                 {
@@ -581,13 +690,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             case IOCTL_AFS_GET_TRACE_BUFFER:
             {
 
-                if ( !AFSIsInGroup( SeExports->SeAliasAdminsSid))
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-                    break;
-                }
-
                 if( pIrpSp->Parameters.DeviceIoControl.OutputBufferLength == 0)
                 {
 
@@ -606,13 +708,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             case IOCTL_AFS_FORCE_CRASH:
             {
 
-                if ( !AFSIsInGroup( SeExports->SeAliasAdminsSid))
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-                    break;
-                }
-
 #if DBG
 
                 if( BooleanFlagOn( AFSDebugFlags, AFS_DBG_FLAG_ENABLE_FORCE_CRASH))
@@ -689,14 +784,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             case IOCTL_AFS_SHUTDOWN:
             {
 
-                if ( !AFSIsService())
-                {
-
-                    ntStatus = STATUS_ACCESS_DENIED;
-
-                    break;
-                }
-
                 ntStatus = AFSShutdownRedirector();
 
                 break;
@@ -851,8 +938,6 @@ AFSProcessControlRequest( IN PIRP Irp)
             }
         }
 
-//try_exit:
-
     }
     __except( AFSExceptionFilter( __FUNCTION__, GetExceptionCode(), GetExceptionInformation()))
     {
@@ -862,6 +947,8 @@ AFSProcessControlRequest( IN PIRP Irp)
         AFSDumpTraceFilesFnc();
     }
 
+try_exit:
+
     if( bCompleteRequest)
     {
 

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
index 3685830..efe1a8c 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
@@ -3452,11 +3452,21 @@ AFSControlDeviceCreate( IN PIRP Irp)
     __Enter
     {
 
-        //
-        // For now, jsut let the open happen
-        //
-
-        Irp->IoStatus.Information = FILE_OPENED;
+        if ( KernelMode == Irp->RequestorMode) {
+            //
+            // For now, just let the open happen
+            //
+            Irp->IoStatus.Information = FILE_OPENED;
+        }
+        else
+        {
+            //
+            // Not from usermode, All access must be via
+            // the FS component (which will do the
+            // security check)
+            //
+            ntStatus = STATUS_ACCESS_DENIED;
+        }
     }
 
     return ntStatus;

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSDevControl.cpp b/src/WINNT/afsrdr/kernel/lib/AFSDevControl.cpp
index 276ca66..d92afaf 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSDevControl.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSDevControl.cpp
@@ -74,6 +74,14 @@ AFSDevControl( IN PDEVICE_OBJECT LibDeviceObject,
 
                 AFSLibraryInitCB *pLibInitCB = (AFSLibraryInitCB *)Irp->AssociatedIrp.SystemBuffer;
 
+                if ( Irp->RequestorMode != KernelMode)
+                {
+
+                    ntStatus = STATUS_ACCESS_DENIED;
+
+                    break;
+                }
+
                 if( pIrpSp->Parameters.DeviceIoControl.InputBufferLength < sizeof( AFSLibraryInitCB))
                 {
 
@@ -417,6 +425,11 @@ AFSDevControl( IN PDEVICE_OBJECT LibDeviceObject,
 
          default:
             {
+                //
+                // Note that this code path is never executed - default behavior is caught in the
+                // security checks in lib.  New Ioctl functions therefore have to be added here and
+                // in ..\fs\AFSCommSupport.cpp
+                //
 
                 ntStatus = STATUS_NOT_IMPLEMENTED;