Merge pull request #419 from GoSecure/file-io-fixes

Improvements to DeviceRedirection handling

pyrdp/core/event.py

@@ -1,12 +1,12 @@
 #
 # This file is part of the PyRDP project.
-# Copyright (C) 2019 GoSecure Inc.
+# Copyright (C) 2019, 2022 GoSecure Inc.
 # Licensed under the GPLv3 or later.
 #
 
 import asyncio
 import operator
-from typing import Callable, Dict
+from typing import Callable, Dict, List
 
 
 class Event:
@@ -118,7 +118,7 @@ def __init__(self):
         """
         Create a new (empty) event engine.
         """
-        self.events: [Event] = []
+        self.events: List[Event] = []
 
     def processObject(self, obj):
         """

pyrdp/enum/__init__.py

@@ -1,6 +1,6 @@
 #
 # This file is part of the PyRDP project.
-# Copyright (C) 2018-2021 GoSecure Inc.
+# Copyright (C) 2018-2022 GoSecure Inc.
 # Licensed under the GPLv3 or later.
 #
 # flake8: noqa
@@ -20,6 +20,7 @@
 from pyrdp.enum.virtual_channel.device_redirection import CreateOption, DeviceRedirectionComponent, \
     DeviceRedirectionPacketID, DeviceType, DirectoryAccessMask, FileAccessMask, FileAttributes, \
     FileCreateDisposition, FileCreateOptions, FileShareAccess, FileSystemInformationClass, GeneralCapabilityVersion, \
-    IOOperationSeverity, MajorFunction, MinorFunction, RDPDRCapabilityType
+    MajorFunction, MinorFunction, RDPDRCapabilityType
 from pyrdp.enum.virtual_channel.virtual_channel import VirtualChannelPDUFlag
+from pyrdp.enum.windows import NTSTATUS, NtStatusSeverity
 from pyrdp.enum.x224 import X224PDUType

pyrdp/enum/virtual_channel/device_redirection.py

@@ -1,6 +1,6 @@
 #
 # This file is part of the PyRDP project.
-# Copyright (C) 2018 GoSecure Inc.
+# Copyright (C) 2018, 2022 GoSecure Inc.
 # Licensed under the GPLv3 or later.
 #
 
@@ -60,16 +60,6 @@ class MinorFunction(IntEnum):
     IRP_MN_NOTIFY_CHANGE_DIRECTORY = 0x00000002
 
 
-class IOOperationSeverity(IntEnum):
-    """
-    https://msdn.microsoft.com/en-us/library/cc231200.aspx
-    """
-    STATUS_SEVERITY_SUCCESS = 0x0
-    STATUS_SEVERITY_INFORMATIONAL = 0x1
-    STATUS_SEVERITY_WARNING = 0x2
-    STATUS_SEVERITY_ERROR = 0x3
-
-
 class CreateOption(IntEnum):
     FILE_DIRECTORY_FILE = 0x00000001
     FILE_WRITE_THROUGH = 0x00000002

pyrdp/enum/windows.py

@@ -0,0 +1,33 @@
+#
+# This file is part of the PyRDP project.
+# Copyright (C) 2022 GoSecure Inc.
+# Licensed under the GPLv3 or later.
+#
+
+# Disable line-too-long lints.
+# flake8: noqa
+
+from enum import IntEnum
+
+
+class NtStatusSeverity(IntEnum):
+    """
+    [MS-ERREF]: Windows Error Codes
+    https://msdn.microsoft.com/en-us/library/cc231199.aspx
+    [MS-ERREF]: NTSTATUS
+    https://msdn.microsoft.com/en-us/library/cc231200.aspx
+    """
+    STATUS_SEVERITY_SUCCESS = 0x0
+    STATUS_SEVERITY_INFORMATIONAL = 0x1
+    STATUS_SEVERITY_WARNING = 0x2
+    STATUS_SEVERITY_ERROR = 0x3
+
+
+class NTSTATUS(IntEnum):
+    """
+    [MS-ERREF]: Windows Error Codes
+    https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/87fba13e-bf06-450e-83b1-9241dc81e781
+    """
+    STATUS_SUCCESS = 0x00000000
+    STATUS_NO_MORE_FILES = 0x80000006
+    STATUS_NO_SUCH_FILE = 0xC000000F

pyrdp/mitm/DeviceRedirectionMITM.py

@@ -1,6 +1,6 @@
 #
 # This file is part of the PyRDP project.
-# Copyright (C) 2019-2021 GoSecure Inc.
+# Copyright (C) 2019-2022 GoSecure Inc.
 # Licensed under the GPLv3 or later.
 #
 
@@ -10,8 +10,8 @@
 from pyrdp.core import ObservedBy, Observer, Subject
 from pyrdp.enum import CreateOption, DeviceRedirectionPacketID, DeviceType, DirectoryAccessMask, FileAccessMask, \
     FileAttributes, \
-    FileCreateDisposition, FileCreateOptions, FileShareAccess, FileSystemInformationClass, IOOperationSeverity, \
-    MajorFunction, MinorFunction
+    FileCreateDisposition, FileCreateOptions, FileShareAccess, FileSystemInformationClass, \
+    MajorFunction, MinorFunction, NTSTATUS, NtStatusSeverity
 from pyrdp.layer import DeviceRedirectionLayer
 from pyrdp.logging.StatCounter import StatCounter, STAT
 from pyrdp.mitm.FileMapping import FileMapping
@@ -177,12 +177,23 @@ def handleIOResponse(self, pdu: DeviceIOResponsePDU):
         elif key in self.currentIORequests:
             requestPDU = self.currentIORequests.pop(key)
 
-            if pdu.ioStatus >> 30 == IOOperationSeverity.STATUS_SEVERITY_ERROR:
+            if pdu.ioStatus >> 30 == NtStatusSeverity.STATUS_SEVERITY_ERROR:
                 self.statCounter.increment(STAT.DEVICE_REDIRECTION_IOERROR)
-                self.log.warning("Received an IO Response with an error IO status: %(responsePDU)s for request %(requestPDU)s", {"responsePDU": repr(pdu), "requestPDU": repr(requestPDU)})
+                # log only for unexpected errors since "no such files" and "no more files" are frequent:
+                # "no such files" for all attempts at fetching .desktop.ini
+                # "no more files" when directory listings are finished
+                if pdu.ioStatus not in [NTSTATUS.STATUS_NO_SUCH_FILE,
+                                        NTSTATUS.STATUS_NO_MORE_FILES]:
+                    self.log.warning("Received an IO Response with an error IO status: "
+                                     "%(responsePDU)s for request %(requestPDU)s",
+                                     {"responsePDU": repr(pdu), "requestPDU": repr(requestPDU)})
 
             if pdu.majorFunction in self.responseHandlers:
                 self.responseHandlers[pdu.majorFunction](requestPDU, pdu)
+            # Good place to debug
+            #else:
+            #    self.log.warning("No handler was triggered for this IO request: %(requestPDU)s. Response: %(responsePDU)s", {"responsePDU": repr(pdu), "requestPDU": repr(requestPDU)})
+
         else:
             self.log.error("Received IO response to unknown request #%(completionID)d", {"completionID": pdu.completionID})
 
@@ -214,8 +225,9 @@ def handleCreateResponse(self, request: DeviceCreateRequestPDU, response: Device
 
         isFileRead = request.desiredAccess & (FileAccessMask.GENERIC_READ | FileAccessMask.FILE_READ_DATA) != 0
         isDirectory = request.createOptions & CreateOption.FILE_NON_DIRECTORY_FILE == 0
+        fileExists = (response.ioStatus != NTSTATUS.STATUS_NO_SUCH_FILE)
 
-        if isFileRead and not isDirectory:
+        if fileExists and isFileRead and not isDirectory:
             mapping = FileMapping.generate(request.path, self.config.fileDir, self.deviceRoot(response.deviceID), self.log)
             key = (response.deviceID, response.fileID)
             self.mappings[key] = mapping

pyrdp/parser/rdp/virtual_channel/device_redirection.py

@@ -11,7 +11,8 @@
 from pyrdp.enum import DeviceRedirectionComponent, DeviceRedirectionPacketID, \
     DeviceType, FileAccessMask, FileAttributes, FileCreateDisposition, \
     FileCreateOptions, FileShareAccess, FileSystemInformationClass, \
-    GeneralCapabilityVersion, MajorFunction, MinorFunction, RDPDRCapabilityType
+    GeneralCapabilityVersion, MajorFunction, MinorFunction, \
+    RDPDRCapabilityType, NTSTATUS
 from pyrdp.parser import Parser
 from pyrdp.pdu import DeviceAnnounce, DeviceCloseRequestPDU, DeviceCloseResponsePDU, DeviceCreateRequestPDU, \
     DeviceCreateResponsePDU, DeviceDirectoryControlResponsePDU, DeviceIORequestPDU, DeviceIOResponsePDU, \
@@ -296,7 +297,7 @@ def writeDeviceIORequest(self, pdu: DeviceIORequestPDU, stream: BytesIO):
     def parseDeviceIOResponse(self, stream: BytesIO) -> DeviceIOResponsePDU:
         deviceID = Uint32LE.unpack(stream)
         completionID = Uint32LE.unpack(stream)
-        ioStatus = Uint32LE.unpack(stream)
+        ioStatus = NTSTATUS(Uint32LE.unpack(stream))
 
         majorFunction = self.majorFunctionsForParsingResponse.pop(completionID, None)
 
@@ -361,7 +362,7 @@ def writeDeviceCreateRequest(self, pdu: DeviceCreateRequestPDU, stream: BytesIO)
         stream.write(path)
 
 
-    def parseDeviceCreateResponse(self, deviceID: int, completionID: int, ioStatus: int, stream: BytesIO) -> DeviceCreateResponsePDU:
+    def parseDeviceCreateResponse(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, stream: BytesIO) -> DeviceCreateResponsePDU:
         """
         The information field is not yet parsed (it's optional).
         This one is a bit special since we need to look at previous packet before parsing it as
@@ -396,7 +397,7 @@ def writeDeviceReadRequest(self, pdu: DeviceReadRequestPDU, stream: BytesIO):
         stream.write(b"\x00" * 20)  # Padding
 
 
-    def parseDeviceReadResponse(self, deviceID: int, completionID: int, ioStatus: int, stream: BytesIO) -> DeviceReadResponsePDU:
+    def parseDeviceReadResponse(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, stream: BytesIO) -> DeviceReadResponsePDU:
         length = Uint32LE.unpack(stream)
         payload = stream.read(length)
 
@@ -415,7 +416,7 @@ def writeDeviceCloseRequest(self, _: DeviceCloseRequestPDU, stream: BytesIO):
         stream.write(b"\x00" * 32)  # Padding
 
 
-    def parseDeviceCloseResponse(self, deviceID: int, completionID: int, ioStatus: int, stream: BytesIO) -> DeviceCloseResponsePDU:
+    def parseDeviceCloseResponse(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, stream: BytesIO) -> DeviceCloseResponsePDU:
         stream.read(4)  # Padding
         return DeviceCloseResponsePDU(deviceID, completionID, ioStatus)
 
@@ -461,7 +462,7 @@ def writeDirectoryControlRequest(self, pdu: Union[DeviceIORequestPDU, DeviceQuer
                 stream.write(b"\x00" * 23) # protocol required padding
                 stream.write(path)
 
-    def parseDirectoryControlResponse(self, deviceID: int, completionID: int, ioStatus: int, stream: BytesIO) -> DeviceIOResponsePDU:
+    def parseDirectoryControlResponse(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, stream: BytesIO) -> DeviceIOResponsePDU:
         minorFunction = self.minorFunctionsForParsingResponse.pop(completionID, None)
 
         if minorFunction is None:

pyrdp/pdu/rdp/virtual_channel/device_redirection.py

@@ -9,7 +9,7 @@
 from pyrdp.enum import DeviceRedirectionComponent, DeviceRedirectionPacketID, \
     DeviceType, FileAccessMask, FileAttributes, FileCreateDisposition, \
     FileCreateOptions, FileShareAccess, FileSystemInformationClass, \
-    MajorFunction, MinorFunction, RDPDRCapabilityType
+    MajorFunction, MinorFunction, RDPDRCapabilityType, NTSTATUS
 from pyrdp.pdu import PDU
 
 
@@ -43,7 +43,7 @@ class DeviceIOResponsePDU(DeviceRedirectionPDU):
     https://msdn.microsoft.com/en-us/library/cc241334.aspx
     """
 
-    def __init__(self, majorFunction: Optional[MajorFunction], deviceID: int, completionID: int, ioStatus: int, payload=b""):
+    def __init__(self, majorFunction: Optional[MajorFunction], deviceID: int, completionID: int, ioStatus: NTSTATUS, payload=b""):
         super().__init__(DeviceRedirectionComponent.RDPDR_CTYP_CORE, DeviceRedirectionPacketID.PAKID_CORE_DEVICE_IOCOMPLETION)
         self.majorFunction = majorFunction
         self.deviceID = deviceID
@@ -75,7 +75,7 @@ class DeviceCreateResponsePDU(DeviceIOResponsePDU):
     https://msdn.microsoft.com/en-us/library/cc241335.aspx
     """
 
-    def __init__(self, deviceID: int, completionID: int, ioStatus: int, fileID: int, information: int):
+    def __init__(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, fileID: int, information: int):
         super().__init__(MajorFunction.IRP_MJ_CREATE, deviceID, completionID, ioStatus)
         self.fileID = fileID
         self.information = information
@@ -98,7 +98,7 @@ class DeviceReadResponsePDU(DeviceIOResponsePDU):
     https://msdn.microsoft.com/en-us/library/cc241337.aspx
     """
 
-    def __init__(self, deviceID: int, completionID: int, ioStatus: int, readData: bytes):
+    def __init__(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, readData: bytes):
         super().__init__(MajorFunction.IRP_MJ_READ, deviceID, completionID, ioStatus, readData)
 
 
@@ -116,7 +116,7 @@ class DeviceCloseResponsePDU(DeviceIOResponsePDU):
     https://msdn.microsoft.com/en-us/library/cc241336.aspx
     """
 
-    def __init__(self, deviceID: int, completionID: int, ioStatus: int):
+    def __init__(self, deviceID: int, completionID: int, ioStatus: NTSTATUS):
         super().__init__(MajorFunction.IRP_MJ_CLOSE, deviceID, completionID, ioStatus)
 
 
@@ -184,13 +184,13 @@ def __init__(self, deviceID: int, fileID: int, completionID: int, informationCla
 
 
 class DeviceDirectoryControlResponsePDU(DeviceIOResponsePDU):
-    def __init__(self, minorFunction: MinorFunction, deviceID: int, completionID: int, ioStatus: int, payload: bytes = b""):
+    def __init__(self, minorFunction: MinorFunction, deviceID: int, completionID: int, ioStatus: NTSTATUS, payload: bytes = b""):
         super().__init__(MajorFunction.IRP_MJ_DIRECTORY_CONTROL, deviceID, completionID, ioStatus, payload)
         self.minorFunction = minorFunction
 
 
 class DeviceQueryDirectoryResponsePDU(DeviceDirectoryControlResponsePDU):
-    def __init__(self, deviceID: int, completionID: int, ioStatus: int, informationClass: FileSystemInformationClass, fileInformation: List[FileInformationBase], endByte: bytes):
+    def __init__(self, deviceID: int, completionID: int, ioStatus: NTSTATUS, informationClass: FileSystemInformationClass, fileInformation: List[FileInformationBase], endByte: bytes):
         super().__init__(MinorFunction.IRP_MN_QUERY_DIRECTORY, deviceID, completionID, ioStatus)
         self.informationClass = informationClass
         self.fileInformation = fileInformation

test/test_DeviceRedirectionMITM.py

@@ -1,22 +1,22 @@
 #
 # This file is part of the PyRDP project.
-# Copyright (C) 2020-2021 GoSecure Inc.
+# Copyright (C) 2020-2022 GoSecure Inc.
 # Licensed under the GPLv3 or later.
 #
 
 import unittest
 from pathlib import Path
 from unittest.mock import Mock, MagicMock, patch
 
-from pyrdp.enum import CreateOption, FileAccessMask, IOOperationSeverity, DeviceRedirectionPacketID, MajorFunction, \
-    MinorFunction
+from pyrdp.enum import CreateOption, FileAccessMask, DeviceRedirectionPacketID, MajorFunction, \
+    MinorFunction, NtStatusSeverity
 from pyrdp.logging.StatCounter import StatCounter, STAT
 from pyrdp.mitm.DeviceRedirectionMITM import DeviceRedirectionMITM
 from pyrdp.pdu import DeviceIOResponsePDU, DeviceRedirectionPDU
 
 
 def MockIOError():
-    ioError = Mock(deviceID = 0, completionID = 0, ioStatus = IOOperationSeverity.STATUS_SEVERITY_ERROR << 30)
+    ioError = Mock(deviceID = 0, completionID = 0, ioStatus = NtStatusSeverity.STATUS_SEVERITY_ERROR << 30)
     return ioError