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.

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

Restore backwards compatibility of SecureDesktopNVDAObject #14116

Merged
merged 1 commit into from
Sep 9, 2022

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Sep 9, 2022

After merging to beta, translations should be pushed and translators should be notified of the lines removed.

Link to issue number:

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.

Testing strategy:

Checked diff between release-2022.2.2 and this branch.
Ensured "Secure Desktop" was still announced when switching to a secure desktop, i.e. #14094 was still fixed.

Known issues with pull request:

SecureDesktopNVDAObject is being used an API end point for handling a EVENT_SYSTEM_DESKTOPSWITCH to a secure desktop.
Using a gainFocus event on an NVDAObject to handle a Windows notification seems to be an obscure method of handling events.
NVDA has extensionPoints for that purpose.
In 2023.1 SecureDesktopNVDAObject should be removed, replaced by an extension point to notify add-ons that NVDA has entered a secure desktop.

Change log entries:

See 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 9, 2022 00:35
@seanbudd seanbudd changed the base branch from master to rc September 9, 2022 00:49
@seanbudd seanbudd self-assigned this Sep 9, 2022
@seanbudd seanbudd added this to the 2022.2.3 milestone Sep 9, 2022
@seanbudd seanbudd merged commit 8eac659 into rc Sep 9, 2022
@seanbudd seanbudd deleted the backwardsCompat-SecureDesktopNVDAObject branch September 9, 2022 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants