Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore foreground event for SecureDesktopNVDAObject #14105

Merged
merged 3 commits into from
Sep 6, 2022
Merged

Ignore foreground event for SecureDesktopNVDAObject #14105

merged 3 commits into from
Sep 6, 2022

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Sep 5, 2022 •
edited
Loading

Link to issue number:

Supersedes #14098
Fixes #14094

Summary of the issue:

SecureDesktopNVDAObject needs to be whitelisted on the lock screen.
This is an NVDAObject used to notify the user and API consumers that NVDA has entered a secure desktop.

Description of user facing changes

Fixes NVDA remote bug described in #14094.
"Secure Desktop" is now consistently announced again when entering a secure desktop.

Description of development approach

Add SecureDesktopNVDAObject to the whitelist of available objects on the lock screen.
Ensures that when setting the foreground event does not occur via doPreGainFocus for SecureDesktopNVDAObject.
This is because the foreground event cannot be handled in a secure manner, and is not required for the SecureDesktopNVDAObject API.

Testing strategy:

Manual testing

Known issues with pull request:

Important note: This change technically results in an add-on API change
SecureDesktopNVDAObject is no longer a subclass of Desktop or Window

It is unexpected that SecureDesktopNVDAObject is used as a general NVDAObject:

  • NVDA is in sleep mode after the internal gain focus event for the SecureDesktopNVDAObject.
  • The only known use-case was as a notification of a desktop change (to a secure desktop). If this assumptions holds, other information on the object is not required.

Allowing this object to use Desktop or Window as a base class creates another vector for information leaks or privilege escalation.

Future plans:
Cater to the notification use-case via an extension point rather than the internal gain gocus event.

Change log entries:

Refer to PR diff

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd requested a review from a team as a code owner September 5, 2022 06:50
@seanbudd seanbudd requested a review from feerrenrut September 5, 2022 06:50
@seanbudd seanbudd mentioned this pull request Sep 5, 2022
Closed
8 tasks
@seanbudd seanbudd changed the base branch from master to rc September 5, 2022 06:51
@seanbudd seanbudd requested a review from feerrenrut September 6, 2022 05:54
@seanbudd seanbudd merged commit e05b39d into rc Sep 6, 2022
@seanbudd seanbudd deleted the fix-14094-2 branch September 6, 2022 07:48
@nvaccessAuto nvaccessAuto added this to the 2022.4 milestone Sep 6, 2022
@seanbudd seanbudd mentioned this pull request Sep 9, 2022
Merged
6 tasks
seanbudd added a commit that referenced this pull request Sep 9, 2022
@seanbudd
Restore backwards compatibility for SecureDesktopNVDAObject (#14116)
8eac659 
Follow up to #14105
Fixes issue described in #14111 (comment)

Summary of the issue:
SecureDesktopNVDAObject is an API end point used to indicate to the user and to API consumers (including NVDA remote), that the user has switched to a secure desktop.
This is triggered when Windows notification EVENT_SYSTEM_DESKTOPSWITCH notifies that the desktop has changed.
The switch is handled via a gainFocus event.
The gainFocus event causes the user instance of NVDA to enter sleep mode as the secure mode NVDA instance starts on the secure screen.

Information from SecureDesktopNVDAObject should not be accessible to the user, as it is backed by a valid MSAA desktop running on a secure profile, that NVDA can report information from.
This should generally be handled by NVDA entering sleep mode.
In #14105, SecureDesktopNVDAObject became based on NVDAObject to improve security for the object, by breaking it's connection to a valid window.
This was to decrease the theoretical risk of information leakage.

However, it was discovered that NVDA core event tracking and API consumers rely on SecureDesktopNVDAObject inheriting from Window (a parent class of Desktop).

As such, SecureDesktopNVDAObject must remain a Desktop subclass to retain backwards compatibility.

However we can prevent neighbouring objects from being accessed.

Description of user facing changes
Fixes bug in NVDA alpha with handling SecureDesktopNVDAObject.
Fixes API breakage.

Description of development approach
Reverts the change in #14105, making SecureDesktopNVDAObject inherit from Desktop.

Prevents neighbouring objects to SecureDesktopNVDAObject from being accessed by overriding relevant methods.
This was referenced Sep 13, 2022
Merged
Closed
seanbudd added a commit that referenced this pull request Sep 15, 2022
@seanbudd
When entering a secure desktop, cancel speech (#14135)
c53bc97 
Summary of the issue:
NVDA 2022.2 would cancel speech when handling the foreground event for entering a secure desktop.

The security patch for 2022.2.1 broke how the foreground event was handled, and subsequently #14105 opted to avoid the foreground event entirely.

This results in NVDA not correctly cancelling speech when entering a secure desktop.

Description of user facing changes
Speech is cancelled when entering a secure desktop

Description of development approach
Cancel speech just before enter sleep mode, when SecureDesktopNVDAObject handles it's gain focus event.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feerrenrut feerrenrut feerrenrut approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2022.4
Development

Successfully merging this pull request may close these issues.

NVDA no longer recognises that the system has switched to a secure desktop when dismissing the lock screen
3 participants
@seanbudd @feerrenrut @nvaccessAuto