From 6039b53f33be0a63aa96e7988e793b19af736a8c Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 10 Nov 2022 14:51:09 +1100 Subject: [PATCH 01/20] Check Z-Order for making content accessible on the lock screen. --- source/api.py | 5 +- source/baseObject.py | 30 ++- source/core.py | 22 +-- source/globalVars.py | 3 +- source/nvda.pyw | 12 +- source/textInfos/__init__.py | 16 +- source/treeInterceptorHandler.py | 18 +- source/utils/security.py | 45 ++++- source/winAPI/messageWindow.py | 2 - source/winAPI/sessionTracking.py | 324 ++++++++----------------------- source/winUser.py | 2 +- 11 files changed, 185 insertions(+), 294 deletions(-) diff --git a/source/api.py b/source/api.py index ad820d8b2f6..36a558f133c 100644 --- a/source/api.py +++ b/source/api.py @@ -237,7 +237,10 @@ def setReviewPosition( @param isCaret: Whether the review position is changed due to caret following. @param isMouse: Whether the review position is changed due to mouse following. """ - if _isSecureObjectWhileLockScreenActivated(reviewPosition.obj): + reviewObj = reviewPosition.obj + if isinstance(reviewObj, treeInterceptorHandler.TreeInterceptor): + reviewObj = reviewObj.rootNVDAObject + if _isSecureObjectWhileLockScreenActivated(reviewObj): return False globalVars.reviewPosition=reviewPosition.copy() globalVars.reviewPositionObj=reviewPosition.obj diff --git a/source/baseObject.py b/source/baseObject.py index 988bfdab883..a63fa57cb86 100755 --- a/source/baseObject.py +++ b/source/baseObject.py @@ -1,16 +1,27 @@ # A part of NonVisual Desktop Access (NVDA) -# Copyright (C) 2007-2020 NV Access Limited, Christopher Toth, Babbage B.V., Julien Cochuyt +# Copyright (C) 2007-2022 NV Access Limited, Christopher Toth, Babbage B.V., Julien Cochuyt # This file is covered by the GNU General Public License. # See the file COPYING for more details. """Contains the base classes that many of NVDA's classes such as NVDAObjects, virtualBuffers, appModules, synthDrivers inherit from. These base classes provide such things as auto properties, and methods and properties for scripting and key binding. """ +from typing import ( + Any, + Callable, + Optional, + Set, + Union, +) import weakref import garbageHandler from logHandler import log from abc import ABCMeta, abstractproperty +GetterReturnT = Any +GetterMethodT = Callable[["AutoPropertyObject"], GetterReturnT] + + class Getter(object): def __init__(self,fget, abstract=False): @@ -18,7 +29,11 @@ def __init__(self,fget, abstract=False): if abstract: self._abstract = self.__isabstractmethod__ = abstract - def __get__(self,instance,owner): + def __get__( + self, + instance: Union[Any, None, "AutoPropertyObject"], + owner, + ) -> Union[GetterReturnT, "Getter"]: if isinstance(self.fget, classmethod): return self.fget.__get__(instance, owner)() elif instance is None: @@ -31,9 +46,14 @@ def setter(self,func): def deleter(self,func): return (abstractproperty if self._abstract else property)(fget=self.fget,fdel=func) + class CachingGetter(Getter): - def __get__(self, instance, owner): + def __get__( + self, + instance: Union[Any, None, "AutoPropertyObject"], + owner, + ) -> Union[GetterReturnT, "CachingGetter"]: if isinstance(self.fget, classmethod): log.warning("Class properties do not support caching") return self.fget.__get__(instance, owner)() @@ -41,6 +61,7 @@ def __get__(self, instance, owner): return self return instance._getPropertyViaCache(self.fget) + class AutoPropertyType(ABCMeta): def __init__(self,name,bases,dict): @@ -125,6 +146,7 @@ class AutoPropertyObject(garbageHandler.TrackedObject, metaclass=AutoPropertyTyp #: @type: bool cachePropertiesByDefault = False + _propertyCache: Set[GetterMethodT] def __new__(cls, *args, **kwargs): self = super(AutoPropertyObject, cls).__new__(cls) @@ -134,7 +156,7 @@ def __new__(cls, *args, **kwargs): self.__instances[self]=None return self - def _getPropertyViaCache(self,getterMethod=None): + def _getPropertyViaCache(self, getterMethod: Optional[GetterMethodT] = None) -> GetterReturnT: if not getterMethod: raise ValueError("getterMethod is None") missing=False diff --git a/source/core.py b/source/core.py index 014fb62936c..6ca1d8433a3 100644 --- a/source/core.py +++ b/source/core.py @@ -601,7 +601,6 @@ def onEndSession(evt): wx.CallAfter(audioDucking.initialize) from winAPI.messageWindow import WindowMessage - from winAPI import sessionTracking import winUser # #3763: In wxPython 3, the class name of frame windows changed from wxWindowClassNR to wxWindowNR. # NVDA uses the main frame to check for and quit another instance of NVDA. @@ -630,27 +629,12 @@ def __init__(self, windowName=None): self.orientationCoordsCache = (0,0) self.handlePowerStatusChange() - # Call must be paired with a call to sessionTracking.unregister - if not sessionTracking.register(self.handle): - import utils.security - wx.CallAfter(utils.security.warnSessionLockStateUnknown) - - def destroy(self): - """ - NVDA must unregister session tracking before destroying the message window. - """ - # Requires an active message window and a handle to unregister. - sessionTracking.unregister(self.handle) - super().destroy() - def windowProc(self, hwnd, msg, wParam, lParam): post_windowMessageReceipt.notify(msg=msg, wParam=wParam, lParam=lParam) if msg == WindowMessage.POWER_BROADCAST and wParam == self.PBT_APMPOWERSTATUSCHANGE: self.handlePowerStatusChange() elif msg == winUser.WM_DISPLAYCHANGE: self.handleScreenOrientationChange(lParam) - elif msg == WindowMessage.WTS_SESSION_CHANGE: - sessionTracking.handleSessionChange(sessionTracking.WindowsTrackedSession(wParam), lParam) def handleScreenOrientationChange(self, lParam): # TODO: move to winAPI @@ -809,7 +793,8 @@ def run(self): mouseHandler.pumpAll() braille.pumpAll() vision.pumpAll() - except: + sessionTracking.pumpAll() + except Exception: log.exception("errors in this core pump cycle") baseObject.AutoPropertyObject.invalidateCaches() watchdog.asleep() @@ -832,6 +817,9 @@ def run(self): log.debug("initializing updateCheck") updateCheck.initialize() + from winAPI import sessionTracking + sessionTracking.initialize() + _TrackNVDAInitialization.markInitializationComplete() log.info("NVDA initialized") diff --git a/source/globalVars.py b/source/globalVars.py index 83b02905e82..ed6c7f43cfa 100644 --- a/source/globalVars.py +++ b/source/globalVars.py @@ -21,6 +21,7 @@ if typing.TYPE_CHECKING: import NVDAObjects # noqa: F401 used for type checking only + import textInfos # noqa: F401 used for type checking only class DefaultAppArgs(argparse.Namespace): @@ -70,7 +71,7 @@ class DefaultAppArgs(argparse.Namespace): mouseOldY=None navigatorObject: typing.Optional['NVDAObjects.NVDAObject'] = None reviewPosition=None -reviewPositionObj=None +reviewPositionObj: typing.Optional["textInfos.TextInfoObjT"] = None lastProgressValue=0 appArgs = DefaultAppArgs() unknownAppArgs: typing.List[str] = [] diff --git a/source/nvda.pyw b/source/nvda.pyw index abe08bccdb2..9d1cddb10f6 100755 --- a/source/nvda.pyw +++ b/source/nvda.pyw @@ -350,13 +350,19 @@ if mutex is None: sys.exit(1) -if _isSecureDesktop(): +def _serviceDebugEnabled() -> bool: import winreg try: k = winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, r"SOFTWARE\NVDA") - if not winreg.QueryValueEx(k, u"serviceDebug")[0]: - globalVars.appArgs.secure = True + if winreg.QueryValueEx(k, "serviceDebug")[0]: + return True except WindowsError: + pass + return False + + +if _isSecureDesktop(): + if not _serviceDebugEnabled(): globalVars.appArgs.secure = True globalVars.appArgs.changeScreenReaderFlag = False globalVars.appArgs.minimal = True diff --git a/source/textInfos/__init__.py b/source/textInfos/__init__.py index 544c72380d2..c2ee09fa944 100755 --- a/source/textInfos/__init__.py +++ b/source/textInfos/__init__.py @@ -1,7 +1,7 @@ # A part of NonVisual Desktop Access (NVDA) # This file is covered by the GNU General Public License. # See the file COPYING for more details. -# Copyright (C) 2006-2021 NV Access Limited, Babbage B.V., Accessolutions, Julien Cochuyt +# Copyright (C) 2006-2022 NV Access Limited, Babbage B.V., Accessolutions, Julien Cochuyt """Framework for accessing text content in widgets. The core component of this framework is the L{TextInfo} class. @@ -32,6 +32,7 @@ if typing.TYPE_CHECKING: import NVDAObjects + import treeInterceptorHandler # noqa: F401 used for type checking only SpeechSequence = List[Union[Any, str]] @@ -287,6 +288,9 @@ def _logBadSequenceTypes(sequence: SpeechSequence, shouldRaise: bool = True): return speech.types.logBadSequenceTypes(sequence, raiseExceptionOnError=shouldRaise) +TextInfoObjT = Union["NVDAObjects.NVDAObject", "treeInterceptorHandler.TreeInterceptor"] + + class TextInfo(baseObject.AutoPropertyObject): """Provides information about a range of text in an object and facilitates access to all text in the widget. A TextInfo represents a specific range of text, providing access to the text itself, as well as information about the text such as its formatting and any associated controls. @@ -307,7 +311,11 @@ class TextInfo(baseObject.AutoPropertyObject): @type bookmark: L{Bookmark} """ - def __init__(self,obj,position): + def __init__( + self, + obj: TextInfoObjT, + position + ): """Constructor. Subclasses must extend this, calling the superclass method first. @param position: The initial position of this range; one of the POSITION_* constants or a position object supported by the implementation. @@ -338,9 +346,9 @@ def _set_end(self, otherEndpoint: "TextInfoEndpoint"): self.end.moveTo(otherEndpoint) #: Typing information for auto-property: _get_obj - obj: "NVDAObjects.NVDAObject" + obj: TextInfoObjT - def _get_obj(self) -> "NVDAObjects.NVDAObject": + def _get_obj(self) -> TextInfoObjT: """The object containing the range of text being represented.""" return self._obj() diff --git a/source/treeInterceptorHandler.py b/source/treeInterceptorHandler.py index 179a40b43ce..6632de8de95 100644 --- a/source/treeInterceptorHandler.py +++ b/source/treeInterceptorHandler.py @@ -1,10 +1,13 @@ -# treeInterceptorHandler.py # A part of NonVisual Desktop Access (NVDA) -# Copyright (C) 2006-2020 NV Access Limited, Davy Kager, Accessolutions, Julien Cochuyt +# Copyright (C) 2006-2022 NV Access Limited, Davy Kager, Accessolutions, Julien Cochuyt # This file is covered by the GNU General Public License. # See the file COPYING for more details. -from typing import Optional, Dict +from typing import ( + TYPE_CHECKING, + Dict, + Optional, +) from logHandler import log import baseObject @@ -18,6 +21,10 @@ from speech.types import SpeechSequence from controlTypes import OutputReason +if TYPE_CHECKING: + import NVDAObjects + + runningTable=set() def getTreeInterceptor(obj): @@ -84,12 +91,11 @@ class TreeInterceptor(baseObject.ScriptableObject): shouldTrapNonCommandGestures=False #: If true then gestures that do not have a script and are not a command gesture should be trapped from going through to Windows. - def __init__(self, rootNVDAObject): + def __init__(self, rootNVDAObject: "NVDAObjects.NVDAObject"): super(TreeInterceptor, self).__init__() self._passThrough = False #: The root object of the tree wherein events and scripts are intercepted. - #: @type: L{NVDAObjects.NVDAObject} - self.rootNVDAObject = rootNVDAObject + self.rootNVDAObject: "NVDAObjects.NVDAObject" = rootNVDAObject def terminate(self): """Terminate this interceptor. diff --git a/source/utils/security.py b/source/utils/security.py index 09fea1545db..b79a26ea6bd 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -14,13 +14,14 @@ import winUser if typing.TYPE_CHECKING: - import appModuleHandler + import appModuleHandler # noqa: F401, use for typing import scriptHandler # noqa: F401, use for typing - import NVDAObjects + import NVDAObjects # noqa: F401, use for typing postSessionLockStateChanged = extensionPoints.Action() """ +# TODO: maintain backwards compat Notifies when a session lock or unlock event occurs. Usage: @@ -141,7 +142,13 @@ def _isSecureObjectWhileLockScreenActivated( As such, NVDA must prevent accessing and reading objects outside of the lockscreen when Windows is locked. @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is outside of the lock screen. """ - if isWindowsLocked() and not isObjectAboveLockScreen(obj): + try: + isObjectInSecure = isWindowsLocked() and not isObjectAboveLockScreen(obj) + except Exception: + log.exception() + return False + + if isObjectInSecure: if shouldLog and log.isEnabledFor(log.DEBUG): devInfo = '\n'.join(obj.devInfo) log.debug(f"Attempt at navigating to a secure object: {devInfo}") @@ -155,7 +162,6 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: When Windows is locked, the foreground Window is usually LockApp, but other Windows can be focused (e.g. Windows Magnifier). """ - import appModuleHandler from IAccessibleHandler import SecureDesktopNVDAObject from NVDAObjects.IAccessible import TaskListIcon @@ -176,19 +182,44 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: or isinstance(obj, SecureDesktopNVDAObject) ): return True + return _isObjectAboveLockScreenCheckZOrder(obj) + +def _isObjectAboveLockScreenCheckZOrder(obj: "NVDAObjects.NVDAObject") -> bool: + """ + This is a risky hack. + If the order is incorrectly detected, + the Windows UX may become inaccessible + or secure information may become accessible. + + If these functions fail, where possible, + NVDA should make NVDA objects accessible. + """ + import appModuleHandler + from NVDAObjects.window import Window + if not isinstance(obj, Window): + # must be a window to get its HWNDVal + return True runningAppModules = appModuleHandler.runningTable.values() lockAppModule = next(filter(_isLockAppAndAlive, runningAppModules), None) if lockAppModule is None: # lockAppModule not running/registered by NVDA yet - log.debugWarning( + log.debug( "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is in lock app, considering object as insecure. " + "Cannot detect if object is in lock app, considering object as safe. " ) - elif lockAppModule is not None and obj.processID == lockAppModule.processID: return True + desktopWindow = winUser.getDesktopWindow() + nextWindow = winUser.getTopWindow(desktopWindow) + while nextWindow: + windowProcessId = winUser.getWindowThreadProcessID(nextWindow) + if nextWindow == obj.windowHandle: + return True + elif windowProcessId == lockAppModule.processID: + return False + nextWindow = winUser.getWindow(nextWindow, winUser.GW_HWNDNEXT) return False diff --git a/source/winAPI/messageWindow.py b/source/winAPI/messageWindow.py index 4f7ee91dba1..34e8a830dc2 100644 --- a/source/winAPI/messageWindow.py +++ b/source/winAPI/messageWindow.py @@ -21,6 +21,4 @@ class WindowMessage(enum.IntEnum): """ WM_WTSSESSION_CHANGE Windows Message for when a Session State Changes. - Receiving these messages is registered by sessionTracking.register. - handleSessionChange handles these messages. """ diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 079f1ed15e0..7c63dc219ed 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -12,8 +12,6 @@ Used to: - only allow a whitelist of safe scripts to run - ensure object navigation cannot occur outside of the lockscreen - -https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification """ from __future__ import annotations @@ -21,18 +19,14 @@ import ctypes from contextlib import contextmanager from ctypes.wintypes import ( - HANDLE, HWND, ) import enum -from threading import Lock from typing import ( - Dict, - Set, Optional, ) -from systemUtils import _isSecureDesktop +from baseObject import AutoPropertyObject from winAPI.wtsApi32 import ( WTSINFOEXW, WTSQuerySessionInformation, @@ -45,44 +39,34 @@ ) from logHandler import log -_updateSessionStateLock = Lock() -"""Used to protect updates to _currentSessionStates""" -_currentSessionStates: Set["WindowsTrackedSession"] = set() -""" -Current state of the Windows session associated with this instance of NVDA. -Maintained via receiving session notifications via the NVDA MessageWindow. -Initial state will be set by querying the current status. -Actions which involve updating this state this should be protected by _updateSessionStateLock. -""" - -_sessionQueryLockStateHasBeenUnknown = False -""" -Track if any 'Unknown' Value when querying the Session Lock status has been encountered. -""" - -_isSessionTrackingRegistered = False -""" -Session tracking is required for NVDA to be notified of lock state changes for security purposes. -""" - RPC_S_INVALID_BINDING = 0x6A6 """ Error which occurs when Windows is not ready to register session notification tracking. This error can be prevented by waiting for the event: 'Global\\TermSrvReadyEvent.' + +Unused in NVDA core. """ NOTIFY_FOR_THIS_SESSION = 0 """ The alternative to NOTIFY_FOR_THIS_SESSION is to be notified for all user sessions. NOTIFY_FOR_ALL_SESSIONS is not required as NVDA runs on separate user profiles, including the system profile. + +Unused in NVDA core. """ SYNCHRONIZE = 0x00100000 """ Parameter for OpenEventW, blocks thread until event is registered. https://docs.microsoft.com/en-us/windows/win32/sync/synchronization-object-security-and-access-rights + +Unused in NVDA core, duplicate of winKernel.SYNCHRONIZE. """ +_initializationLockState: Optional[bool] = None +_lockStateTracker: Optional["_WindowsLockedState"] = None +_windowsWasPreviouslyLocked = False + class WindowsTrackedSession(enum.IntEnum): """ @@ -106,47 +90,30 @@ class WindowsTrackedSession(enum.IntEnum): SESSION_TERMINATE = 11 -_toggleWindowsSessionStatePair: Dict[WindowsTrackedSession, WindowsTrackedSession] = { - WindowsTrackedSession.CONSOLE_CONNECT: WindowsTrackedSession.CONSOLE_DISCONNECT, - WindowsTrackedSession.CONSOLE_DISCONNECT: WindowsTrackedSession.CONSOLE_CONNECT, - WindowsTrackedSession.REMOTE_CONNECT: WindowsTrackedSession.REMOTE_DISCONNECT, - WindowsTrackedSession.REMOTE_DISCONNECT: WindowsTrackedSession.REMOTE_CONNECT, - WindowsTrackedSession.SESSION_LOGON: WindowsTrackedSession.SESSION_LOGOFF, - WindowsTrackedSession.SESSION_LOGOFF: WindowsTrackedSession.SESSION_LOGON, - WindowsTrackedSession.SESSION_LOCK: WindowsTrackedSession.SESSION_UNLOCK, - WindowsTrackedSession.SESSION_UNLOCK: WindowsTrackedSession.SESSION_LOCK, - WindowsTrackedSession.SESSION_CREATE: WindowsTrackedSession.SESSION_TERMINATE, - WindowsTrackedSession.SESSION_TERMINATE: WindowsTrackedSession.SESSION_CREATE, -} -""" -Pair of WindowsTrackedSession, where each key has a value of the opposite state. -e.g. SESSION_LOCK/SESSION_UNLOCK. -""" +class _WindowsLockedState(AutoPropertyObject): + # Refer to AutoPropertyObject for notes on caching + _cache_isWindowsLocked = True + + # Typing information for auto-property _get_isWindowsLocked + isWindowsLocked: bool + def _get_isWindowsLocked(self) -> bool: + from winAPI.sessionTracking import _isWindowsLocked_checkViaSessionQuery + return _isWindowsLocked_checkViaSessionQuery() -def _hasLockStateBeenTracked() -> bool: - """ - Checks if NVDA is aware of a session lock state change since NVDA started. - """ - return bool(_currentSessionStates.intersection({ - WindowsTrackedSession.SESSION_LOCK, - WindowsTrackedSession.SESSION_UNLOCK - })) +def initialize(): + global _lockStateTracker + _lockStateTracker = _WindowsLockedState() -def _recordLockStateTrackingFailure(error: Optional[Exception] = None): - log.error( - "Unknown lock state, unexpected, potential security issue, please report.", - exc_info=error - ) # Report error repeatedly, attention is required. - ## - # For security it would be best to treat unknown as locked. - # However, this would make NVDA unusable. - # Instead, the user should be warned, and allowed to mitigate the impact themselves. - # Reporting is achieved via _sessionQueryLockStateHasBeenUnknown exposed with - # L{hasLockStateBeenUnknown}. - global _sessionQueryLockStateHasBeenUnknown - _sessionQueryLockStateHasBeenUnknown = True + +def pumpAll(): + global _windowsWasPreviouslyLocked + from utils.security import postSessionLockStateChanged + windowsIsNowLocked = isWindowsLocked() + if windowsIsNowLocked != _windowsWasPreviouslyLocked: + _windowsWasPreviouslyLocked = windowsIsNowLocked + postSessionLockStateChanged.notify(isNowLocked=windowsIsNowLocked) def isWindowsLocked() -> bool: @@ -155,74 +122,37 @@ def isWindowsLocked() -> bool: Not to be confused with the Windows sign-in screen, a secure screen. """ from core import _TrackNVDAInitialization + from winAPI.sessionTracking import _isWindowsLocked_checkViaSessionQuery + from systemUtils import _isSecureDesktop if not _TrackNVDAInitialization.isInitializationComplete(): - # During NVDA initialization, - # core._initializeObjectCaches needs to cache the desktop object, - # regardless of lock state. - # Security checks may cause the desktop object to not be set if NVDA starts on the lock screen. - # As such, during initialization, NVDA should behave as if Windows is unlocked. return False if _isSecureDesktop(): - # If this is the Secure Desktop, - # we are in secure mode and on a secure screen, - # e.g. on the sign-in screen. - # _isSecureDesktop may also return True on the lock screen before a user has signed in. - - # For more information, refer to devDocs/technicalDesignOverview.md 'Logging in secure mode' - # and the following userGuide sections: - # - SystemWideParameters (information on the serviceDebug parameter) - # - SecureMode and SecureScreens return False - lockStateTracked = _hasLockStateBeenTracked() - if lockStateTracked: - return WindowsTrackedSession.SESSION_LOCK in _currentSessionStates + global _initializationLockState + if _lockStateTracker is not None: + _isWindowsLocked = _lockStateTracker.isWindowsLocked else: - _recordLockStateTrackingFailure() # Report error repeatedly, attention is required. - ## - # For security it would be best to treat unknown as locked. - # However, this would make NVDA unusable. - # Instead, the user should be warned via UI, and allowed to mitigate the impact themselves. - # See usage of L{hasLockStateBeenUnknown}. - return False # return False, indicating unlocked, to allow NVDA to be used - - -def _setInitialWindowLockState() -> None: - """ - Ensure that session tracking state is initialized. - If NVDA has started on a lockScreen, it needs to be aware of this. - As NVDA has already registered for session tracking notifications, - a lock is used to prevent conflicts. - """ - with _updateSessionStateLock: - lockStateTracked = _hasLockStateBeenTracked() - if lockStateTracked: - log.debugWarning( - "Initial state already set." - " NVDA may have received a session change notification before initialising" - ) - # Fall back to explicit query - try: - isLocked = _isWindowsLocked_checkViaSessionQuery() - _currentSessionStates.add( - WindowsTrackedSession.SESSION_LOCK - if isLocked - else WindowsTrackedSession.SESSION_UNLOCK - ) - except RuntimeError as error: - _recordLockStateTrackingFailure(error) + if _initializationLockState is None: + _initializationLockState = _isWindowsLocked_checkViaSessionQuery() + _isWindowsLocked = _initializationLockState + return _isWindowsLocked def _isWindowsLocked_checkViaSessionQuery() -> bool: """ Use a session query to check if the session is locked @return: True is the session is locked. - @raise: Runtime error if the lock state can not be determined via a Session Query. + Also returns False if the lock state can not be determined via a Session Query. """ - sessionQueryLockState = _getSessionLockedValue() + try: + sessionQueryLockState = _getSessionLockedValue() + except RuntimeError: + return False if sessionQueryLockState == WTS_LockState.WTS_SESSIONSTATE_UNKNOWN: - raise RuntimeError( + log.error( "Unable to determine lock state via Session Query." f" Lock state value: {sessionQueryLockState!r}" ) + return False return sessionQueryLockState == WTS_LockState.WTS_SESSIONSTATE_LOCK @@ -231,145 +161,41 @@ def isLockStateSuccessfullyTracked() -> bool: I.E. Registered for session tracking AND initial value set correctly. @return: True when successfully tracked. """ - return ( - not _sessionQueryLockStateHasBeenUnknown - or not _isSessionTrackingRegistered + # TODO: improve deprecation practice on beta/master merges + log.error( + "NVDA no longer registers session tracking notifications. " + "This function is deprecated, for removal in 2023.1. " + "It was never expected that add-on authors would use this function" ) + return True -def register(handle: HWND) -> bool: - """ - @param handle: handle for NVDA message window. - When registered, Windows Messages related to session event changes will be - sent to the message window. - @returns: True is session tracking is successfully registered. - - Blocks until Windows accepts session tracking registration. - - Every call to this function must be paired with a call to unregister. - - If registration fails, NVDA may not work properly until the session can be registered in a new instance. - NVDA will not know when the lock screen is activated, which means it becomes a security risk. - NVDA should warn the user if registering the session notification fails. - - https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification - """ - - # OpenEvent handle must be closed with CloseHandle. - eventObjectHandle: HANDLE = ctypes.windll.kernel32.OpenEventW( - # Blocks until WTS session tracking can be registered. - # Windows needs time for the WTS session tracking service to initialize. - # NVDA must ensure that the WTS session tracking service is ready before trying to register - SYNCHRONIZE, # DWORD dwDesiredAccess - False, # BOOL bInheritHandle - NVDA sub-processes do not need to inherit this handle - # According to the docs, when the Global\TermSrvReadyEvent global event is set, - # all dependent services have started and WTSRegisterSessionNotification can be successfully called. - # https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification#remarks - "Global\\TermSrvReadyEvent" # LPCWSTR lpName - The name of the event object. +def register(handle: int) -> bool: + # TODO: improve deprecation practice on beta/master merges + log.error( + "NVDA no longer registers session tracking notifications. " + "This function is deprecated, for removal in 2023.1. " + "It was never expected that add-on authors would use this function" ) - if not eventObjectHandle: - error = ctypes.WinError() - log.error("Unexpected error waiting to register session tracking.", exc_info=error) - return False - - registrationSuccess = ctypes.windll.wtsapi32.WTSRegisterSessionNotification(handle, NOTIFY_FOR_THIS_SESSION) - ctypes.windll.kernel32.CloseHandle(eventObjectHandle) - - if registrationSuccess: - log.debug("Registered session tracking") - # Ensure that an initial state is set. - # Do this only when session tracking has been registered, - # so that any changes to the state are not missed via a race condition with session tracking registration. - # As this occurs after NVDA hs registered for session tracking, - # it is possible NVDA is expected to handle a session change notification - # at the same time as initialisation. - # _updateSessionStateLock is used to prevent received session notifications from being handled at the - # same time as initialisation. - _setInitialWindowLockState() - else: - error = ctypes.WinError() - if error.errno == RPC_S_INVALID_BINDING: - log.error( - "WTS registration failed. " - "NVDA waited successfully on TermSrvReadyEvent to ensure that WTS is ready to allow registration. " - "Cause of failure unknown. " - ) - else: - log.error("Unexpected error registering session tracking.", exc_info=error) - - global _isSessionTrackingRegistered - _isSessionTrackingRegistered = registrationSuccess - return isLockStateSuccessfullyTracked() + return True def unregister(handle: HWND) -> None: - """ - This function must be called once for every call to register. - If unregistration fails, NVDA may not work properly until the session can be unregistered in a new instance. - - https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsunregistersessionnotification - """ - if not _isSessionTrackingRegistered: - log.info("Not unregistered session tracking, it was not registered.") - return - if ctypes.windll.wtsapi32.WTSUnRegisterSessionNotification(handle): - log.debug("Unregistered session tracking") - else: - error = ctypes.WinError() - log.error("Unexpected error unregistering session tracking.", exc_info=error) + # TODO: improve deprecation practice on beta/master merges + log.error( + "NVDA no longer registers session tracking notifications. " + "This function is deprecated, for removal in 2023.1. " + "It was never expected that add-on authors would use this function" + ) def handleSessionChange(newState: WindowsTrackedSession, sessionId: int) -> None: - """ - Keeps track of the Windows session state. - When a session change event occurs, the new state is added and the opposite state - is removed. - - For example a "SESSION_LOCK" event removes the "SESSION_UNLOCK" state. - - This does not track SESSION_REMOTE_CONTROL, which isn't part of a logical pair of states. - Managing the state of this is more complex, and NVDA does not need to track this status. - - https://docs.microsoft.com/en-us/windows/win32/termserv/wm-wtssession-change - """ - with _updateSessionStateLock: - stateChanged = False - - log.debug(f"Windows Session state notification received: {newState.name}") - - if not _isSessionTrackingRegistered: - log.debugWarning("Session tracking not registered, unexpected session change message") - - if newState not in _toggleWindowsSessionStatePair: - log.debug(f"Ignoring {newState} event as tracking is not required.") - return - - oppositeState = _toggleWindowsSessionStatePair[newState] - if newState in _currentSessionStates: - log.error( - f"NVDA expects Windows to be in {newState} already. " - f"NVDA may have dropped a {oppositeState} event. " - f"Dropping this {newState} event. " - ) - else: - _currentSessionStates.add(newState) - stateChanged = True - - if oppositeState in _currentSessionStates: - _currentSessionStates.remove(oppositeState) - else: - log.debugWarning( - f"NVDA expects Windows to be in {newState} already. " - f"NVDA may have dropped a {oppositeState} event. " - ) - - log.debug(f"New Windows Session state: {_currentSessionStates}") - if ( - stateChanged - and newState in {WindowsTrackedSession.SESSION_LOCK, WindowsTrackedSession.SESSION_UNLOCK} - ): - from utils.security import postSessionLockStateChanged - postSessionLockStateChanged.notify(isNowLocked=newState == WindowsTrackedSession.SESSION_LOCK) + # TODO: improve deprecation practice on beta/master merges + log.error( + "NVDA no longer registers session tracking notifications. " + "This function is deprecated, for removal in 2023.1. " + "It was never expected that add-on authors would use this function" + ) @contextmanager @@ -445,6 +271,8 @@ def _getSessionLockedValue() -> WTS_LockState: with WTSCurrentSessionInfoEx() as info: infoEx: WTSINFOEX_LEVEL1_W = info.contents.Data.WTSInfoExLevel1 sessionFlags: ctypes.wintypes.LONG = infoEx.SessionFlags - lockState = WTS_LockState(sessionFlags) - log.debug(f"Query Lock state result: {lockState!r}") + try: + lockState = WTS_LockState(sessionFlags) + except ValueError: + return WTS_LockState.WTS_SESSIONSTATE_UNKNOWN return lockState diff --git a/source/winUser.py b/source/winUser.py index 5984f7fd299..855581a8319 100644 --- a/source/winUser.py +++ b/source/winUser.py @@ -500,7 +500,7 @@ def sendMessage(hwnd,msg,param1,param2): return user32.SendMessageW(hwnd,msg,param1,param2) -def getWindowThreadProcessID(hwnd: HWND) -> Tuple[int, int]: +def getWindowThreadProcessID(hwnd: HWNDVal) -> Tuple[int, int]: """Returns a tuple of (processID, threadID)""" processID=c_int() threadID=user32.GetWindowThreadProcessId(hwnd,byref(processID)) From b78dc610554f377ae3e38e716c03118d65af1b84 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Mon, 5 Dec 2022 16:59:30 +1100 Subject: [PATCH 02/20] add unit tests for lockscreen order tracking --- source/NVDAObjects/__init__.py | 9 ++ source/utils/security.py | 92 +++++++++++++++----- tests/unit/test_util/test_security.py | 121 ++++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 23 deletions(-) create mode 100644 tests/unit/test_util/test_security.py diff --git a/source/NVDAObjects/__init__.py b/source/NVDAObjects/__init__.py index 21b2522eb2e..552f453bdde 100644 --- a/source/NVDAObjects/__init__.py +++ b/source/NVDAObjects/__init__.py @@ -30,6 +30,7 @@ TreeInterceptor, ) import braille +from utils.security import _isObjectAboveLockScreen import vision import globalPluginHandler import brailleInput @@ -1433,3 +1434,11 @@ def getSelectedItemsCount(self,maxCount=2): For performance, this method will only count up to the given maxCount number, and if there is one more above that, then sys.maxint is returned stating that many items are selected. """ return 0 + + #: Type definition for auto prop '_get_isAboveLockScreen' + isAboveLockScreen: bool + + def _get_isAboveLockScreen(self) -> bool: + if not isWindowsLocked(): + return True + return _isObjectAboveLockScreen(self) diff --git a/source/utils/security.py b/source/utils/security.py index b79a26ea6bd..8b983eedcd3 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -5,6 +5,8 @@ import typing from typing import ( + Callable, + Optional, Set, ) @@ -143,7 +145,7 @@ def _isSecureObjectWhileLockScreenActivated( @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is outside of the lock screen. """ try: - isObjectInSecure = isWindowsLocked() and not isObjectAboveLockScreen(obj) + isObjectInSecure = isWindowsLocked() and not obj.isAboveLockScreen except Exception: log.exception() return False @@ -157,7 +159,7 @@ def _isSecureObjectWhileLockScreenActivated( return False -def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: +def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, but other Windows can be focused (e.g. Windows Magnifier). @@ -182,10 +184,28 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: or isinstance(obj, SecureDesktopNVDAObject) ): return True - return _isObjectAboveLockScreenCheckZOrder(obj) + import appModuleHandler + runningAppModules = appModuleHandler.runningTable.values() + lockAppModule = next(filter(_isLockAppAndAlive, runningAppModules), None) + + if lockAppModule is None: + # lockAppModule not running/registered by NVDA yet + log.debug( + "lockAppModule not detected when Windows is locked. " + "Cannot detect if object is in lock app, considering object as safe. " + ) + return True -def _isObjectAboveLockScreenCheckZOrder(obj: "NVDAObjects.NVDAObject") -> bool: + from NVDAObjects.window import Window + if not isinstance(obj, Window): + # must be a window to get its HWNDVal + return True + + return _isObjectAboveLockScreenCheckZOrder(obj.windowHandle, lockAppModule.processID) + + +def _isObjectAboveLockScreenCheckZOrder(objWindowHandle: int, lockAppModuleProcessId: int) -> bool: """ This is a risky hack. If the order is incorrectly detected, @@ -195,32 +215,58 @@ def _isObjectAboveLockScreenCheckZOrder(obj: "NVDAObjects.NVDAObject") -> bool: If these functions fail, where possible, NVDA should make NVDA objects accessible. """ - import appModuleHandler - from NVDAObjects.window import Window - if not isinstance(obj, Window): - # must be a window to get its HWNDVal - return True - runningAppModules = appModuleHandler.runningTable.values() - lockAppModule = next(filter(_isLockAppAndAlive, runningAppModules), None) - if lockAppModule is None: - # lockAppModule not running/registered by NVDA yet - log.debug( - "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is in lock app, considering object as safe. " - ) + def _isWindowLockApp(hwnd: winUser.HWNDVal) -> bool: + windowProcessId, _threadId = winUser.getWindowThreadProcessID(hwnd) + return windowProcessId == lockAppModuleProcessId + + def _isNVDAObjectWindow(hwnd: winUser.HWNDVal) -> bool: + return hwnd == objWindowHandle + + lockAppZIndex = _getWindowZIndex(_isWindowLockApp) + objectZIndex = _getWindowZIndex(_isNVDAObjectWindow) + lockAppZIndexCheck = _getWindowZIndex(_isWindowLockApp) + objectZIndexCheck = _getWindowZIndex(_isNVDAObjectWindow) + if lockAppZIndex != lockAppZIndexCheck or objectZIndex != objectZIndexCheck: + log.debugWarning("Order of Windows has changed during execution") + + if lockAppZIndex is None or lockAppZIndexCheck is None: + # this is an unexpected state + # err on accessibility + log.error("Couldn't find lock screen") + return True + elif objectZIndex is None or objectZIndexCheck is None: + # this is an unexpected state + # err on accessibility + log.error("Couldn't find NVDA object's window") + return True + elif lockAppZIndex > objectZIndex and lockAppZIndexCheck > objectZIndexCheck: + # object is behind the lock screen, hide it from the user + return False + elif lockAppZIndex <= objectZIndex and lockAppZIndexCheck <= objectZIndexCheck: + # object is above the lock screen, show it to the user return True + else: + log.debugWarning("Z-index of Windows has changed, unable to determine z-order") + # mixed state between checks + # err on accessibility + return True + +def _getWindowZIndex(matchCond: Callable[[winUser.HWNDVal], bool]) -> Optional[int]: + """ + Z-order can change while this is being checked. + This means this may not always return the correct result. + """ desktopWindow = winUser.getDesktopWindow() nextWindow = winUser.getTopWindow(desktopWindow) + index = 0 while nextWindow: - windowProcessId = winUser.getWindowThreadProcessID(nextWindow) - if nextWindow == obj.windowHandle: - return True - elif windowProcessId == lockAppModule.processID: - return False + if matchCond(nextWindow): + return index nextWindow = winUser.getWindow(nextWindow, winUser.GW_HWNDNEXT) - return False + index += 1 + return None _hasSessionLockStateUnknownWarningBeenGiven = False diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py new file mode 100644 index 00000000000..14b91f31bb0 --- /dev/null +++ b/tests/unit/test_util/test_security.py @@ -0,0 +1,121 @@ +# A part of NonVisual Desktop Access (NVDA) +# This file is covered by the GNU General Public License. +# See the file COPYING for more details. +# Copyright (C) 2022 NV Access Limited. + +"""Unit tests for the blockUntilConditionMet submodule. +""" + +import unittest +from unittest.mock import patch + +from utils.security import ( + _getWindowZIndex, +) +import winUser + + +class _Test_getWindowZIndex(unittest.TestCase): + def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: + currentWindowIndex = self._windows.index(hwnd) + if relation == winUser.GW_HWNDNEXT: + try: + return self._windows[currentWindowIndex + 1] + except IndexError: + return 0 + elif relation == winUser.GW_HWNDPREV: + try: + return self._windows[currentWindowIndex - 1] + except IndexError: + return 0 + else: + return 0 + + def _windowMatches(self, expectedWindow: int): + def _helper(hwnd: int) -> bool: + return hwnd == expectedWindow + return _helper + + def setUp(self) -> None: + self._getDesktopWindowPatch = patch("winUser.getDesktopWindow", lambda: 0) # value is discarded by _getTopWindowPatch + self._getTopWindowPatch = patch("winUser.getTopWindow", lambda _hwnd: self._windows[0]) + self._getWindowPatch = patch("winUser.getWindow", self._getWindow_patched) + self._getDesktopWindowPatch.start() + self._getTopWindowPatch.start() + self._getWindowPatch.start() + self._windows = list(range(1, 11)) # must be 1 indexed + return super().setUp() + + def tearDown(self) -> None: + self._getDesktopWindowPatch.stop() + self._getTopWindowPatch.stop() + self._getWindowPatch.stop() + return super().tearDown() + + +class Test_getWindowZIndex_static(_Test_getWindowZIndex): + def test_noMatch(self): + self.assertIsNone(_getWindowZIndex(lambda x: False)) + + def test_firstWindowMatch_noChanges(self): + targetWindow = 1 + expectedIndex = targetWindow - 1 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + + def test_lastWindowMatch_noChanges(self): + targetWindow = len(self._windows) + expectedIndex = targetWindow - 1 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + + +class Test_getWindowZIndex_dynamic(_Test_getWindowZIndex): + _triggered = False + + def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: + self._moveIndexToNewIndexAtIndexOnce(self._windows.index(hwnd)) + result = super()._getWindow_patched(hwnd, relation) + return result + + def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): + from logging import getLogger + + getLogger().error(f"{currentIndex}") + if currentIndex == self._triggerIndex and not self._triggered: + self._triggered = True + window = self._windows.pop(self._startIndex) + self._windows.insert(self._endIndex, window) + + def test_prev_windowMoves_pastTarget(self): + """A previous window is moved past the target window. This does not affect the z-order.""" + self._startIndex = 2 + self._endIndex = 9 + self._triggerIndex = 5 + targetWindow = 7 + expectedIndex = targetWindow - 1 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + + def test_prev_windowMoves_beforeTarget(self): + """A previous window is moved before the target window. It is counted twice.""" + self._startIndex = 2 + self._endIndex = 5 + self._triggerIndex = 3 + targetWindow = 7 + expectedIndex = targetWindow # difference is normally 1, however a window is counted twice + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + + def test_active_windowMoves_pastTarget(self): + """The window we are looking at moves past the match, skipping our target window""" + self._startIndex = 3 + self._endIndex = 8 + self._triggerIndex = 3 + targetWindow = 6 + self.assertEqual(None, _getWindowZIndex(self._windowMatches(targetWindow))) + + def test_active_windowMoves_beforeTarget(self): + pass + + def test_future_windowMoves_pastTarget(self): + pass + + def test_future_windowMoves_beforeTarget(self): + pass From 25e5b4c32d07d72f704a51b202f340fb2aa6c3cb Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Mon, 5 Dec 2022 17:06:47 +1100 Subject: [PATCH 03/20] improve temp documentation --- tests/unit/test_util/test_security.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index 14b91f31bb0..bc12eec1e6b 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -87,9 +87,9 @@ def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): def test_prev_windowMoves_pastTarget(self): """A previous window is moved past the target window. This does not affect the z-order.""" - self._startIndex = 2 - self._endIndex = 9 - self._triggerIndex = 5 + self._startIndex = 2 # A window at this index + self._endIndex = 9 # is moved to this index + self._triggerIndex = 5 # when this index is reached targetWindow = 7 expectedIndex = targetWindow - 1 self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) @@ -104,7 +104,7 @@ def test_prev_windowMoves_beforeTarget(self): self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) def test_active_windowMoves_pastTarget(self): - """The window we are looking at moves past the match, skipping our target window""" + """A window we are looking at moves past the match, skipping our target window.""" self._startIndex = 3 self._endIndex = 8 self._triggerIndex = 3 From cb47620f5cc635950f92ff6522e1af7ecec6ee1a Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Tue, 6 Dec 2022 11:45:57 +1100 Subject: [PATCH 04/20] improve test docs --- tests/unit/test_util/test_security.py | 91 +++++++++++++++++++++------ 1 file changed, 73 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index bc12eec1e6b..fdcc731c96c 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -6,6 +6,10 @@ """Unit tests for the blockUntilConditionMet submodule. """ +from dataclasses import dataclass +from typing import ( + Optional, +) import unittest from unittest.mock import patch @@ -15,8 +19,17 @@ import winUser +@dataclass +class _MoveWindow: + startIndex: int # A window at this index + endIndex: int # is moved to this index + triggerIndex: int # when this index is reached + triggered = False + + class _Test_getWindowZIndex(unittest.TestCase): def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: + """Fetch current window, find adjacent window by relation.""" currentWindowIndex = self._windows.index(hwnd) if relation == winUser.GW_HWNDNEXT: try: @@ -37,7 +50,8 @@ def _helper(hwnd: int) -> bool: return _helper def setUp(self) -> None: - self._getDesktopWindowPatch = patch("winUser.getDesktopWindow", lambda: 0) # value is discarded by _getTopWindowPatch + # value from _getDesktopWindowPatch is discarded by _getTopWindowPatch + self._getDesktopWindowPatch = patch("winUser.getDesktopWindow", lambda: 0) self._getTopWindowPatch = patch("winUser.getTopWindow", lambda _hwnd: self._windows[0]) self._getWindowPatch = patch("winUser.getWindow", self._getWindow_patched) self._getDesktopWindowPatch.start() @@ -69,7 +83,7 @@ def test_lastWindowMatch_noChanges(self): class Test_getWindowZIndex_dynamic(_Test_getWindowZIndex): - _triggered = False + _queuedMove: Optional[_MoveWindow] = None def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: self._moveIndexToNewIndexAtIndexOnce(self._windows.index(hwnd)) @@ -80,42 +94,83 @@ def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): from logging import getLogger getLogger().error(f"{currentIndex}") - if currentIndex == self._triggerIndex and not self._triggered: - self._triggered = True - window = self._windows.pop(self._startIndex) - self._windows.insert(self._endIndex, window) + if ( + self._queuedMove + and currentIndex == self._queuedMove.triggerIndex + and not self._queuedMove.triggered + ): + self._queuedMove.triggered = True + window = self._windows.pop(self._queuedMove.startIndex) + self._windows.insert(self._queuedMove.endIndex, window) def test_prev_windowMoves_pastTarget(self): """A previous window is moved past the target window. This does not affect the z-order.""" - self._startIndex = 2 # A window at this index - self._endIndex = 9 # is moved to this index - self._triggerIndex = 5 # when this index is reached + self._queuedMove = _MoveWindow( + startIndex=2, + endIndex=9, + triggerIndex=5 + ) targetWindow = 7 expectedIndex = targetWindow - 1 self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) def test_prev_windowMoves_beforeTarget(self): """A previous window is moved before the target window. It is counted twice.""" - self._startIndex = 2 - self._endIndex = 5 - self._triggerIndex = 3 + self._queuedMove = _MoveWindow( + startIndex=2, + endIndex=5, + triggerIndex=3 + ) targetWindow = 7 expectedIndex = targetWindow # difference is normally 1, however a window is counted twice self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) def test_active_windowMoves_pastTarget(self): """A window we are looking at moves past the match, skipping our target window.""" - self._startIndex = 3 - self._endIndex = 8 - self._triggerIndex = 3 + self._queuedMove = _MoveWindow( + startIndex=3, + endIndex=8, + triggerIndex=3 + ) targetWindow = 6 self.assertEqual(None, _getWindowZIndex(self._windowMatches(targetWindow))) def test_active_windowMoves_beforeTarget(self): - pass + """A future window we are looking at moves forward, but before the match, decreasing our z-index.""" + self._queuedMove = _MoveWindow( + startIndex=3, + endIndex=4, + triggerIndex=3 + ) + targetWindow = 7 + # difference is normally 1, however a window is skipped due to the move + expectedIndex = targetWindow - 2 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) def test_future_windowMoves_pastTarget(self): - pass + """A future window moves forward, and past the match. + This does not change anything in practice, the windows affected are yet to be indexed. + However, during the move, the z-index to target is decreased, + as a window is moved past the match.""" + self._queuedMove = _MoveWindow( + startIndex=5, + endIndex=9, + triggerIndex=3 + ) + targetWindow = 7 + # difference is normally 1, however a window is skipped due to the move + expectedIndex = targetWindow - 2 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) def test_future_windowMoves_beforeTarget(self): - pass + """A future window moves forward, and past the match. + This does not change anything in practice, the windows affected are yet to be indexed.""" + self._queuedMove = _MoveWindow( + startIndex=5, + endIndex=7, + triggerIndex=3 + ) + targetWindow = 9 + # difference is normally 1 + expectedIndex = targetWindow - 1 + self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) From cea66f3cf6d9ab2e9c60173d82d38d37a7abc4f7 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Tue, 6 Dec 2022 11:58:24 +1100 Subject: [PATCH 05/20] improve docs and testing --- source/utils/security.py | 21 +++++++++++++++++---- tests/unit/test_util/test_security.py | 9 ++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index 8b983eedcd3..c931f0d532f 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -23,7 +23,6 @@ postSessionLockStateChanged = extensionPoints.Action() """ -# TODO: maintain backwards compat Notifies when a session lock or unlock event occurs. Usage: @@ -145,20 +144,29 @@ def _isSecureObjectWhileLockScreenActivated( @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is outside of the lock screen. """ try: - isObjectInSecure = isWindowsLocked() and not obj.isAboveLockScreen + isObjectBelowLockScreen = isWindowsLocked() and not obj.isAboveLockScreen except Exception: log.exception() return False - if isObjectInSecure: + if isObjectBelowLockScreen: if shouldLog and log.isEnabledFor(log.DEBUG): devInfo = '\n'.join(obj.devInfo) - log.debug(f"Attempt at navigating to a secure object: {devInfo}") + log.debug(f"Attempt at navigating to an object below the lock screen: {devInfo}") return True return False +def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: + # TODO: improve deprecation practice on beta/master merges + log.error( + "This function is deprecated. " + "Instead use obj.isAboveLockScreen. " + ) + return obj.isAboveLockScreen + + def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, @@ -199,6 +207,9 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: from NVDAObjects.window import Window if not isinstance(obj, Window): + log.debug( + "Cannot detect if object is in lock app, considering object as safe. " + ) # must be a window to get its HWNDVal return True @@ -257,6 +268,8 @@ def _getWindowZIndex(matchCond: Callable[[winUser.HWNDVal], bool]) -> Optional[i """ Z-order can change while this is being checked. This means this may not always return the correct result. + + Refer to test_security.Test_getWindowZIndex_dynamic for behaviour. """ desktopWindow = winUser.getDesktopWindow() nextWindow = winUser.getTopWindow(desktopWindow) diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index fdcc731c96c..1ff984dc6fa 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -21,13 +21,18 @@ @dataclass class _MoveWindow: + """Used to move a window from one index to another when a specific index is reached.""" startIndex: int # A window at this index endIndex: int # is moved to this index triggerIndex: int # when this index is reached - triggered = False + triggered = False # If the move has been triggered class _Test_getWindowZIndex(unittest.TestCase): + """ + Base class to patch winUser functions used in _getWindowZIndex. + Navigating windows is replaced by a list of fake HWNDs. + """ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: """Fetch current window, find adjacent window by relation.""" currentWindowIndex = self._windows.index(hwnd) @@ -68,6 +73,7 @@ def tearDown(self) -> None: class Test_getWindowZIndex_static(_Test_getWindowZIndex): + """Test fetching a z-index when the order of window does not change""" def test_noMatch(self): self.assertIsNone(_getWindowZIndex(lambda x: False)) @@ -83,6 +89,7 @@ def test_lastWindowMatch_noChanges(self): class Test_getWindowZIndex_dynamic(_Test_getWindowZIndex): + """Test fetching a z-index when a window moves during the operation""" _queuedMove: Optional[_MoveWindow] = None def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: From 3b31a3cd3e0fe83cca59f7453d5ae7ed2126df69 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Wed, 7 Dec 2022 11:23:26 +1100 Subject: [PATCH 06/20] improve tests --- source/utils/security.py | 2 +- tests/unit/test_util/test_security.py | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index c931f0d532f..55c2d9c2bb8 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -249,7 +249,7 @@ def _isNVDAObjectWindow(hwnd: winUser.HWNDVal) -> bool: elif objectZIndex is None or objectZIndexCheck is None: # this is an unexpected state # err on accessibility - log.error("Couldn't find NVDA object's window") + log.debugWarning("Couldn't find NVDA object's window") return True elif lockAppZIndex > objectZIndex and lockAppZIndexCheck > objectZIndexCheck: # object is behind the lock screen, hide it from the user diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index 1ff984dc6fa..ff47416d873 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -8,6 +8,7 @@ from dataclasses import dataclass from typing import ( + List, Optional, ) import unittest @@ -31,7 +32,11 @@ class _MoveWindow: class _Test_getWindowZIndex(unittest.TestCase): """ Base class to patch winUser functions used in _getWindowZIndex. + Navigating windows is replaced by a list of fake HWNDs. + HWNDs are represented by integers (1-10). + The initial relative z-order of HWNDs is defined by the order of the HWND value in the self._windows list. + A HWND value at index 0, should be considered to have a z-order "above" a HWND value at index 1. """ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: """Fetch current window, find adjacent window by relation.""" @@ -62,7 +67,11 @@ def setUp(self) -> None: self._getDesktopWindowPatch.start() self._getTopWindowPatch.start() self._getWindowPatch.start() - self._windows = list(range(1, 11)) # must be 1 indexed + self._windows: List[winUser.HWNDVal] = list(range(1, 11)) + """ + List of fake HWNDs, given an ordered index to make testing easier. + Must be 1 indexed as a HWND of 0 is treated an error. + """ return super().setUp() def tearDown(self) -> None: @@ -89,7 +98,15 @@ def test_lastWindowMatch_noChanges(self): class Test_getWindowZIndex_dynamic(_Test_getWindowZIndex): - """Test fetching a z-index when a window moves during the operation""" + """ + Test fetching a z-index when a window moves during the operation. + + To model changes in z-order, a _MoveWindow is used to describe the change. + When the getWindow is called with the triggerIndex, the value at start index is moved + "in front" of the window at end index. + Effectively this means that before getting the window at the triggerIndex, the order of + windows will change. + """ _queuedMove: Optional[_MoveWindow] = None def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: From da56e3f82caa8ae55fcc15cc1e039215d9277839 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Wed, 7 Dec 2022 15:04:05 +1100 Subject: [PATCH 07/20] fix up typing --- source/globalVars.py | 4 ++-- source/textInfos/__init__.py | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/source/globalVars.py b/source/globalVars.py index ed6c7f43cfa..40120890504 100644 --- a/source/globalVars.py +++ b/source/globalVars.py @@ -20,8 +20,8 @@ import typing if typing.TYPE_CHECKING: + import documentBase # noqa: F401 used for type checking only import NVDAObjects # noqa: F401 used for type checking only - import textInfos # noqa: F401 used for type checking only class DefaultAppArgs(argparse.Namespace): @@ -71,7 +71,7 @@ class DefaultAppArgs(argparse.Namespace): mouseOldY=None navigatorObject: typing.Optional['NVDAObjects.NVDAObject'] = None reviewPosition=None -reviewPositionObj: typing.Optional["textInfos.TextInfoObjT"] = None +reviewPositionObj: typing.Optional["documentBase.TextContainerObject"] = None lastProgressValue=0 appArgs = DefaultAppArgs() unknownAppArgs: typing.List[str] = [] diff --git a/source/textInfos/__init__.py b/source/textInfos/__init__.py index c2ee09fa944..72023971c53 100755 --- a/source/textInfos/__init__.py +++ b/source/textInfos/__init__.py @@ -31,8 +31,9 @@ from logHandler import log if typing.TYPE_CHECKING: + import documentBase # noqa: F401 used for type checking only import NVDAObjects - import treeInterceptorHandler # noqa: F401 used for type checking only + SpeechSequence = List[Union[Any, str]] @@ -288,9 +289,6 @@ def _logBadSequenceTypes(sequence: SpeechSequence, shouldRaise: bool = True): return speech.types.logBadSequenceTypes(sequence, raiseExceptionOnError=shouldRaise) -TextInfoObjT = Union["NVDAObjects.NVDAObject", "treeInterceptorHandler.TreeInterceptor"] - - class TextInfo(baseObject.AutoPropertyObject): """Provides information about a range of text in an object and facilitates access to all text in the widget. A TextInfo represents a specific range of text, providing access to the text itself, as well as information about the text such as its formatting and any associated controls. @@ -313,7 +311,7 @@ class TextInfo(baseObject.AutoPropertyObject): def __init__( self, - obj: TextInfoObjT, + obj: "documentBase.TextContainerObject", position ): """Constructor. @@ -346,9 +344,9 @@ def _set_end(self, otherEndpoint: "TextInfoEndpoint"): self.end.moveTo(otherEndpoint) #: Typing information for auto-property: _get_obj - obj: TextInfoObjT + obj: "documentBase.TextContainerObject" - def _get_obj(self) -> TextInfoObjT: + def _get_obj(self) -> "documentBase.TextContainerObject": """The object containing the range of text being represented.""" return self._obj() From 49e6de030f0f01ef3d24f9c8f361274bd41c2839 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 8 Dec 2022 12:20:38 +1100 Subject: [PATCH 08/20] use bi-directional search --- source/utils/security.py | 73 ++++----- tests/unit/test_util/test_security.py | 207 +++++++++++++++++--------- 2 files changed, 162 insertions(+), 118 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index 55c2d9c2bb8..01d4c9f57d1 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -220,8 +220,7 @@ def _isObjectAboveLockScreenCheckZOrder(objWindowHandle: int, lockAppModuleProce """ This is a risky hack. If the order is incorrectly detected, - the Windows UX may become inaccessible - or secure information may become accessible. + secure information may become accessible. If these functions fail, where possible, NVDA should make NVDA objects accessible. @@ -231,55 +230,39 @@ def _isWindowLockApp(hwnd: winUser.HWNDVal) -> bool: windowProcessId, _threadId = winUser.getWindowThreadProcessID(hwnd) return windowProcessId == lockAppModuleProcessId - def _isNVDAObjectWindow(hwnd: winUser.HWNDVal) -> bool: - return hwnd == objWindowHandle - - lockAppZIndex = _getWindowZIndex(_isWindowLockApp) - objectZIndex = _getWindowZIndex(_isNVDAObjectWindow) - lockAppZIndexCheck = _getWindowZIndex(_isWindowLockApp) - objectZIndexCheck = _getWindowZIndex(_isNVDAObjectWindow) - if lockAppZIndex != lockAppZIndexCheck or objectZIndex != objectZIndexCheck: - log.debugWarning("Order of Windows has changed during execution") - - if lockAppZIndex is None or lockAppZIndexCheck is None: - # this is an unexpected state - # err on accessibility - log.error("Couldn't find lock screen") - return True - elif objectZIndex is None or objectZIndexCheck is None: - # this is an unexpected state - # err on accessibility - log.debugWarning("Couldn't find NVDA object's window") - return True - elif lockAppZIndex > objectZIndex and lockAppZIndexCheck > objectZIndexCheck: - # object is behind the lock screen, hide it from the user - return False - elif lockAppZIndex <= objectZIndex and lockAppZIndexCheck <= objectZIndexCheck: - # object is above the lock screen, show it to the user - return True - else: - log.debugWarning("Z-index of Windows has changed, unable to determine z-order") - # mixed state between checks - # err on accessibility + try: + return _isWindowAboveWindowMatchesCond(objWindowHandle, _isWindowLockApp) + except _WindowNotFoundError: + log.debugWarning("Couldn't find lock screen") return True -def _getWindowZIndex(matchCond: Callable[[winUser.HWNDVal], bool]) -> Optional[int]: +class _WindowNotFoundError(Exception): + """ + Raised when a window which matches the expected condition + is not found by _isWindowAboveWindowMatchesCond """ - Z-order can change while this is being checked. - This means this may not always return the correct result. + pass - Refer to test_security.Test_getWindowZIndex_dynamic for behaviour. + +def _isWindowAboveWindowMatchesCond( + targetWindow: winUser.HWNDVal, + matchCond: Callable[[winUser.HWNDVal], bool] +) -> bool: + """ Returns True if targetWindow is above a window that matches the match condition. """ - desktopWindow = winUser.getDesktopWindow() - nextWindow = winUser.getTopWindow(desktopWindow) - index = 0 - while nextWindow: - if matchCond(nextWindow): - return index - nextWindow = winUser.getWindow(nextWindow, winUser.GW_HWNDNEXT) - index += 1 - return None + aboveWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDPREV) + belowWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDNEXT) + while aboveWindow or belowWindow: + if matchCond(belowWindow): # target window is above the found window + return True + elif matchCond(aboveWindow): # found window is above the target window + return False + if aboveWindow: + aboveWindow = winUser.getWindow(aboveWindow, winUser.GW_HWNDPREV) + if belowWindow: + belowWindow = winUser.getWindow(belowWindow, winUser.GW_HWNDNEXT) + raise _WindowNotFoundError(f"Window not found matching condition {matchCond}") _hasSessionLockStateUnknownWarningBeenGiven = False diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index ff47416d873..476e7e5a602 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -15,7 +15,8 @@ from unittest.mock import patch from utils.security import ( - _getWindowZIndex, + _WindowNotFoundError, + _isWindowAboveWindowMatchesCond, ) import winUser @@ -29,9 +30,9 @@ class _MoveWindow: triggered = False # If the move has been triggered -class _Test_getWindowZIndex(unittest.TestCase): +class _Test_isWindowAboveWindowMatchesCond(unittest.TestCase): """ - Base class to patch winUser functions used in _getWindowZIndex. + Base class to patch winUser functions used in _isWindowAboveWindowMatchesCond. Navigating windows is replaced by a list of fake HWNDs. HWNDs are represented by integers (1-10). @@ -43,16 +44,19 @@ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: currentWindowIndex = self._windows.index(hwnd) if relation == winUser.GW_HWNDNEXT: try: - return self._windows[currentWindowIndex + 1] + nextIndex = currentWindowIndex + 1 except IndexError: return 0 elif relation == winUser.GW_HWNDPREV: try: - return self._windows[currentWindowIndex - 1] + nextIndex = currentWindowIndex - 1 except IndexError: return 0 else: return 0 + if nextIndex >= len(self._windows) or nextIndex < 0: + return 0 + return self._windows[nextIndex] def _windowMatches(self, expectedWindow: int): def _helper(hwnd: int) -> bool: @@ -60,12 +64,7 @@ def _helper(hwnd: int) -> bool: return _helper def setUp(self) -> None: - # value from _getDesktopWindowPatch is discarded by _getTopWindowPatch - self._getDesktopWindowPatch = patch("winUser.getDesktopWindow", lambda: 0) - self._getTopWindowPatch = patch("winUser.getTopWindow", lambda _hwnd: self._windows[0]) self._getWindowPatch = patch("winUser.getWindow", self._getWindow_patched) - self._getDesktopWindowPatch.start() - self._getTopWindowPatch.start() self._getWindowPatch.start() self._windows: List[winUser.HWNDVal] = list(range(1, 11)) """ @@ -75,31 +74,37 @@ def setUp(self) -> None: return super().setUp() def tearDown(self) -> None: - self._getDesktopWindowPatch.stop() - self._getTopWindowPatch.stop() self._getWindowPatch.stop() return super().tearDown() -class Test_getWindowZIndex_static(_Test_getWindowZIndex): +class Test_isWindowAboveWindowMatchesCond_static(_Test_isWindowAboveWindowMatchesCond): """Test fetching a z-index when the order of window does not change""" - def test_noMatch(self): - self.assertIsNone(_getWindowZIndex(lambda x: False)) + def test_windowNotFound(self): + startWindow = len(self._windows) // 2 + with self.assertRaises(_WindowNotFoundError): + _isWindowAboveWindowMatchesCond(startWindow, lambda x: False) - def test_firstWindowMatch_noChanges(self): - targetWindow = 1 - expectedIndex = targetWindow - 1 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + def test_isAbove(self): + aboveIndex = 1 + belowIndex = 2 + self.assertTrue(_isWindowAboveWindowMatchesCond(aboveIndex, self._windowMatches(belowIndex))) - def test_lastWindowMatch_noChanges(self): - targetWindow = len(self._windows) - expectedIndex = targetWindow - 1 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + def test_isBelow(self): + aboveIndex = 1 + belowIndex = 2 + self.assertFalse(_isWindowAboveWindowMatchesCond(belowIndex, self._windowMatches(aboveIndex))) -class Test_getWindowZIndex_dynamic(_Test_getWindowZIndex): +class Test_isWindowAboveWindowMatchesCond_dynamic(_Test_isWindowAboveWindowMatchesCond): """ - Test fetching a z-index when a window moves during the operation. + Test fetching comparing the relative order of 2 windows, + where a window moves during the operation. + + This test models changes when performing a bi-direction search that expects the + start window to be before/above the end window. + By symmetry, the same test results are expected if the search goes the other way, + expecting it to return False, rather than True. To model changes in z-order, a _MoveWindow is used to describe the change. When the getWindow is called with the triggerIndex, the value at start index is moved @@ -117,7 +122,7 @@ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): from logging import getLogger - getLogger().error(f"{currentIndex}") + getLogger().error(f"Current index {currentIndex}, {self._windows}") if ( self._queuedMove and currentIndex == self._queuedMove.triggerIndex @@ -127,74 +132,130 @@ def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): window = self._windows.pop(self._queuedMove.startIndex) self._windows.insert(self._queuedMove.endIndex, window) - def test_prev_windowMoves_pastTarget(self): - """A previous window is moved past the target window. This does not affect the z-order.""" + def test_visited_windowMoves_pastTarget(self): + """ + A visited window is moved past the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 5 self._queuedMove = _MoveWindow( - startIndex=2, - endIndex=9, - triggerIndex=5 + startIndex=3, + endIndex=6, + triggerIndex=4 ) - targetWindow = 7 - expectedIndex = targetWindow - 1 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_prev_windowMoves_beforeTarget(self): - """A previous window is moved before the target window. It is counted twice.""" + def test_visited_windowMoves_beforeTarget(self): + """ + A visited window is moved towards but before the target window. + It is counted twice. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 6 self._queuedMove = _MoveWindow( - startIndex=2, + startIndex=3, endIndex=5, - triggerIndex=3 + triggerIndex=4 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_visited_windowMoves_awayFromTarget(self): + """ + A visited window is moved in the opposite direction from the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 6 + self._queuedMove = _MoveWindow( + startIndex=3, + endIndex=1, + triggerIndex=4 ) - targetWindow = 7 - expectedIndex = targetWindow # difference is normally 1, however a window is counted twice - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) def test_active_windowMoves_pastTarget(self): - """A window we are looking at moves past the match, skipping our target window.""" + """ + A window we are currently visiting moves past the target window. + This causes the search to skip the target window. + """ + startWindow = 2 + targetWindow = 5 self._queuedMove = _MoveWindow( startIndex=3, - endIndex=8, + endIndex=7, triggerIndex=3 ) - targetWindow = 6 - self.assertEqual(None, _getWindowZIndex(self._windowMatches(targetWindow))) + with self.assertRaises(_WindowNotFoundError): + _isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow)) def test_active_windowMoves_beforeTarget(self): - """A future window we are looking at moves forward, but before the match, decreasing our z-index.""" + """ + A window we are currently visiting moves towards but before the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 10 self._queuedMove = _MoveWindow( startIndex=3, - endIndex=4, + endIndex=5, triggerIndex=3 ) - targetWindow = 7 - # difference is normally 1, however a window is skipped due to the move - expectedIndex = targetWindow - 2 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) - - def test_future_windowMoves_pastTarget(self): - """A future window moves forward, and past the match. - This does not change anything in practice, the windows affected are yet to be indexed. - However, during the move, the z-index to target is decreased, - as a window is moved past the match.""" + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_active_windowMoves_awayFromTarget(self): + """ + A window we are currently visiting moves in the opposite direction to the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 10 self._queuedMove = _MoveWindow( - startIndex=5, - endIndex=9, + startIndex=3, + endIndex=1, triggerIndex=3 ) - targetWindow = 7 - # difference is normally 1, however a window is skipped due to the move - expectedIndex = targetWindow - 2 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) - - def test_future_windowMoves_beforeTarget(self): - """A future window moves forward, and past the match. - This does not change anything in practice, the windows affected are yet to be indexed.""" + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_unvisited_windowMoves_pastTarget(self): + """ + An unvisited window moves towards, and past the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 6 self._queuedMove = _MoveWindow( - startIndex=5, - endIndex=7, + startIndex=4, + endIndex=8, + triggerIndex=3 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_unvisited_windowMoves_beforeTarget(self): + """ + An unvisited window moves towards, but before the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 8 + self._queuedMove = _MoveWindow( + startIndex=4, + endIndex=6, + triggerIndex=3 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_unvisited_windowMoves_awayFromTarget(self): + """ + An unvisited window moves in the opposite direction the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 8 + self._queuedMove = _MoveWindow( + startIndex=4, + endIndex=1, triggerIndex=3 ) - targetWindow = 9 - # difference is normally 1 - expectedIndex = targetWindow - 1 - self.assertEqual(expectedIndex, _getWindowZIndex(self._windowMatches(targetWindow))) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) From f35fe011d2fd390756a295ce2f4faa6833c293fd Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 8 Dec 2022 13:01:44 +1100 Subject: [PATCH 09/20] add missing tests --- tests/unit/test_util/test_security.py | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index 476e7e5a602..8bcf3816eaf 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -259,3 +259,46 @@ def test_unvisited_windowMoves_awayFromTarget(self): triggerIndex=3 ) self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_startWindow_windowMoves_pastTarget(self): + """ + Start window moves towards, and past the target window. + This means the relative z-order has changed, but the change is not detected. + The relative z-order of the start of the search is returned. + """ + startWindow = 2 + targetWindow = 6 + self._queuedMove = _MoveWindow( + startIndex=startWindow - 1, + endIndex=8, + triggerIndex=3 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_startWindow_windowMoves_beforeTarget(self): + """ + Start window moves towards, but before the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 8 + self._queuedMove = _MoveWindow( + startIndex=startWindow - 1, + endIndex=6, + triggerIndex=3 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) + + def test_StartWindow_windowMoves_awayFromTarget(self): + """ + Start window moves in the opposite direction the target window. + This does not affect the relative z-order. + """ + startWindow = 2 + targetWindow = 8 + self._queuedMove = _MoveWindow( + startIndex=startWindow - 1, + endIndex=1, + triggerIndex=3 + ) + self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) From 70707620b66ecbae26f5016f1d95d320c34bfb4a Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 8 Dec 2022 13:02:15 +1100 Subject: [PATCH 10/20] remove redundant initialization changes --- source/winAPI/sessionTracking.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 7c63dc219ed..43ed19aecfe 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -63,7 +63,6 @@ Unused in NVDA core, duplicate of winKernel.SYNCHRONIZE. """ -_initializationLockState: Optional[bool] = None _lockStateTracker: Optional["_WindowsLockedState"] = None _windowsWasPreviouslyLocked = False @@ -122,20 +121,15 @@ def isWindowsLocked() -> bool: Not to be confused with the Windows sign-in screen, a secure screen. """ from core import _TrackNVDAInitialization - from winAPI.sessionTracking import _isWindowsLocked_checkViaSessionQuery from systemUtils import _isSecureDesktop if not _TrackNVDAInitialization.isInitializationComplete(): return False if _isSecureDesktop(): return False - global _initializationLockState - if _lockStateTracker is not None: - _isWindowsLocked = _lockStateTracker.isWindowsLocked - else: - if _initializationLockState is None: - _initializationLockState = _isWindowsLocked_checkViaSessionQuery() - _isWindowsLocked = _initializationLockState - return _isWindowsLocked + if _lockStateTracker is None: + log.error("_TrackNVDAInitialization.markInitializationComplete was called before sessionTracking.initialize") + return False + return _lockStateTracker.isWindowsLocked def _isWindowsLocked_checkViaSessionQuery() -> bool: From f8fb3745f42f5222e290902393a15ef63a961240 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Fri, 9 Dec 2022 10:39:46 +1100 Subject: [PATCH 11/20] make approach more robust --- source/nvda.pyw | 1 + source/utils/security.py | 40 ++++++++++++++++++++++----- source/winUser.py | 9 +++++- tests/unit/test_util/test_security.py | 10 ++----- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/source/nvda.pyw b/source/nvda.pyw index 9d1cddb10f6..9b7c78d6201 100755 --- a/source/nvda.pyw +++ b/source/nvda.pyw @@ -357,6 +357,7 @@ def _serviceDebugEnabled() -> bool: if winreg.QueryValueEx(k, "serviceDebug")[0]: return True except WindowsError: + # Expected state by default, serviceDebug parameter not set pass return False diff --git a/source/utils/security.py b/source/utils/security.py index 01d4c9f57d1..57879c31590 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -167,6 +167,29 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: return obj.isAboveLockScreen +def _checkWindowsForAppModules(): + from ctypes import c_bool, WINFUNCTYPE, c_int, POINTER, windll # TODO move and order + from ctypes.wintypes import LPARAM + import appModuleHandler + # BOOL CALLBACK EnumWindowsProc _In_ HWND,_In_ LPARAM + # HWND as a pointer creates confusion, treat as an int + # http://makble.com/the-story-of-lpclong + @WINFUNCTYPE(c_bool, c_int, POINTER(c_int)) + def _appModuleHandlerUpdate(hwnd: winUser.HWNDVal, _lParam: LPARAM) -> bool: + processID, _threadID = winUser.getWindowThreadProcessID(hwnd) + appModuleHandler.update(processID) + return True + + if not windll.user32.EnumWindows(_appModuleHandlerUpdate, 0): + log.error("Failed search for lockapp App Module") + + +def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: + import appModuleHandler + runningAppModules = appModuleHandler.runningTable.values() + return next(filter(_isLockAppAndAlive, runningAppModules), None) + + def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, @@ -193,10 +216,10 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: ): return True - import appModuleHandler - runningAppModules = appModuleHandler.runningTable.values() - lockAppModule = next(filter(_isLockAppAndAlive, runningAppModules), None) - + lockAppModule = _findLockAppModule() + if lockAppModule is None: + _checkWindowsForAppModules() + lockAppModule = _findLockAppModule() if lockAppModule is None: # lockAppModule not running/registered by NVDA yet log.debug( @@ -253,14 +276,17 @@ def _isWindowAboveWindowMatchesCond( """ aboveWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDPREV) belowWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDNEXT) - while aboveWindow or belowWindow: + while ( + aboveWindow != winUser.GW_RESULT_NOT_FOUND + or belowWindow != winUser.GW_RESULT_NOT_FOUND + ): if matchCond(belowWindow): # target window is above the found window return True elif matchCond(aboveWindow): # found window is above the target window return False - if aboveWindow: + if aboveWindow != winUser.GW_RESULT_NOT_FOUND: aboveWindow = winUser.getWindow(aboveWindow, winUser.GW_HWNDPREV) - if belowWindow: + if belowWindow != winUser.GW_RESULT_NOT_FOUND: belowWindow = winUser.getWindow(belowWindow, winUser.GW_HWNDNEXT) raise _WindowNotFoundError(f"Window not found matching condition {matchCond}") diff --git a/source/winUser.py b/source/winUser.py index 855581a8319..51815078d43 100644 --- a/source/winUser.py +++ b/source/winUser.py @@ -132,10 +132,17 @@ class GUITHREADINFO(Structure): GWL_ID=-12 GWL_STYLE=-16 GWL_EXSTYLE=-20 -#getWindow + +# getWindow: TODO turn to enum GW_HWNDNEXT=2 GW_HWNDPREV=3 GW_OWNER=4 +GW_RESULT_NOT_FOUND = 0 +""" +When GetWindow returns 0, it means the window has not been found. +For example the last window has been reached, or an error has occurred. +""" + # SetLayeredWindowAttributes LWA_ALPHA = 2 LWA_COLORKEY = 1 diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index 8bcf3816eaf..d668a0d3ca3 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -43,19 +43,13 @@ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: """Fetch current window, find adjacent window by relation.""" currentWindowIndex = self._windows.index(hwnd) if relation == winUser.GW_HWNDNEXT: - try: nextIndex = currentWindowIndex + 1 - except IndexError: - return 0 elif relation == winUser.GW_HWNDPREV: - try: nextIndex = currentWindowIndex - 1 - except IndexError: - return 0 else: - return 0 + return winUser.GW_RESULT_NOT_FOUND if nextIndex >= len(self._windows) or nextIndex < 0: - return 0 + return winUser.GW_RESULT_NOT_FOUND return self._windows[nextIndex] def _windowMatches(self, expectedWindow: int): From 452babcf286c72b0645e02e9a48d098c9ca09e59 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Fri, 9 Dec 2022 14:59:14 +1100 Subject: [PATCH 12/20] walk backwards algorithm --- source/utils/security.py | 67 ++++++++++++++++++++++++---------------- source/winUser.py | 6 +++- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index 57879c31590..299dacc7ab1 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -6,6 +6,7 @@ import typing from typing import ( Callable, + List, Optional, Set, ) @@ -220,13 +221,13 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: if lockAppModule is None: _checkWindowsForAppModules() lockAppModule = _findLockAppModule() - if lockAppModule is None: - # lockAppModule not running/registered by NVDA yet - log.debug( - "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is in lock app, considering object as safe. " - ) - return True + if lockAppModule is None: + # lockAppModule not running/registered by NVDA yet + log.debug( + "lockAppModule not detected when Windows is locked. " + "Cannot detect if object is in lock app, considering object as safe. " + ) + return True from NVDAObjects.window import Window if not isinstance(obj, Window): @@ -236,7 +237,9 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: # must be a window to get its HWNDVal return True - return _isObjectAboveLockScreenCheckZOrder(obj.windowHandle, lockAppModule.processID) + topLevelWindowHandle = winUser.getAncestor(obj.windowHandle, winUser.GA_ROOT) + # TODO: time this + return _isObjectAboveLockScreenCheckZOrder(topLevelWindowHandle, lockAppModule.processID) def _isObjectAboveLockScreenCheckZOrder(objWindowHandle: int, lockAppModuleProcessId: int) -> bool: @@ -255,12 +258,12 @@ def _isWindowLockApp(hwnd: winUser.HWNDVal) -> bool: try: return _isWindowAboveWindowMatchesCond(objWindowHandle, _isWindowLockApp) - except _WindowNotFoundError: + except _UnexpectedWindowCountError: log.debugWarning("Couldn't find lock screen") return True -class _WindowNotFoundError(Exception): +class _UnexpectedWindowCountError(Exception): """ Raised when a window which matches the expected condition is not found by _isWindowAboveWindowMatchesCond @@ -269,26 +272,38 @@ class _WindowNotFoundError(Exception): def _isWindowAboveWindowMatchesCond( - targetWindow: winUser.HWNDVal, + window: winUser.HWNDVal, matchCond: Callable[[winUser.HWNDVal], bool] ) -> bool: """ Returns True if targetWindow is above a window that matches the match condition. """ - aboveWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDPREV) - belowWindow = winUser.getWindow(targetWindow, winUser.GW_HWNDNEXT) - while ( - aboveWindow != winUser.GW_RESULT_NOT_FOUND - or belowWindow != winUser.GW_RESULT_NOT_FOUND - ): - if matchCond(belowWindow): # target window is above the found window - return True - elif matchCond(aboveWindow): # found window is above the target window - return False - if aboveWindow != winUser.GW_RESULT_NOT_FOUND: - aboveWindow = winUser.getWindow(aboveWindow, winUser.GW_HWNDPREV) - if belowWindow != winUser.GW_RESULT_NOT_FOUND: - belowWindow = winUser.getWindow(belowWindow, winUser.GW_HWNDNEXT) - raise _WindowNotFoundError(f"Window not found matching condition {matchCond}") + desktopWindow = winUser.getDesktopWindow() + topLevelWindow = winUser.getTopWindow(desktopWindow) + bottomWindow = winUser.getWindow(topLevelWindow, winUser.GW_HWNDLAST) + currentWindow = bottomWindow + currentIndex = 0 # 0 is the last/lowest window + window1Indexes: List[int] = [] + window2Indexes: List[int] = [] + while currentWindow != winUser.GW_RESULT_NOT_FOUND: + if currentWindow == window: + window1Indexes.append(currentIndex) + if matchCond(currentWindow): + window2Indexes.append(currentIndex) + currentWindow = winUser.getWindow(currentWindow, winUser.GW_HWNDPREV) + currentIndex += 1 + if len(window1Indexes) != 1 or len(window2Indexes) == 0: + raise _UnexpectedWindowCountError( + "Windows found\n" + f" - window 1 indexes: {window1Indexes} (expects len 1)\n" + f" - window 2 indexes: {window2Indexes} (expects len >= 1)\n" + ) + lowestWin2Window = min(window2Indexes) + highestWin1Window = max(window1Indexes) + if highestWin1Window >= lowestWin2Window: + # this means it is above the lockscreen + return True + else: + return False _hasSessionLockStateUnknownWarningBeenGiven = False diff --git a/source/winUser.py b/source/winUser.py index 51815078d43..a5b954ac2c6 100644 --- a/source/winUser.py +++ b/source/winUser.py @@ -133,10 +133,14 @@ class GUITHREADINFO(Structure): GWL_STYLE=-16 GWL_EXSTYLE=-20 -# getWindow: TODO turn to enum +# getWindow relationship parameters: TODO turn to enum +GW_HWNDFIRST = 0 +GW_HWNDLAST = 1 GW_HWNDNEXT=2 GW_HWNDPREV=3 GW_OWNER=4 + +# getWindow results GW_RESULT_NOT_FOUND = 0 """ When GetWindow returns 0, it means the window has not been found. From a69a4a2b27646749c4fb4bc3df4397fc89893c7c Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Mon, 12 Dec 2022 15:41:16 +1100 Subject: [PATCH 13/20] fix up algorithm and tests --- source/utils/security.py | 22 +- source/winAPI/sessionTracking.py | 5 +- tests/unit/test_util/test_security.py | 442 +++++++++++++++++--------- 3 files changed, 306 insertions(+), 163 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index 299dacc7ab1..1b160be786f 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -172,9 +172,11 @@ def _checkWindowsForAppModules(): from ctypes import c_bool, WINFUNCTYPE, c_int, POINTER, windll # TODO move and order from ctypes.wintypes import LPARAM import appModuleHandler + # BOOL CALLBACK EnumWindowsProc _In_ HWND,_In_ LPARAM # HWND as a pointer creates confusion, treat as an int # http://makble.com/the-story-of-lpclong + @WINFUNCTYPE(c_bool, c_int, POINTER(c_int)) def _appModuleHandlerUpdate(hwnd: winUser.HWNDVal, _lParam: LPARAM) -> bool: processID, _threadID = winUser.getWindowThreadProcessID(hwnd) @@ -275,7 +277,9 @@ def _isWindowAboveWindowMatchesCond( window: winUser.HWNDVal, matchCond: Callable[[winUser.HWNDVal], bool] ) -> bool: - """ Returns True if targetWindow is above a window that matches the match condition. + """ Returns True if window is above a window that matches matchCond. + If the first window is not found, but the second window is, + it is assumed that the first window is above the second window. """ desktopWindow = winUser.getDesktopWindow() topLevelWindow = winUser.getTopWindow(desktopWindow) @@ -283,24 +287,24 @@ def _isWindowAboveWindowMatchesCond( currentWindow = bottomWindow currentIndex = 0 # 0 is the last/lowest window window1Indexes: List[int] = [] - window2Indexes: List[int] = [] + window2Index: Optional[int] = None while currentWindow != winUser.GW_RESULT_NOT_FOUND: if currentWindow == window: window1Indexes.append(currentIndex) if matchCond(currentWindow): - window2Indexes.append(currentIndex) + if not window1Indexes: + return True + window2Index = currentIndex + break currentWindow = winUser.getWindow(currentWindow, winUser.GW_HWNDPREV) currentIndex += 1 - if len(window1Indexes) != 1 or len(window2Indexes) == 0: + if len(window1Indexes) != 1 or window2Index is None: raise _UnexpectedWindowCountError( "Windows found\n" f" - window 1 indexes: {window1Indexes} (expects len 1)\n" - f" - window 2 indexes: {window2Indexes} (expects len >= 1)\n" + f" - window 2 index: {window2Index}\n" ) - lowestWin2Window = min(window2Indexes) - highestWin1Window = max(window1Indexes) - if highestWin1Window >= lowestWin2Window: - # this means it is above the lockscreen + if window1Indexes[0] >= window2Index: return True else: return False diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 43ed19aecfe..cdc5a1a93f1 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -127,7 +127,10 @@ def isWindowsLocked() -> bool: if _isSecureDesktop(): return False if _lockStateTracker is None: - log.error("_TrackNVDAInitialization.markInitializationComplete was called before sessionTracking.initialize") + log.error( + "_TrackNVDAInitialization.markInitializationComplete was called " + "before sessionTracking.initialize" + ) return False return _lockStateTracker.isWindowsLocked diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index d668a0d3ca3..aeecd30c4c8 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -10,12 +10,13 @@ from typing import ( List, Optional, + Type, ) import unittest from unittest.mock import patch from utils.security import ( - _WindowNotFoundError, + _UnexpectedWindowCountError, _isWindowAboveWindowMatchesCond, ) import winUser @@ -24,9 +25,9 @@ @dataclass class _MoveWindow: """Used to move a window from one index to another when a specific index is reached.""" - startIndex: int # A window at this index - endIndex: int # is moved to this index - triggerIndex: int # when this index is reached + HWNDToMove: winUser.HWNDVal # This window + insertBelowHWND: winUser.HWNDVal # is moved to below this window + triggerHWND: winUser.HWNDVal # when this window is reached triggered = False # If the move has been triggered @@ -40,12 +41,19 @@ class _Test_isWindowAboveWindowMatchesCond(unittest.TestCase): A HWND value at index 0, should be considered to have a z-order "above" a HWND value at index 1. """ def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: - """Fetch current window, find adjacent window by relation.""" + """ + Fetch current window, find adjacent window by relation. + The top level window has the highest index. + """ currentWindowIndex = self._windows.index(hwnd) if relation == winUser.GW_HWNDNEXT: - nextIndex = currentWindowIndex + 1 + nextIndex = currentWindowIndex - 1 elif relation == winUser.GW_HWNDPREV: - nextIndex = currentWindowIndex - 1 + nextIndex = currentWindowIndex + 1 + elif relation == winUser.GW_HWNDFIRST: + nextIndex = len(self._windows) - 1 + elif relation == winUser.GW_HWNDLAST: + nextIndex = 0 else: return winUser.GW_RESULT_NOT_FOUND if nextIndex >= len(self._windows) or nextIndex < 0: @@ -59,34 +67,46 @@ def _helper(hwnd: int) -> bool: def setUp(self) -> None: self._getWindowPatch = patch("winUser.getWindow", self._getWindow_patched) + self._getDesktopWindowPatch = patch("winUser.getDesktopWindow", lambda: 0) + self._getTopWindowPatch = patch("winUser.getTopWindow", lambda _: self._windows[0]) self._getWindowPatch.start() - self._windows: List[winUser.HWNDVal] = list(range(1, 11)) + self._getTopWindowPatch.start() + self._getDesktopWindowPatch.start() + self._generateWindows() + return super().setUp() + + def _generateWindows(self): """ List of fake HWNDs, given an ordered index to make testing easier. Must be 1 indexed as a HWND of 0 is treated an error. """ - return super().setUp() + self._windows: List[winUser.HWNDVal] = list(range(1, 11)) def tearDown(self) -> None: self._getWindowPatch.stop() + self._getTopWindowPatch.stop() + self._getDesktopWindowPatch.stop() return super().tearDown() class Test_isWindowAboveWindowMatchesCond_static(_Test_isWindowAboveWindowMatchesCond): """Test fetching a z-index when the order of window does not change""" - def test_windowNotFound(self): - startWindow = len(self._windows) // 2 - with self.assertRaises(_WindowNotFoundError): - _isWindowAboveWindowMatchesCond(startWindow, lambda x: False) + def test_secondWindowNotFound(self): + with self.assertRaises(_UnexpectedWindowCountError): + # Errors are handled as if window is above + _isWindowAboveWindowMatchesCond(5, lambda x: False) + + def test_firstWindowNotFound(self): + self.assertTrue(_isWindowAboveWindowMatchesCond(-1, lambda x: x == 5)) def test_isAbove(self): - aboveIndex = 1 - belowIndex = 2 + aboveIndex = 2 + belowIndex = 1 self.assertTrue(_isWindowAboveWindowMatchesCond(aboveIndex, self._windowMatches(belowIndex))) def test_isBelow(self): - aboveIndex = 1 - belowIndex = 2 + aboveIndex = 2 + belowIndex = 1 self.assertFalse(_isWindowAboveWindowMatchesCond(belowIndex, self._windowMatches(aboveIndex))) @@ -95,204 +115,320 @@ class Test_isWindowAboveWindowMatchesCond_dynamic(_Test_isWindowAboveWindowMatch Test fetching comparing the relative order of 2 windows, where a window moves during the operation. - This test models changes when performing a bi-direction search that expects the - start window to be before/above the end window. - By symmetry, the same test results are expected if the search goes the other way, - expecting it to return False, rather than True. - To model changes in z-order, a _MoveWindow is used to describe the change. - When the getWindow is called with the triggerIndex, the value at start index is moved - "in front" of the window at end index. Effectively this means that before getting the window at the triggerIndex, the order of windows will change. """ _queuedMove: Optional[_MoveWindow] = None def _getWindow_patched(self, hwnd: winUser.HWNDVal, relation: int) -> int: - self._moveIndexToNewIndexAtIndexOnce(self._windows.index(hwnd)) + self._triggerQueuedMove(hwnd) result = super()._getWindow_patched(hwnd, relation) return result - def _moveIndexToNewIndexAtIndexOnce(self, currentIndex: int): + def _triggerQueuedMove(self, currentHWND: winUser.HWNDVal): from logging import getLogger - getLogger().error(f"Current index {currentIndex}, {self._windows}") + getLogger().debug(f"Current HWND {currentHWND}, {self._windows}") if ( self._queuedMove - and currentIndex == self._queuedMove.triggerIndex + and currentHWND == self._queuedMove.triggerHWND and not self._queuedMove.triggered ): self._queuedMove.triggered = True - window = self._windows.pop(self._queuedMove.startIndex) - self._windows.insert(self._queuedMove.endIndex, window) + self._windows.remove(self._queuedMove.HWNDToMove) + insertIndex = self._windows.index(self._queuedMove.insertBelowHWND) + self._windows.insert(insertIndex, self._queuedMove.HWNDToMove) + + def _test_windowWithMove( + self, + move: _MoveWindow, + aboveWindow: winUser.HWNDVal, + belowWindow: winUser.HWNDVal, + aboveRaises: Optional[Type[Exception]] = None, + belowRaises: Optional[Type[Exception]] = None, + aboveExpectFailure: bool = False, + belowExpectFailure: bool = False, + ): + """ + Compares the relative z-order of two windows. + Checks the inverse behaviour: + i.e. swaps the order of the window, expect the result to be the opposite. + For the expected "above is true" case, if aboveRaises is provided, expect the exception. + If aboveExpectFailure is provided, expect the "above is false", i.e. an incorrect result. + Similarly is true for the "below is true" case. + @param move: move a window when another window is reached + @param aboveWindow: expected above window + @param belowWindow: expected below window + @param aboveRaises: if provided, expect the exception to be raised when checking the "above is true" + @param belowRaises: if provided, expect the exception to be raised when checking the "below is true" + @param aboveExpectFailure: if True, expect a failure when checking the "above is true" + @param belowExpectFailure: if True, expect a failure when checking the "below is true" + """ + self._queuedMove = move + + # Check aboveWindow is above belowWindow + isAbove = True + if aboveExpectFailure: + isAbove = not isAbove + if aboveRaises is None: + self.assertEqual(isAbove, _isWindowAboveWindowMatchesCond(aboveWindow, self._windowMatches(belowWindow))) + else: + with self.assertRaises(aboveRaises): + _isWindowAboveWindowMatchesCond(aboveWindow, self._windowMatches(belowWindow)) + + # Reset the window list + self._generateWindows() + # Reset the move + self._queuedMove.triggered = False + + # Check belowWindow is below aboveWindow + isAbove = False + if belowExpectFailure: + isAbove = not isAbove + if belowRaises is None: + self.assertEqual(isAbove, _isWindowAboveWindowMatchesCond(belowWindow, self._windowMatches(aboveWindow))) + else: + with self.assertRaises(belowRaises): + _isWindowAboveWindowMatchesCond(belowWindow, self._windowMatches(aboveWindow)) - def test_visited_windowMoves_pastTarget(self): + def test_visited_windowMoves_aboveTargets(self): """ - A visited window is moved past the target window. - This does not affect the relative z-order. + A visited window is moved above the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 5 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=6, - triggerIndex=4 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=3, + insertBelowHWND=6, + triggerHWND=4 + ), + aboveWindow=5, + belowWindow=2, ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_visited_windowMoves_beforeTarget(self): + def test_visited_windowMoves_betweenTargets(self): """ - A visited window is moved towards but before the target window. + A visited window is moved between the target windows. It is counted twice. - This does not affect the relative z-order. - """ - startWindow = 2 - targetWindow = 6 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=5, - triggerIndex=4 + This does not affect the relative z-order of the target windows. + """ + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=3, + insertBelowHWND=5, + triggerHWND=4 + ), + aboveWindow=6, + belowWindow=2 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_visited_windowMoves_awayFromTarget(self): + def test_visited_windowMoves_belowTargets(self): """ - A visited window is moved in the opposite direction from the target window. - This does not affect the relative z-order. + A visited window is moved in the opposite direction from the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 6 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=1, - triggerIndex=4 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=3, + insertBelowHWND=1, + triggerHWND=4 + ), + aboveWindow=5, + belowWindow=2, ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_active_windowMoves_pastTarget(self): + def test_active_windowMoves_betweenTargets(self): """ - A window we are currently visiting moves past the target window. - This causes the search to skip the target window. + A window we are currently visiting moves between target windows. + This causes the search to skip the first window. + This can cause a false negative (i.e. content becomes accessible when it should be secure). """ - startWindow = 2 - targetWindow = 5 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=7, - triggerIndex=3 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=3, + insertBelowHWND=8, + triggerHWND=3 + ), + aboveWindow=10, + belowWindow=6, + aboveRaises=_UnexpectedWindowCountError, # handled as if window is above + belowExpectFailure=True # handled as if window is above ) - with self.assertRaises(_WindowNotFoundError): - _isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow)) - def test_active_windowMoves_beforeTarget(self): + def test_active_windowMoves_beforeTargets(self): """ - A window we are currently visiting moves towards but before the target window. - This does not affect the relative z-order. + A window we are currently visiting moves before the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 10 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=5, - triggerIndex=3 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=3, + insertBelowHWND=5, + triggerHWND=3 + ), + aboveWindow=10, + belowWindow=6, ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_active_windowMoves_awayFromTarget(self): + def test_active_windowMoves_belowTargets(self): """ - A window we are currently visiting moves in the opposite direction to the target window. - This does not affect the relative z-order. + A window we are currently visiting moves in the opposite direction + from the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 10 - self._queuedMove = _MoveWindow( - startIndex=3, - endIndex=1, - triggerIndex=3 + self._test_windowWithMove( + _MoveWindow( + HWNDToMove=3, + insertBelowHWND=1, + triggerHWND=3 + ), + aboveWindow=10, + belowWindow=6 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_unvisited_windowMoves_pastTarget(self): + def test_unvisited_windowMoves_aboveTargets(self): """ - An unvisited window moves towards, and past the target window. - This does not affect the relative z-order. + An unvisited window moves above the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 6 - self._queuedMove = _MoveWindow( - startIndex=4, - endIndex=8, - triggerIndex=3 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=5, + insertBelowHWND=8, + triggerHWND=3 + ), + aboveWindow=6, + belowWindow=2 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_unvisited_windowMoves_beforeTarget(self): + def test_unvisited_windowMoves_betweenTargets(self): """ - An unvisited window moves towards, but before the target window. - This does not affect the relative z-order. + An unvisited window moves between the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 8 - self._queuedMove = _MoveWindow( - startIndex=4, - endIndex=6, - triggerIndex=3 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=4, + insertBelowHWND=6, + triggerHWND=3 + ), + aboveWindow=8, + belowWindow=2 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_unvisited_windowMoves_awayFromTarget(self): + def test_unvisited_windowMoves_belowTargets(self): """ - An unvisited window moves in the opposite direction the target window. - This does not affect the relative z-order. + An unvisited window moves in the opposite direction from the target windows. + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 8 - self._queuedMove = _MoveWindow( - startIndex=4, - endIndex=1, - triggerIndex=3 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=4, + insertBelowHWND=1, + triggerHWND=3 + ), + aboveWindow=8, + belowWindow=2 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_startWindow_windowMoves_pastTarget(self): + def test_belowWindow_windowMoves_aboveAboveWindow(self): """ - Start window moves towards, and past the target window. + Below window moves above the above window. This means the relative z-order has changed, but the change is not detected. - The relative z-order of the start of the search is returned. - """ - startWindow = 2 - targetWindow = 6 - self._queuedMove = _MoveWindow( - startIndex=startWindow - 1, - endIndex=8, - triggerIndex=3 + The initial relative z-order is returned. + """ + belowWindow = 2 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=belowWindow, + insertBelowHWND=8, + triggerHWND=3 + ), + aboveWindow=6, + belowWindow=belowWindow + ) + + def test_belowWindow_windowMoves_towardsAboveWindow(self): + """ + Below window moves towards the above window. + This causes the window to be counted twice. + This does not affect the relative z-order of the target windows. + """ + belowWindow = 2 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=belowWindow, + insertBelowHWND=6, + triggerHWND=3 + ), + aboveWindow=8, + belowWindow=belowWindow, + belowRaises=_UnexpectedWindowCountError, # handled as if window is above + ) + + def test_belowWindow_windowMoves_furtherBelow(self): + """ + The below window moves away from the above window. + This does not affect the relative z-order of the target windows. + """ + belowWindow = 2 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=belowWindow, + insertBelowHWND=1, + triggerHWND=3 + ), + aboveWindow=8, + belowWindow=belowWindow ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_startWindow_windowMoves_beforeTarget(self): + def test_aboveWindow_windowMoves_furtherAbove(self): """ - Start window moves towards, but before the target window. - This does not affect the relative z-order. + Above window moves further above + This does not affect the relative z-order of the target windows. """ - startWindow = 2 - targetWindow = 8 - self._queuedMove = _MoveWindow( - startIndex=startWindow - 1, - endIndex=6, - triggerIndex=3 + aboveWindow = 6 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=aboveWindow, + insertBelowHWND=8, + triggerHWND=3 + ), + aboveWindow=aboveWindow, + belowWindow=2 ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) - def test_StartWindow_windowMoves_awayFromTarget(self): + def test_aboveWindow_windowMoves_towardsBelowWindow(self): """ - Start window moves in the opposite direction the target window. - This does not affect the relative z-order. + Above window moves towards, but above the below window. + This does not affect the relative z-order of the target windows. + """ + aboveWindow = 8 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=aboveWindow, + insertBelowHWND=6, + triggerHWND=3 + ), + aboveWindow=aboveWindow, + belowWindow=2 + ) + + def test_aboveWindow_windowMoves_belowBelowWindow(self): + """ + Above window moves in the opposite direction of the above window. + This means the relative z-order has changed, but the change is not detected. + The initial relative z-order is returned. """ - startWindow = 2 - targetWindow = 8 - self._queuedMove = _MoveWindow( - startIndex=startWindow - 1, - endIndex=1, - triggerIndex=3 + aboveWindow = 8 + self._test_windowWithMove( + move=_MoveWindow( + HWNDToMove=aboveWindow, + insertBelowHWND=1, + triggerHWND=3 + ), + aboveWindow=aboveWindow, + belowWindow=2, + belowRaises=_UnexpectedWindowCountError # handled as if window is above ) - self.assertTrue(_isWindowAboveWindowMatchesCond(startWindow, self._windowMatches(targetWindow))) From 962c26fc9a1a38a15b7a61e2bca3579403ff8527 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Mon, 12 Dec 2022 15:45:08 +1100 Subject: [PATCH 14/20] fix up docstring --- tests/unit/test_util/test_security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_util/test_security.py b/tests/unit/test_util/test_security.py index aeecd30c4c8..8b761f77553 100644 --- a/tests/unit/test_util/test_security.py +++ b/tests/unit/test_util/test_security.py @@ -24,7 +24,7 @@ @dataclass class _MoveWindow: - """Used to move a window from one index to another when a specific index is reached.""" + """Used to move a window from one spot to another when a specific window is reached.""" HWNDToMove: winUser.HWNDVal # This window insertBelowHWND: winUser.HWNDVal # is moved to below this window triggerHWND: winUser.HWNDVal # when this window is reached From 7e99d817982fbb6bc09f1411e9c06e440b647d89 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Tue, 13 Dec 2022 11:40:12 +1100 Subject: [PATCH 15/20] address review comments --- source/NVDAObjects/__init__.py | 1 + source/api.py | 16 +++++++++++++--- source/textInfos/__init__.py | 3 +-- source/utils/security.py | 6 +++--- source/winAPI/sessionTracking.py | 30 +++++++++++++++++++++--------- source/winAPI/wtsApi32.py | 11 ++++++++++- 6 files changed, 49 insertions(+), 18 deletions(-) diff --git a/source/NVDAObjects/__init__.py b/source/NVDAObjects/__init__.py index 552f453bdde..fa71453697e 100644 --- a/source/NVDAObjects/__init__.py +++ b/source/NVDAObjects/__init__.py @@ -1441,4 +1441,5 @@ def getSelectedItemsCount(self,maxCount=2): def _get_isAboveLockScreen(self) -> bool: if not isWindowsLocked(): return True + # TODO time _isObjectAbovelockScreen return _isObjectAboveLockScreen(self) diff --git a/source/api.py b/source/api.py index 36a558f133c..9b5994bb4c8 100644 --- a/source/api.py +++ b/source/api.py @@ -238,10 +238,20 @@ def setReviewPosition( @param isMouse: Whether the review position is changed due to mouse following. """ reviewObj = reviewPosition.obj - if isinstance(reviewObj, treeInterceptorHandler.TreeInterceptor): + + if isinstance(reviewObj, treeInterceptorHandler.DocumentTreeInterceptor): + # reviewPosition.obj can be a number of classes, e.g. + # CursorManager, DocumentWithTableNavigation, EditableText. + # We can only handle the NVDAObject case. reviewObj = reviewObj.rootNVDAObject - if _isSecureObjectWhileLockScreenActivated(reviewObj): - return False + + if isinstance(reviewObj, NVDAObjects.NVDAObject): + # reviewPosition.obj can be a number of classes, e.g. + # CursorManager, DocumentWithTableNavigation, EditableText. + # We can only handle the NVDAObject case. + if _isSecureObjectWhileLockScreenActivated(reviewObj): + return False + globalVars.reviewPosition=reviewPosition.copy() globalVars.reviewPositionObj=reviewPosition.obj if clearNavigatorObject: globalVars.navigatorObject=None diff --git a/source/textInfos/__init__.py b/source/textInfos/__init__.py index 72023971c53..327c1d18082 100755 --- a/source/textInfos/__init__.py +++ b/source/textInfos/__init__.py @@ -312,12 +312,11 @@ class TextInfo(baseObject.AutoPropertyObject): def __init__( self, obj: "documentBase.TextContainerObject", - position + position: Union[int, Tuple, str], ): """Constructor. Subclasses must extend this, calling the superclass method first. @param position: The initial position of this range; one of the POSITION_* constants or a position object supported by the implementation. - @type position: int, tuple or string @param obj: The object containing the range of text being represented. """ super(TextInfo,self).__init__() diff --git a/source/utils/security.py b/source/utils/security.py index 1b160be786f..263bbc23f95 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -227,16 +227,16 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: # lockAppModule not running/registered by NVDA yet log.debug( "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is in lock app, considering object as safe. " + "Cannot detect if object is above lock app, considering object as safe. " ) return True from NVDAObjects.window import Window if not isinstance(obj, Window): log.debug( - "Cannot detect if object is in lock app, considering object as safe. " + "Cannot detect if object is above lock app, considering object as safe. " ) - # must be a window to get its HWNDVal + # Must be a window instance to get the HWNDVal, other NVDAObjects do not support this. return True topLevelWindowHandle = winUser.getAncestor(obj.windowHandle, winUser.GA_ROOT) diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index cdc5a1a93f1..336ebb5bcd5 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -64,7 +64,7 @@ """ _lockStateTracker: Optional["_WindowsLockedState"] = None -_windowsWasPreviouslyLocked = False +_wasLockedPreviousPumpAll = False class WindowsTrackedSession(enum.IntEnum): @@ -107,11 +107,11 @@ def initialize(): def pumpAll(): - global _windowsWasPreviouslyLocked + global _wasLockedPreviousPumpAll from utils.security import postSessionLockStateChanged windowsIsNowLocked = isWindowsLocked() - if windowsIsNowLocked != _windowsWasPreviouslyLocked: - _windowsWasPreviouslyLocked = windowsIsNowLocked + if windowsIsNowLocked != _wasLockedPreviousPumpAll: + _wasLockedPreviousPumpAll = windowsIsNowLocked postSessionLockStateChanged.notify(isNowLocked=windowsIsNowLocked) @@ -143,6 +143,7 @@ def _isWindowsLocked_checkViaSessionQuery() -> bool: try: sessionQueryLockState = _getSessionLockedValue() except RuntimeError: + log.exception("Failure querying session locked state") return False if sessionQueryLockState == WTS_LockState.WTS_SESSIONSTATE_UNKNOWN: log.error( @@ -160,7 +161,7 @@ def isLockStateSuccessfullyTracked() -> bool: """ # TODO: improve deprecation practice on beta/master merges log.error( - "NVDA no longer registers session tracking notifications. " + "NVDA no longer registers to receive session tracking notifications. " "This function is deprecated, for removal in 2023.1. " "It was never expected that add-on authors would use this function" ) @@ -170,7 +171,7 @@ def isLockStateSuccessfullyTracked() -> bool: def register(handle: int) -> bool: # TODO: improve deprecation practice on beta/master merges log.error( - "NVDA no longer registers session tracking notifications. " + "NVDA no longer registers to receive session tracking notifications. " "This function is deprecated, for removal in 2023.1. " "It was never expected that add-on authors would use this function" ) @@ -180,7 +181,7 @@ def register(handle: int) -> bool: def unregister(handle: HWND) -> None: # TODO: improve deprecation practice on beta/master merges log.error( - "NVDA no longer registers session tracking notifications. " + "NVDA no longer registers to receive session tracking notifications. " "This function is deprecated, for removal in 2023.1. " "It was never expected that add-on authors would use this function" ) @@ -189,7 +190,7 @@ def unregister(handle: HWND) -> None: def handleSessionChange(newState: WindowsTrackedSession, sessionId: int) -> None: # TODO: improve deprecation practice on beta/master merges log.error( - "NVDA no longer registers session tracking notifications. " + "NVDA no longer registers to receive session tracking notifications. " "This function is deprecated, for removal in 2023.1. " "It was never expected that add-on authors would use this function" ) @@ -199,6 +200,7 @@ def handleSessionChange(newState: WindowsTrackedSession, sessionId: int) -> None def WTSCurrentSessionInfoEx() -> contextlib.AbstractContextManager[ctypes.pointer[WTSINFOEXW]]: """Context manager to get the WTSINFOEXW for the current server/session or raises a RuntimeError. Handles freeing the memory when usage is complete. + @raises RuntimeError: On failure """ info = _getCurrentSessionInfoEx() try: @@ -210,11 +212,13 @@ def WTSCurrentSessionInfoEx() -> contextlib.AbstractContextManager[ctypes.pointe def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): - """ Gets the WTSINFOEXW for the current server/session or raises a RuntimeError + """ + Gets the WTSINFOEXW for the current server/session or raises a RuntimeError on failure. On RuntimeError memory is first freed. In other cases use WTSFreeMemory. Ideally use the WTSCurrentSessionInfoEx context manager which will handle freeing the memory. + @raises RuntimeError: On failure """ ppBuffer = ctypes.wintypes.LPWSTR(None) pBytesReturned = ctypes.wintypes.DWORD(0) @@ -264,6 +268,7 @@ def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): def _getSessionLockedValue() -> WTS_LockState: """Get the WTS_LockState for the current server/session or raises a RuntimeError + @raises RuntimeError: if fetching the session info fails. """ with WTSCurrentSessionInfoEx() as info: infoEx: WTSINFOEX_LEVEL1_W = info.contents.Data.WTSInfoExLevel1 @@ -271,5 +276,12 @@ def _getSessionLockedValue() -> WTS_LockState: try: lockState = WTS_LockState(sessionFlags) except ValueError: + # If an unexpected flag value is provided, + # the WTS_LockState enum will not be constructed and will raise ValueError. + # In some cases sessionFlags=-0x1 is returned (#14379). + # Also, SessionFlags returns a flag state. This means that the following is a valid result: + # sessionFlags=WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. + # As mixed states imply an unknown state, + # WTS_LockState is an IntEnum rather than an IntFlag and mixed state flags are unexpected enum values. return WTS_LockState.WTS_SESSIONSTATE_UNKNOWN return lockState diff --git a/source/winAPI/wtsApi32.py b/source/winAPI/wtsApi32.py index 47e9a80cd4a..6071f757a51 100644 --- a/source/winAPI/wtsApi32.py +++ b/source/winAPI/wtsApi32.py @@ -178,7 +178,16 @@ class WTSINFOEXW(ctypes.Structure): class _WTS_LockState(IntEnum): - """WtsApi32.h#L437""" + """ + In some cases, -0x1 is also a known state returned when queried (#14379). + WTSINFOEX_LEVEL1_W.SessionFlags returns a flag state. + This means that the following is a valid state according to the winAPI: + WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. + As mixed states imply an unknown state, + WTS_LockState is an IntEnum rather than an IntFlag and mixed state flags are unexpected enum values. + + WtsApi32.h#L437 + """ WTS_SESSIONSTATE_UNKNOWN = 0xFFFFFFFF # dec(4294967295) """The session state is not known.""" From 828ccea5ae44b54a380052cdfb767c3966598487 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Tue, 13 Dec 2022 14:36:25 +1100 Subject: [PATCH 16/20] minor further fixups --- source/api.py | 2 + source/utils/security.py | 22 +++++--- source/winAPI/{wtsApi32.py => _wtsApi32.py} | 29 ++++++---- source/winAPI/sessionTracking.py | 59 ++++++++++++++------- 4 files changed, 76 insertions(+), 36 deletions(-) rename source/winAPI/{wtsApi32.py => _wtsApi32.py} (85%) diff --git a/source/api.py b/source/api.py index 9b5994bb4c8..434d4110dc3 100644 --- a/source/api.py +++ b/source/api.py @@ -251,6 +251,8 @@ def setReviewPosition( # We can only handle the NVDAObject case. if _isSecureObjectWhileLockScreenActivated(reviewObj): return False + else: + log.debug(f"Unhandled reviewObj type {type(reviewObj)} when checking security of reviewObj") globalVars.reviewPosition=reviewPosition.copy() globalVars.reviewPositionObj=reviewPosition.obj diff --git a/source/utils/security.py b/source/utils/security.py index 263bbc23f95..ceefe865c0e 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -142,7 +142,7 @@ def _isSecureObjectWhileLockScreenActivated( """ While Windows is locked, Windows 10 and 11 doesn't prevent object navigation outside of the lockscreen. As such, NVDA must prevent accessing and reading objects outside of the lockscreen when Windows is locked. - @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is outside of the lock screen. + @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is below the lock screen. """ try: isObjectBelowLockScreen = isWindowsLocked() and not obj.isAboveLockScreen @@ -169,6 +169,10 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: def _checkWindowsForAppModules(): + """ + Updates the appModuleHandler with the process from all top level windows. + Adds any missing processes. + """ from ctypes import c_bool, WINFUNCTYPE, c_int, POINTER, windll # TODO move and order from ctypes.wintypes import LPARAM import appModuleHandler @@ -180,11 +184,12 @@ def _checkWindowsForAppModules(): @WINFUNCTYPE(c_bool, c_int, POINTER(c_int)) def _appModuleHandlerUpdate(hwnd: winUser.HWNDVal, _lParam: LPARAM) -> bool: processID, _threadID = winUser.getWindowThreadProcessID(hwnd) - appModuleHandler.update(processID) + if processID not in appModuleHandler.runningTable: + appModuleHandler.update(processID) return True if not windll.user32.EnumWindows(_appModuleHandlerUpdate, 0): - log.error("Failed search for lockapp App Module") + log.error("Failed to refresh app modules") def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: @@ -196,7 +201,7 @@ def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, - but other Windows can be focused (e.g. Windows Magnifier). + but other Windows can be focused (e.g. Windows Magnifier, reset PIN workflow). """ from IAccessibleHandler import SecureDesktopNVDAObject from NVDAObjects.IAccessible import TaskListIcon @@ -240,7 +245,6 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: return True topLevelWindowHandle = winUser.getAncestor(obj.windowHandle, winUser.GA_ROOT) - # TODO: time this return _isObjectAboveLockScreenCheckZOrder(topLevelWindowHandle, lockAppModule.processID) @@ -268,7 +272,7 @@ def _isWindowLockApp(hwnd: winUser.HWNDVal) -> bool: class _UnexpectedWindowCountError(Exception): """ Raised when a window which matches the expected condition - is not found by _isWindowAboveWindowMatchesCond + is not found by _isWindowAboveWindowMatchesCond """ pass @@ -277,7 +281,11 @@ def _isWindowAboveWindowMatchesCond( window: winUser.HWNDVal, matchCond: Callable[[winUser.HWNDVal], bool] ) -> bool: - """ Returns True if window is above a window that matches matchCond. + """ + This is a risky hack. + The order may be incorrectly detected. + + @returns: True if window is above a window that matches matchCond. If the first window is not found, but the second window is, it is assumed that the first window is above the second window. """ diff --git a/source/winAPI/wtsApi32.py b/source/winAPI/_wtsApi32.py similarity index 85% rename from source/winAPI/wtsApi32.py rename to source/winAPI/_wtsApi32.py index 6071f757a51..1caf9794af7 100644 --- a/source/winAPI/wtsApi32.py +++ b/source/winAPI/_wtsApi32.py @@ -11,7 +11,7 @@ """ from enum import ( - IntEnum + IntEnum, ) from typing import ( Callable, @@ -133,6 +133,8 @@ class WTSINFOEX_LEVEL1_W(ctypes.Structure): ("OutgoingCompressedBytes", DWORD), ) + SessionFlags: LONG + class WTSINFOEX_LEVEL_W(ctypes.Union): """ WtsApi32.h#L483 @@ -142,6 +144,8 @@ class WTSINFOEX_LEVEL_W(ctypes.Union): ("WTSInfoExLevel1", WTSINFOEX_LEVEL1_W), ) + WTSInfoExLevel1: WTSINFOEX_LEVEL1_W + class WTSINFOEXW(ctypes.Structure): """WtsApi32.h#L491 @@ -152,6 +156,9 @@ class WTSINFOEXW(ctypes.Structure): ('Data', WTSINFOEX_LEVEL_W), ) + Level: DWORD + Data: WTSINFOEX_LEVEL_W + # WTSQuerySessionInformationW Definition # WtsApi32.h#L1011 @@ -179,13 +186,6 @@ class WTSINFOEXW(ctypes.Structure): class _WTS_LockState(IntEnum): """ - In some cases, -0x1 is also a known state returned when queried (#14379). - WTSINFOEX_LEVEL1_W.SessionFlags returns a flag state. - This means that the following is a valid state according to the winAPI: - WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. - As mixed states imply an unknown state, - WTS_LockState is an IntEnum rather than an IntFlag and mixed state flags are unexpected enum values. - WtsApi32.h#L437 """ WTS_SESSIONSTATE_UNKNOWN = 0xFFFFFFFF # dec(4294967295) @@ -221,6 +221,15 @@ def _setWTS_LockState() -> Type[Union[_WTS_LockState, _WTS_LockState_Win7]]: return _WTS_LockState_Win7 if (winVersion.getWinVer() < winVersion.WIN8) else _WTS_LockState -WTS_LockState: Type[_WTS_LockState] = _setWTS_LockState() -"""Contains WTS_SESSIONSTATE_LOCK, WTS_SESSIONSTATE_UNLOCK, WTS_SESSIONSTATE_UNKNOWN +WTS_LockState: Type[Union[_WTS_LockState, _WTS_LockState_Win7]] = _setWTS_LockState() +""" +Set of known session states that NVDA can handle. +These values are different on different versions of Windows. + +In some cases, other states such as -0x1 are returned when queried (#14379). +Generally, WTSINFOEX_LEVEL1_W.SessionFlags returns a flag state. +This means that the following is a valid state according to the winAPI: +WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. +As mixed states imply an unknown state, +_WTS_LockStateT is an IntEnum rather than an IntFlag and mixed state flags are unexpected enum values. """ diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 336ebb5bcd5..69cc04ff693 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -15,19 +15,21 @@ """ from __future__ import annotations -import contextlib import ctypes from contextlib import contextmanager from ctypes.wintypes import ( + DWORD, HWND, + LPWSTR, ) import enum from typing import ( + Generator, Optional, ) from baseObject import AutoPropertyObject -from winAPI.wtsApi32 import ( +from winAPI._wtsApi32 import ( WTSINFOEXW, WTSQuerySessionInformation, WTS_CURRENT_SERVER_HANDLE, @@ -35,7 +37,6 @@ WTS_INFO_CLASS, WTSFreeMemory, WTS_LockState, - WTSINFOEX_LEVEL1_W, ) from logHandler import log @@ -64,7 +65,14 @@ """ _lockStateTracker: Optional["_WindowsLockedState"] = None +""" +Caches the Windows lock state as an auto property object. +""" _wasLockedPreviousPumpAll = False +""" +Each core pump cycle, the Windows lock state is updated. +The previous value is tracked, so that changes to the lock state can be detected. +""" class WindowsTrackedSession(enum.IntEnum): @@ -75,6 +83,8 @@ class WindowsTrackedSession(enum.IntEnum): Values from: https://learn.microsoft.com/en-us/windows/win32/termserv/wm-wtssession-change Context: https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification + + Unused in NVDA core. """ CONSOLE_CONNECT = 1 CONSOLE_DISCONNECT = 2 @@ -90,6 +100,9 @@ class WindowsTrackedSession(enum.IntEnum): class _WindowsLockedState(AutoPropertyObject): + """ + Class to encapsulate caching the Windows lock state. + """ # Refer to AutoPropertyObject for notes on caching _cache_isWindowsLocked = True @@ -107,6 +120,7 @@ def initialize(): def pumpAll(): + """Used to track the session lock state every core cycle, and detect changes.""" global _wasLockedPreviousPumpAll from utils.security import postSessionLockStateChanged windowsIsNowLocked = isWindowsLocked() @@ -117,8 +131,12 @@ def pumpAll(): def isWindowsLocked() -> bool: """ + TODO: rename to isWindowsLockscreenActive + Checks if the Window lockscreen is active. Not to be confused with the Windows sign-in screen, a secure screen. + Includes temporary locked desktops, + such as the PIN workflow reset and the Out Of Box Experience. """ from core import _TrackNVDAInitialization from systemUtils import _isSecureDesktop @@ -137,7 +155,7 @@ def isWindowsLocked() -> bool: def _isWindowsLocked_checkViaSessionQuery() -> bool: """ Use a session query to check if the session is locked - @return: True is the session is locked. + @returns: True is the session is locked. Also returns False if the lock state can not be determined via a Session Query. """ try: @@ -155,10 +173,6 @@ def _isWindowsLocked_checkViaSessionQuery() -> bool: def isLockStateSuccessfullyTracked() -> bool: - """Check if the lock state is successfully tracked. - I.E. Registered for session tracking AND initial value set correctly. - @return: True when successfully tracked. - """ # TODO: improve deprecation practice on beta/master merges log.error( "NVDA no longer registers to receive session tracking notifications. " @@ -196,13 +210,18 @@ def handleSessionChange(newState: WindowsTrackedSession, sessionId: int) -> None ) +_WTS_INFO_POINTER_T = ctypes.POINTER(WTSINFOEXW) + + @contextmanager -def WTSCurrentSessionInfoEx() -> contextlib.AbstractContextManager[ctypes.pointer[WTSINFOEXW]]: +def WTSCurrentSessionInfoEx() -> Generator[_WTS_INFO_POINTER_T, None, None]: """Context manager to get the WTSINFOEXW for the current server/session or raises a RuntimeError. Handles freeing the memory when usage is complete. @raises RuntimeError: On failure """ info = _getCurrentSessionInfoEx() + if info is None: + return try: yield info finally: @@ -211,7 +230,7 @@ def WTSCurrentSessionInfoEx() -> contextlib.AbstractContextManager[ctypes.pointe ) -def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): +def _getCurrentSessionInfoEx() -> Optional[_WTS_INFO_POINTER_T]: """ Gets the WTSINFOEXW for the current server/session or raises a RuntimeError on failure. @@ -220,8 +239,9 @@ def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): Ideally use the WTSCurrentSessionInfoEx context manager which will handle freeing the memory. @raises RuntimeError: On failure """ - ppBuffer = ctypes.wintypes.LPWSTR(None) - pBytesReturned = ctypes.wintypes.DWORD(0) + ppBuffer = LPWSTR(None) + pBytesReturned = DWORD(0) + info = None res = WTSQuerySessionInformation( WTS_CURRENT_SERVER_HANDLE, # WTS_CURRENT_SERVER_HANDLE to indicate the RD Session Host server on @@ -246,7 +266,7 @@ def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): ) info = ctypes.cast( ppBuffer, - ctypes.POINTER(WTSINFOEXW) + _WTS_INFO_POINTER_T ) if ( not info.contents @@ -261,26 +281,27 @@ def _getCurrentSessionInfoEx() -> ctypes.POINTER(WTSINFOEXW): return info except Exception as e: log.exception("Unexpected WTSQuerySessionInformation value:", exc_info=e) - WTSFreeMemory( + WTSFreeMemory( # should this be moved to a finally block? ctypes.cast(ppBuffer, ctypes.c_void_p), ) + return None def _getSessionLockedValue() -> WTS_LockState: - """Get the WTS_LockState for the current server/session or raises a RuntimeError + """Get the WTS_LockState for the current server/session. @raises RuntimeError: if fetching the session info fails. """ with WTSCurrentSessionInfoEx() as info: - infoEx: WTSINFOEX_LEVEL1_W = info.contents.Data.WTSInfoExLevel1 - sessionFlags: ctypes.wintypes.LONG = infoEx.SessionFlags + infoEx = info.contents.Data.WTSInfoExLevel1 + sessionFlags = infoEx.SessionFlags try: lockState = WTS_LockState(sessionFlags) except ValueError: # If an unexpected flag value is provided, # the WTS_LockState enum will not be constructed and will raise ValueError. - # In some cases sessionFlags=-0x1 is returned (#14379). + # In some cases sessionFlags = -0x1 is returned (#14379). # Also, SessionFlags returns a flag state. This means that the following is a valid result: - # sessionFlags=WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. + # sessionFlags = WTS_SESSIONSTATE_UNKNOWN | WTS_SESSIONSTATE_LOCK | WTS_SESSIONSTATE_UNLOCK. # As mixed states imply an unknown state, # WTS_LockState is an IntEnum rather than an IntFlag and mixed state flags are unexpected enum values. return WTS_LockState.WTS_SESSIONSTATE_UNKNOWN From 544fa7afb35d7747d933a11b03b236de52999798 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Wed, 14 Dec 2022 14:01:47 +1100 Subject: [PATCH 17/20] fix up todos --- source/NVDAHelper.py | 6 ++-- source/NVDAObjects/__init__.py | 7 ++-- source/appModuleHandler.py | 25 +++++++++++++++ source/appModules/lockapp.py | 10 +++--- source/brailleViewer/brailleViewerGui.py | 8 ++--- source/core.py | 4 +-- source/scriptHandler.py | 4 +-- source/speechViewer.py | 6 ++-- source/utils/security.py | 41 +++++++++--------------- source/winAPI/sessionTracking.py | 37 ++++++++++++++------- 10 files changed, 88 insertions(+), 60 deletions(-) diff --git a/source/NVDAHelper.py b/source/NVDAHelper.py index 759c3cd5db4..536d2e2fe5f 100755 --- a/source/NVDAHelper.py +++ b/source/NVDAHelper.py @@ -28,7 +28,7 @@ import api import globalVars from logHandler import log -from utils.security import isWindowsLocked +from utils.security import _isLockScreenModeActive versionedLibPath = os.path.join(globalVars.appDir, 'lib') if os.environ.get('PROCESSOR_ARCHITEW6432') == 'ARM64': @@ -455,7 +455,7 @@ def nvdaControllerInternal_installAddonPackageFromPath(addonPath): if globalVars.appArgs.secure: log.debugWarning("Unable to install add-on into secure copy of NVDA.") return - if isWindowsLocked(): + if _isLockScreenModeActive(): log.debugWarning("Unable to install add-on while Windows is locked.") return import wx @@ -470,7 +470,7 @@ def nvdaControllerInternal_openConfigDirectory(): if globalVars.appArgs.secure: log.debugWarning("Unable to open user config directory for secure copy of NVDA.") return - if isWindowsLocked(): + if _isLockScreenModeActive(): log.debugWarning("Unable to open user config directory while Windows is locked.") return import systemUtils diff --git a/source/NVDAObjects/__init__.py b/source/NVDAObjects/__init__.py index fa71453697e..23a66ff2ae5 100644 --- a/source/NVDAObjects/__init__.py +++ b/source/NVDAObjects/__init__.py @@ -36,7 +36,7 @@ import brailleInput import locationHelper import aria -from winAPI.sessionTracking import isWindowsLocked +from winAPI.sessionTracking import _isLockScreenModeActive class NVDAObjectTextInfo(textInfos.offsets.OffsetsTextInfo): @@ -182,7 +182,7 @@ def _insertLockScreenObject(self, clsList: typing.List["NVDAObject"]) -> None: Inserts LockScreenObject to the start of the clsList if Windows is locked. """ from .lockscreen import LockScreenObject - if isWindowsLocked(): + if _isLockScreenModeActive(): # This must be resolved first to prevent object navigation outside of the lockscreen. clsList.insert(0, LockScreenObject) @@ -1439,7 +1439,6 @@ def getSelectedItemsCount(self,maxCount=2): isAboveLockScreen: bool def _get_isAboveLockScreen(self) -> bool: - if not isWindowsLocked(): + if not _isLockScreenModeActive(): return True - # TODO time _isObjectAbovelockScreen return _isObjectAboveLockScreen(self) diff --git a/source/appModuleHandler.py b/source/appModuleHandler.py index 339134b5506..6f7561991c5 100644 --- a/source/appModuleHandler.py +++ b/source/appModuleHandler.py @@ -832,3 +832,28 @@ def getWmiProcessInfo(processId): except: raise LookupError("Couldn't get process information using WMI") raise LookupError("No such process") + + +def _checkWindowsForAppModules(): + """ + Updates the appModuleHandler with the process from all top level windows. + Adds any missing processes. + """ + import winUser + + # BOOL CALLBACK EnumWindowsProc _In_ HWND,_In_ LPARAM + # HWND as a pointer creates confusion, treat as an int + # http://makble.com/the-story-of-lpclong + + @ctypes.WINFUNCTYPE(ctypes.c_bool, ctypes.c_int, ctypes.POINTER(ctypes.c_int)) + def _appModuleHandlerUpdate( + hwnd: winUser.HWNDVal, + _lParam: ctypes.wintypes.LPARAM + ) -> bool: + processID, _threadID = winUser.getWindowThreadProcessID(hwnd) + if processID not in runningTable: + update(processID) + return True + + if not ctypes.windll.user32.EnumWindows(_appModuleHandlerUpdate, 0): + log.error("Failed to refresh app modules") diff --git a/source/appModules/lockapp.py b/source/appModules/lockapp.py index 2931df841a1..54faa2767e9 100644 --- a/source/appModules/lockapp.py +++ b/source/appModules/lockapp.py @@ -21,13 +21,13 @@ from NVDAObjects.lockscreen import LockScreenObject from NVDAObjects.UIA import UIA from utils.security import getSafeScripts -from winAPI.sessionTracking import isWindowsLocked +from winAPI.sessionTracking import _isLockScreenModeActive """App module for the Windows 10 and 11 lock screen. The lock screen allows other windows to be opened, so security related functions are done at a higher level than the lockapp app module. -Refer to usages of `winAPI.sessionTracking.isWindowsLocked`. +Refer to usages of `winAPI.sessionTracking._isLockScreenModeActive`. """ @@ -60,11 +60,11 @@ def chooseNVDAObjectOverlayClasses( if isinstance(obj,UIA) and obj.role==controlTypes.Role.PANE and obj.UIAElement.cachedClassName=="LockAppContainer": clsList.insert(0,LockAppContainer) - if not isWindowsLocked(): + if not _isLockScreenModeActive(): log.debugWarning( "LockApp is being initialized but NVDA does not expect Windows to be locked. " "DynamicNVDAObjectType may have failed to apply LockScreenObject. " - "This means Windows session tracking has failed or NVDA is yet to receive lock event. " + "This means session lock state tracking has failed. " ) clsList.insert(0, LockScreenObject) @@ -83,7 +83,7 @@ def _inputCaptor(self, gesture: inputCore.InputGesture) -> bool: if not scriptShouldRun: log.error( "scriptHandler failed to block script when Windows is locked. " - "This means Windows session tracking has failed. " + "This means session lock state tracking has failed. " ) return scriptShouldRun diff --git a/source/brailleViewer/brailleViewerGui.py b/source/brailleViewer/brailleViewerGui.py index 5d005181f08..a6e3796a039 100644 --- a/source/brailleViewer/brailleViewerGui.py +++ b/source/brailleViewer/brailleViewerGui.py @@ -17,7 +17,7 @@ import fonts import inputCore import gui.contextHelp -from utils.security import isWindowsLocked, postSessionLockStateChanged +from utils.security import _isLockScreenModeActive, postSessionLockStateChanged BRAILLE_UNICODE_PATTERNS_START = 0x2800 BRAILLE_SPACE_CHARACTER = chr(BRAILLE_UNICODE_PATTERNS_START) @@ -398,7 +398,7 @@ def _createControls(self, sizer: wx.Sizer, parent: wx.Control) -> None: self._shouldShowOnStartupCheckBox.SetValue(config.conf["brailleViewer"]["showBrailleViewerAtStartup"]) self._shouldShowOnStartupCheckBox.Bind(wx.EVT_CHECKBOX, self._onShouldShowOnStartupChanged) optionsSizer.Add(self._shouldShowOnStartupCheckBox) - if isWindowsLocked(): + if _isLockScreenModeActive(): self._shouldShowOnStartupCheckBox.Disable() # Translators: The label for a setting in the braille viewer that controls @@ -415,11 +415,11 @@ def _createControls(self, sizer: wx.Sizer, parent: wx.Control) -> None: sizer.Add(optionsSizer, flag=wx.EXPAND | wx.TOP, border=5) def _onShouldShowOnStartupChanged(self, evt: wx.CommandEvent): - if not isWindowsLocked(): + if not _isLockScreenModeActive(): config.conf["brailleViewer"]["showBrailleViewerAtStartup"] = self._shouldShowOnStartupCheckBox.IsChecked() def _onShouldHoverRouteToCellCheckBoxChanged(self, evt: wx.CommandEvent): - if not isWindowsLocked(): + if not _isLockScreenModeActive(): config.conf["brailleViewer"]["shouldHoverRouteToCell"] = self._shouldHoverRouteToCellCheckBox.IsChecked() self._updateMouseOverBinding(self._shouldHoverRouteToCellCheckBox.IsChecked()) diff --git a/source/core.py b/source/core.py index 6ca1d8433a3..d0461f7c925 100644 --- a/source/core.py +++ b/source/core.py @@ -451,13 +451,13 @@ class _TrackNVDAInitialization: regardless of lock state. Security checks may cause the desktop object to not be set if NVDA starts on the lock screen. As such, during initialization, NVDA should behave as if Windows is unlocked, - i.e. winAPI.sessionTracking.isWindowsLocked should return False. + i.e. winAPI.sessionTracking._isLockScreenModeActive should return False. TODO: move to NVDAState module """ _isNVDAInitialized = False - """When False, isWindowsLocked is forced to return False. + """When False, _isLockScreenModeActive is forced to return False. """ @staticmethod diff --git a/source/scriptHandler.py b/source/scriptHandler.py index 803984e5304..5136ec56566 100644 --- a/source/scriptHandler.py +++ b/source/scriptHandler.py @@ -95,11 +95,11 @@ def getGlobalMapScripts(gesture: "inputCore.InputGesture") -> List["inputCore.In def findScript(gesture: "inputCore.InputGesture") -> Optional[_ScriptFunctionT]: from utils.security import getSafeScripts - from winAPI.sessionTracking import isWindowsLocked + from winAPI.sessionTracking import _isLockScreenModeActive foundScript = _findScript(gesture) if ( foundScript is not None - and isWindowsLocked() + and _isLockScreenModeActive() and foundScript not in getSafeScripts() ): return None diff --git a/source/speechViewer.py b/source/speechViewer.py index 42690d77728..87d9c98dd45 100644 --- a/source/speechViewer.py +++ b/source/speechViewer.py @@ -13,7 +13,7 @@ from logHandler import log from speech import SpeechSequence import gui.contextHelp -from utils.security import isWindowsLocked, postSessionLockStateChanged +from utils.security import _isLockScreenModeActive, postSessionLockStateChanged # Inherit from wx.Frame because these windows show in the alt+tab menu (where miniFrame does not) @@ -102,7 +102,7 @@ def _createControls(self, sizer, parent): wx.EVT_CHECKBOX, self.onShouldShowOnStartupChanged ) - if isWindowsLocked(): + if _isLockScreenModeActive(): self.shouldShowOnStartupCheckBox.Disable() def _onDialogActivated(self, evt): @@ -119,7 +119,7 @@ def onClose(self, evt): deactivate() def onShouldShowOnStartupChanged(self, evt: wx.CommandEvent): - if not isWindowsLocked(): + if not _isLockScreenModeActive(): config.conf["speechViewer"]["showSpeechViewerAtStartup"] = self.shouldShowOnStartupCheckBox.IsChecked() _isDestroyed: bool diff --git a/source/utils/security.py b/source/utils/security.py index ceefe865c0e..ec21beb4748 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -13,7 +13,7 @@ import extensionPoints from logHandler import log -from winAPI.sessionTracking import isWindowsLocked +from winAPI.sessionTracking import _isLockScreenModeActive import winUser if typing.TYPE_CHECKING: @@ -145,7 +145,7 @@ def _isSecureObjectWhileLockScreenActivated( @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is below the lock screen. """ try: - isObjectBelowLockScreen = isWindowsLocked() and not obj.isAboveLockScreen + isObjectBelowLockScreen = _isLockScreenModeActive() and not obj.isAboveLockScreen except Exception: log.exception() return False @@ -168,30 +168,6 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: return obj.isAboveLockScreen -def _checkWindowsForAppModules(): - """ - Updates the appModuleHandler with the process from all top level windows. - Adds any missing processes. - """ - from ctypes import c_bool, WINFUNCTYPE, c_int, POINTER, windll # TODO move and order - from ctypes.wintypes import LPARAM - import appModuleHandler - - # BOOL CALLBACK EnumWindowsProc _In_ HWND,_In_ LPARAM - # HWND as a pointer creates confusion, treat as an int - # http://makble.com/the-story-of-lpclong - - @WINFUNCTYPE(c_bool, c_int, POINTER(c_int)) - def _appModuleHandlerUpdate(hwnd: winUser.HWNDVal, _lParam: LPARAM) -> bool: - processID, _threadID = winUser.getWindowThreadProcessID(hwnd) - if processID not in appModuleHandler.runningTable: - appModuleHandler.update(processID) - return True - - if not windll.user32.EnumWindows(_appModuleHandlerUpdate, 0): - log.error("Failed to refresh app modules") - - def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: import appModuleHandler runningAppModules = appModuleHandler.runningTable.values() @@ -224,6 +200,7 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: ): return True + from appModuleHandler import _checkWindowsForAppModules lockAppModule = _findLockAppModule() if lockAppModule is None: _checkWindowsForAppModules() @@ -288,6 +265,18 @@ def _isWindowAboveWindowMatchesCond( @returns: True if window is above a window that matches matchCond. If the first window is not found, but the second window is, it is assumed that the first window is above the second window. + + In the context of _isObjectAboveLockScreenCheckZOrder, NVDA starts at the lowest window, + and searches up towards the closest/lowest lock screen window. + If the lock screen window is found before the NVDAobject, the object should be made accessible. + This is because if the NVDAObject is not present, we want to make it accessible by default. + If the lock screen window is not present, we also want to make the NVDAObject accessible, + so the lock screen window must be comprehensively searched for. + If the NVDAObject is found, and then a lock screen window, + the object is not made accessible as it is below the lock screen. + Edge cases and failures should be handled by making the object accessible. + + Refer to test_security for testing cases which demonstrate this behaviour. """ desktopWindow = winUser.getDesktopWindow() topLevelWindow = winUser.getTopWindow(desktopWindow) diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 69cc04ff693..842c97bbe2a 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -123,26 +123,27 @@ def pumpAll(): """Used to track the session lock state every core cycle, and detect changes.""" global _wasLockedPreviousPumpAll from utils.security import postSessionLockStateChanged - windowsIsNowLocked = isWindowsLocked() + windowsIsNowLocked = _isWindowsLocked() if windowsIsNowLocked != _wasLockedPreviousPumpAll: _wasLockedPreviousPumpAll = windowsIsNowLocked postSessionLockStateChanged.notify(isNowLocked=windowsIsNowLocked) def isWindowsLocked() -> bool: - """ - TODO: rename to isWindowsLockscreenActive + # TODO: improve deprecation practice on beta/master merges + log.error( + "This function is deprecated, for removal in 2023.1. " + "It was never expected that add-on authors would use this function" + ) + return _isWindowsLocked() - Checks if the Window lockscreen is active. - Not to be confused with the Windows sign-in screen, a secure screen. - Includes temporary locked desktops, - such as the PIN workflow reset and the Out Of Box Experience. - """ + +def _isWindowsLocked() -> bool: from core import _TrackNVDAInitialization - from systemUtils import _isSecureDesktop if not _TrackNVDAInitialization.isInitializationComplete(): - return False - if _isSecureDesktop(): + # Wait until initialization is complete, + # so NVDA and other consumers can register the lock state + # via postSessionLockStateChanged. return False if _lockStateTracker is None: log.error( @@ -153,6 +154,20 @@ def isWindowsLocked() -> bool: return _lockStateTracker.isWindowsLocked +def _isLockScreenModeActive() -> bool: + """ + Checks if the Window lock screen is active. + Not to be confused with the Windows sign-in screen, a secure screen. + Includes temporary locked desktops, + such as the PIN workflow reset and the Out Of Box Experience. + """ + from systemUtils import _isSecureDesktop + if _isSecureDesktop(): + # Use secure mode instead if on the secure desktop + return False + return _isWindowsLocked() + + def _isWindowsLocked_checkViaSessionQuery() -> bool: """ Use a session query to check if the session is locked @returns: True is the session is locked. From 97d86ad2d2ebfbf375f37a49b7913918bdda6934 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 15 Dec 2022 12:10:23 +1100 Subject: [PATCH 18/20] search for lock app appmodule in core pump --- source/utils/security.py | 25 +++++++++++++++---------- source/winAPI/sessionTracking.py | 11 ++++++++++- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/source/utils/security.py b/source/utils/security.py index ec21beb4748..03316616203 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -174,6 +174,14 @@ def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: return next(filter(_isLockAppAndAlive, runningAppModules), None) +def _searchForLockAppModule(): + from appModuleHandler import _checkWindowsForAppModules + if _isLockScreenModeActive(): + lockAppModule = _findLockAppModule() + if lockAppModule is None: + _checkWindowsForAppModules() + + def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, @@ -200,18 +208,15 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: ): return True - from appModuleHandler import _checkWindowsForAppModules lockAppModule = _findLockAppModule() if lockAppModule is None: - _checkWindowsForAppModules() - lockAppModule = _findLockAppModule() - if lockAppModule is None: - # lockAppModule not running/registered by NVDA yet - log.debug( - "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is above lock app, considering object as safe. " - ) - return True + # lockAppModule not running/registered by NVDA yet. + # This is not an expected state. + log.error( + "lockAppModule not detected when Windows is locked. " + "Cannot detect if object is above lock app, considering object as safe. " + ) + return True from NVDAObjects.window import Window if not isinstance(obj, Window): diff --git a/source/winAPI/sessionTracking.py b/source/winAPI/sessionTracking.py index 842c97bbe2a..61f399582aa 100644 --- a/source/winAPI/sessionTracking.py +++ b/source/winAPI/sessionTracking.py @@ -122,11 +122,12 @@ def initialize(): def pumpAll(): """Used to track the session lock state every core cycle, and detect changes.""" global _wasLockedPreviousPumpAll - from utils.security import postSessionLockStateChanged + from utils.security import postSessionLockStateChanged, _searchForLockAppModule windowsIsNowLocked = _isWindowsLocked() if windowsIsNowLocked != _wasLockedPreviousPumpAll: _wasLockedPreviousPumpAll = windowsIsNowLocked postSessionLockStateChanged.notify(isNowLocked=windowsIsNowLocked) + _searchForLockAppModule() def isWindowsLocked() -> bool: @@ -165,6 +166,14 @@ def _isLockScreenModeActive() -> bool: if _isSecureDesktop(): # Use secure mode instead if on the secure desktop return False + + import winVersion + if winVersion.getWinVer() < winVersion.WIN10: + # On Windows 8 and Earlier, the lock screen runs on + # the secure desktop. + # Lock screen mode is not supported on these Windows versions. + return False + return _isWindowsLocked() From c4b3f8e13bb8b0fbdfe324474d05c54da480cd53 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 15 Dec 2022 12:19:00 +1100 Subject: [PATCH 19/20] refer as below instead of above --- source/NVDAObjects/__init__.py | 12 ++++----- source/utils/security.py | 48 ++++++++++++++++------------------ 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/source/NVDAObjects/__init__.py b/source/NVDAObjects/__init__.py index 23a66ff2ae5..d3b3fccdf3c 100644 --- a/source/NVDAObjects/__init__.py +++ b/source/NVDAObjects/__init__.py @@ -30,7 +30,7 @@ TreeInterceptor, ) import braille -from utils.security import _isObjectAboveLockScreen +from utils.security import _isObjectBelowLockScreen import vision import globalPluginHandler import brailleInput @@ -1435,10 +1435,10 @@ def getSelectedItemsCount(self,maxCount=2): """ return 0 - #: Type definition for auto prop '_get_isAboveLockScreen' - isAboveLockScreen: bool + #: Type definition for auto prop '_get_isBelowLockScreen' + isBelowLockScreen: bool - def _get_isAboveLockScreen(self) -> bool: + def _get_isBelowLockScreen(self) -> bool: if not _isLockScreenModeActive(): - return True - return _isObjectAboveLockScreen(self) + return False + return _isObjectBelowLockScreen(self) diff --git a/source/utils/security.py b/source/utils/security.py index 03316616203..b94c671be0a 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -132,9 +132,7 @@ def _isLockAppAndAlive(appModule: "appModuleHandler.AppModule") -> bool: return appModule.appName == "lockapp" and appModule.isAlive -# TODO: mark this API as public when it becomes stable (i.e. remove the underscore). -# Add-on authors may require this function to make their code secure. -# Consider renaming (e.g. objectOutsideOfLockScreenAndWindowsIsLocked). +# TODO: Consider renaming to _objectBelowLockScreenAndWindowsIsLocked. def _isSecureObjectWhileLockScreenActivated( obj: "NVDAObjects.NVDAObject", shouldLog: bool = True, @@ -145,7 +143,7 @@ def _isSecureObjectWhileLockScreenActivated( @return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is below the lock screen. """ try: - isObjectBelowLockScreen = _isLockScreenModeActive() and not obj.isAboveLockScreen + isObjectBelowLockScreen = _isLockScreenModeActive() and obj.isBelowLockScreen except Exception: log.exception() return False @@ -163,9 +161,9 @@ def isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: # TODO: improve deprecation practice on beta/master merges log.error( "This function is deprecated. " - "Instead use obj.isAboveLockScreen. " + "Instead use obj.isBelowLockScreen. " ) - return obj.isAboveLockScreen + return not obj.isBelowLockScreen def _findLockAppModule() -> Optional["appModuleHandler.AppModule"]: @@ -182,7 +180,7 @@ def _searchForLockAppModule(): _checkWindowsForAppModules() -def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: +def _isObjectBelowLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: """ When Windows is locked, the foreground Window is usually LockApp, but other Windows can be focused (e.g. Windows Magnifier, reset PIN workflow). @@ -194,7 +192,7 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: foregroundProcessID, _foregroundThreadID = winUser.getWindowThreadProcessID(foregroundWindow) if obj.processID == foregroundProcessID: - return True + return False if ( # alt+tab task switcher item. @@ -206,7 +204,7 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: # that the user has switched to a secure desktop. or isinstance(obj, SecureDesktopNVDAObject) ): - return True + return False lockAppModule = _findLockAppModule() if lockAppModule is None: @@ -214,23 +212,23 @@ def _isObjectAboveLockScreen(obj: "NVDAObjects.NVDAObject") -> bool: # This is not an expected state. log.error( "lockAppModule not detected when Windows is locked. " - "Cannot detect if object is above lock app, considering object as safe. " + "Cannot detect if object is below lock app, considering object as safe. " ) - return True + return False from NVDAObjects.window import Window if not isinstance(obj, Window): log.debug( - "Cannot detect if object is above lock app, considering object as safe. " + "Cannot detect if object is below lock app, considering object as safe. " ) # Must be a window instance to get the HWNDVal, other NVDAObjects do not support this. - return True + return False topLevelWindowHandle = winUser.getAncestor(obj.windowHandle, winUser.GA_ROOT) - return _isObjectAboveLockScreenCheckZOrder(topLevelWindowHandle, lockAppModule.processID) + return _isObjectBelowLockScreenCheckZOrder(topLevelWindowHandle, lockAppModule.processID) -def _isObjectAboveLockScreenCheckZOrder(objWindowHandle: int, lockAppModuleProcessId: int) -> bool: +def _isObjectBelowLockScreenCheckZOrder(objWindowHandle: int, lockAppModuleProcessId: int) -> bool: """ This is a risky hack. If the order is incorrectly detected, @@ -245,21 +243,21 @@ def _isWindowLockApp(hwnd: winUser.HWNDVal) -> bool: return windowProcessId == lockAppModuleProcessId try: - return _isWindowAboveWindowMatchesCond(objWindowHandle, _isWindowLockApp) + return _isWindowBelowWindowMatchesCond(objWindowHandle, _isWindowLockApp) except _UnexpectedWindowCountError: log.debugWarning("Couldn't find lock screen") - return True + return False class _UnexpectedWindowCountError(Exception): """ Raised when a window which matches the expected condition - is not found by _isWindowAboveWindowMatchesCond + is not found by _isWindowBelowWindowMatchesCond """ pass -def _isWindowAboveWindowMatchesCond( +def _isWindowBelowWindowMatchesCond( window: winUser.HWNDVal, matchCond: Callable[[winUser.HWNDVal], bool] ) -> bool: @@ -267,11 +265,11 @@ def _isWindowAboveWindowMatchesCond( This is a risky hack. The order may be incorrectly detected. - @returns: True if window is above a window that matches matchCond. + @returns: True if window is below a window that matches matchCond. If the first window is not found, but the second window is, - it is assumed that the first window is above the second window. + it is assumed that the first window is below the second window. - In the context of _isObjectAboveLockScreenCheckZOrder, NVDA starts at the lowest window, + In the context of _isObjectBelowLockScreenCheckZOrder, NVDA starts at the lowest window, and searches up towards the closest/lowest lock screen window. If the lock screen window is found before the NVDAobject, the object should be made accessible. This is because if the NVDAObject is not present, we want to make it accessible by default. @@ -295,7 +293,7 @@ def _isWindowAboveWindowMatchesCond( window1Indexes.append(currentIndex) if matchCond(currentWindow): if not window1Indexes: - return True + return False window2Index = currentIndex break currentWindow = winUser.getWindow(currentWindow, winUser.GW_HWNDPREV) @@ -307,9 +305,9 @@ def _isWindowAboveWindowMatchesCond( f" - window 2 index: {window2Index}\n" ) if window1Indexes[0] >= window2Index: - return True - else: return False + else: + return True _hasSessionLockStateUnknownWarningBeenGiven = False From dfa30dd33a99adbe62ecb8c91201a5dbbf99a1e3 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Thu, 15 Dec 2022 12:20:17 +1100 Subject: [PATCH 20/20] fixup comment from review suggestions Co-authored-by: Michael Curran --- source/utils/security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/utils/security.py b/source/utils/security.py index b94c671be0a..bf145f73896 100644 --- a/source/utils/security.py +++ b/source/utils/security.py @@ -273,7 +273,7 @@ def _isWindowBelowWindowMatchesCond( and searches up towards the closest/lowest lock screen window. If the lock screen window is found before the NVDAobject, the object should be made accessible. This is because if the NVDAObject is not present, we want to make it accessible by default. - If the lock screen window is not present, we also want to make the NVDAObject accessible, + If the lock screen window is not present at all, we also want to make the NVDAObject accessible, so the lock screen window must be comprehensively searched for. If the NVDAObject is found, and then a lock screen window, the object is not made accessible as it is below the lock screen.