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

Require UAC prompt to Install add-ons to installed NVDA copy: Install to Program Files, not AppData #16235

Closed
seanbudd opened this issue Feb 28, 2024 · 65 comments
Labels
api-breaking-change blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. needs-triage

Comments

@seanbudd
Copy link
Member

seanbudd commented Feb 28, 2024

Is your feature request related to a problem? Please describe.

Add-ons are currently installed to AppData, a user specific folder generally used for holding app configuration and data.
Add-ons are executable code, that runs at a system level.

Windows is designed to protect the system from installation of dangerous code through using the Write protected folders for Program Files.
Code that must run at a system level is generally installed to program files, and protected by UAC administrator approval for installation.

A commonly raised security concern is that NVDA users can install malicious add-ons, meaning that a malicious or naïve user can wreak havoc on their system.
Secure mode prevents the installation or modification of add-ons, however in many environments secure mode is overkill.
By installing add-ons to Program Files, and requiring admin approval for each install, we are following the common Windows patterns associated with installing potentially dangerous software.
By performing the UAC approval on a per add-on basis, users may better understand the risk involved, and approach with the same caution that they would when installing any software with a UAC prompt.

Describe the solution you'd like

Installing add-ons to program files would ensure that administrator approval is required for the installation of any add-on.
It would have the side affect of installing add-ons for all users of a machine, so different users of the same machine would have to disable/enable desired installed add-ons on their own profile.

Add-on authors would probably have to be conscious of the change, differentiating between the add-on installation folder, and the add-on individual user config folder.
As such, this is probably an API breaking change.

For portable copies, this won't be a significant change, as the config directory is stored within the portable copy and portable copies are can be inherently packaged with arbitrary insecure code.

Describe alternatives you've considered

The current warning system conveys appropriate risk, and administrators are able to prevent modification/installation of add-ons with secure mode.
As such, this change is not of a large material benefit.
This proposal just aims to align the process with a common UX for windows.

Additional context

Other extension libraries generally don't require a UAC prompt to install an add-on, but these extension systems generally aren't as risky as NVDA, and are safely sandboxed.

@seanbudd seanbudd added blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. api-breaking-change labels Feb 28, 2024
@josephsl
Copy link
Collaborator

Hi,

While I understand the security implications for moving add-on data to program files (with or without "x86" suffix), I think this is quite an overkill. More importantly, the proposed change does not align with portable NVDA where add-ons are used by users for various reasons, or when wishing to make a portable backup of NVDA installation which includes add-ons for currently logged on user. Also, add-on authors must declare that their add-ons are secure and are trustworthy, including not abusing admin privileges because just saying "yes" to UAC does not fully guarantee that Python/C code are secure (UAC communicates two things: the code about to be executed is trustworthy, and users really know what they are doing). In summary, relying on UAC and its messaging/experience may provide appearance of asking for trust, but it does not fully guarantee what the add-on does is what it claims to do (for that, I think source code audit of add-ons is more effective).

Thanks.

@seanbudd
Copy link
Member Author

@josephsl

More importantly, the proposed change does not align with portable NVDA where add-ons are used by users for various reasons, or when wishing to make a portable backup of NVDA installation which includes add-ons for currently logged on user.

Maybe this title is misleading. The idea here is installing add-ons to the install directory, not the config directory. For portable copies, these are more or less the same (userConfig in the portable copy directory).

Also, add-on authors must declare that their add-ons are secure and are trustworthy, including not abusing admin privileges because just saying "yes" to UAC does not fully guarantee that Python/C code are secure (UAC communicates two things: the code about to be executed is trustworthy, and users really know what they are doing).

We don't have the capacity to meaningfully declare add-ons as secure or trustworthy. What we can do is ensure users understand these risks clearly and only users with permission to install software can take these risks.

I disagree here with your understanding of the purpose of UAC prompts. Users most often run into this prompt when installing software. For most users it is about trusting the source of the software and being willing to let it modify your system. I am not sure to what extent users trust the code itself just because it requires administrator approval to install it.
You perform the same UAC operation on installing unsafe packages, trust in the code itself is independent of the safety process around restricting installation. At the UAC installation prompt is where the admin/trusted user decides "I trust this software installation to let it modify my machine".

@Adriani90
Copy link
Collaborator

I agree with @seanbudd. This will definitely increase the trust in NVDA in corporate environments.
However, I think a separate manual or a help notice for system admins on the NV Access website would be really helpful.
This shelp notice should include all security related features, command line parameters, help on minimizing security risk from originating from NVDA etc.

@seanbudd
Copy link
Member Author

@Adriani90 - we try to include all that information here: https://www.nvaccess.org/corporate-government/

@seanbudd seanbudd changed the title Install add-ons to Program Files, not AppData Require UAC prompt to Install add-ons to installed NVDA copy: Install to Program Files, not AppData Feb 28, 2024
@XLTechie
Copy link
Collaborator

XLTechie commented Feb 28, 2024 via email

@seanbudd
Copy link
Member Author

seanbudd commented Feb 28, 2024

Therefore an add-on that was only installed in one, potentially non-privileged
account before, must now be installed for all accounts, system wide.

Add-ons are inherently capable of performing privileged actions even from non-privileged accounts. There is a pending advisory related to this, but this is also documented in the user guide and corporate page. As such, this change doesn't really affect the range of risk, but more accurately reflects it.

I think this also means that we must remove the scratchpad feature. Else any
add-on could be installed there, circumventing this supposedly more secure
add-on paradigm.

This could also be moved to program files? I guess the other concern is the python console also has the same risks as scratchpad and add-ons, and that is included by default.
I think the python console should probably be disabled by default on release builds and enabled via a registry key parameter .
Being able to disable the python console individually via the registry, or even have it off by default is something often requested by corporate environments.

I disagree with the UAC autonomy note, as I believe this is a clear example of when UAC should be used, i.e. when installing code that can run at system level.

@Adriani90
Copy link
Collaborator

@XLTechie

Therefore an add-on that was only installed in one, potentially non-privileged
account before, must now be installed for all accounts, system wide.

Are you completely sure? I am on a corporate environment with quite strict security policies, I cannot update or install anything and every UAC promt requires admin priviledges to be accepted.
Still I can install addons without any restriction.

@seanbudd
I am not sure if I miss something, but I think we should mention in the documentation on the corporate environment page that admins will have to allow the configuration of addons if they allow their installation.
It would be very annoying if we are allowed to install addons in a compnay but the configuration is not saved.
Maybe it is possible to split the install path for addons but maintain their configuration in the appdata.
Also can we actually pretend that addons from the addon store are secure? Is there any security check happening on the addon submission repository?

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Feb 28, 2024

@seanbudd I think there are some statements in your proposal that are not true, at least not necessarily.

Add-ons are executable code, that runs at a system level.

If I'm running NVDA as a standard Windows user (not administrator), apart from UI Access, the add-on has no more permissions than every other application running either portable or installed on a per user basis (see below).

By installing add-ons to Program Files, and requiring admin approval for each install, we are following the common Windows patterns associated with installing potentially dangerous software.

I think this is also incorrect. With the Windows Store, Microsoft is following a pattern in which applications are installed on a user basis (see %localappdata%\Microsoft\WindowsApps).

In my opinion, NVDA has no duty to avoid running malicious code, that's up to the system. Instead, it should avoid installing it. The patterns that Microsoft follows are also much more meaningful in that case. I can think of:

  • Installing third party add-ons (outside the store) should be something that can be disabled without explicitly enabling secure mode. it makes sense to make that the default and explicitly opt-in.
  • Introduce a security mechanism where add-on packages or manifests must be signed, though that's probably a bit overkill.

@seanbudd
Copy link
Member Author

seanbudd commented Feb 28, 2024

@Adriani90

I am not sure if I miss something, but I think we should mention in the documentation on the corporate environment page that admins will have to allow the configuration of addons if they allow their installation.
It would be very annoying if we are allowed to install addons in a compnay but the configuration is not saved.
Maybe it is possible to split the install path for addons but maintain their configuration in the appdata.

As mentioned in the proposal, add-ons would continue to have a separate configuration folder with NVDA config, e.g. in AppData.

Also can we actually pretend that addons from the addon store are secure? Is there any security check happening on the addon submission repository?

No - this is not in any way a change how add-ons are perceived security wise. As mentioned all over NVDA, add-ons are not secure or tested and you must trust the author if you wish to install one. The idea here is to highlight that you are essentially installing a new independent and unverified program every time you install an add-on.

@seanbudd
Copy link
Member Author

@LeonarddeR

If I'm running NVDA as a standard Windows user (not administrator), apart from UI Access, the add-on has no more permissions than every other application running either portable or installed on a per user basis (see below).

As mentioned earlier, add-ons can run at whatever level NVDA is running, including as administrator. i.e. when performing an update or running the com reg fixing tool add-ons can hijack UAC elevated privileges.
This is documented in the user guide and corporate page:

NVDA allows the installation of custom add-ons, which can execute arbitrary code, including when NVDA is elevated to administrator privileges.

https://www.nvaccess.org/files/nvda/releases/2024.1beta10/documentation/userGuide.html#SecureMode

@seanbudd
Copy link
Member Author

seanbudd commented Feb 28, 2024

Installing third party add-ons (outside the store) should be something that can be disabled without explicitly enabling secure mode. it makes sense to make that the default and explicitly opt-in.

This would be a false sense of security. There is no security difference between add-ons from outside of the add-on store and add-on store add-ons.

Introduce a security mechanism where add-on packages or manifests must be signed, though that's probably a bit overkill.

I think implementation of this and adoption of this from add-on authors would be more challenging, but this is certainly an alternative proposal.

@seanbudd
Copy link
Member Author

With the Windows Store, Microsoft is following a pattern in which applications are installed on a user basis (see %localappdata%\Microsoft\WindowsApps).

I think a difference with this case is these applications are not being run from Program Files, I think the explicit issue is we take code from AppData and execute it from Program Files.

@XLTechie
Copy link
Collaborator

@seanbudd Perhaps an intermediate proposal may be considered?:

Create a new "mode" in NVDA, called "corporate mode" or "Restricted Mode" or some other appropriate name.

  • This mode would be activated by the appearance of a file, call it restrictedMode.ini, in the install directory.
  • At first, this file would just cause NVDA to implement the policy in this issue.
  • However in the future, it could implement other, more configurable, features, of interest to corporate or mass-install users. There have been issues in the past (I may edit in numbers later, if this is interesting), where the specific needs of corporate users might require more granular control of NVDA features than the average user would need at an install level.

It would be a trivial matter, to create two installers for every release: one that contains this file, called the "Restricted Corporate Mode Installer", and the usual installer that we generate now.

  • The file approach has the benefit of not requiring a separate compilation, thus not increasing build times notably.
  • Further, as opposed to a registry key for triggering this, a file makes it possible for us to include it in the distribution, giving corporate users an instant path to knowing which version they should use, without any risk of the registry key not being turned on in desirous installations; and without any need for commandline options in the installer to implement it.

This lets us separate the user groups by their perceived needs, at little inconvenience to either NV Access or the disinterested portion of the user base.

@mwhapples
Copy link
Collaborator

This proposal really bothers me, the fact that an addon would be installed globally. So another user who can perform an admin right may inject an unknown addon into my screen reader which I treat as a very sensitive app as it is capable of gathering keypresses and thus passwords, etc. Would I get a notification when I start NVDA that another user has installed an addon and would I have any chance of disabling it before any of its code could run as my user? I personally would see this as a degrading of security.
Per user installing of addons feels more correct. If we want to restrict users from being able to install addons without the appropriate rights, then I feel may be other approaches should be considered. One suggestion would be for NVDA to have its own authentication system which prompts users to authenticate before installing an addon. If the authentication system were passwords, then the computer admin could set a NVDA password system wide. Other authentication systems could be implemented by NVDA if wanted (eg. key files).
My view on addons is that they need extreme checking before I will install them into NVDA, so much so that that addons are not desireable things. It could be argued that a user with admin rights could cause similar security issues on a computer without this change to installing addons globally, but I would say we should not put another place the user needs to use that admin right in front of them, normalising the use of admin rights and create yet another attack vector for malicious code.
As noted in the original proposal this is only dealing with where and how addons can be installed, for proper addon security better sandboxing and limiting what addons can access/do would be needed.

@Adriani90
Copy link
Collaborator

Actually yeah we really should think more about some alternatives here and I find @mwhapples point about user specific add-on installation very valid. It didn't came to my head before. :-)
XLTechie

Create a new "mode" in NVDA, called "corporate mode" or "Restricted Mode" or some other appropriate name.
I am not really convinced this is a good solution. Corporate specific requirements can be easily implemented via admin features such as command line parameters or so. And user related feature in NVDA are useful for everyone no matter if the user is in a corporate environment or not.
Moreover, your proposal would really bring the maintenance effort to a next level and I think this is an overkill.

@XLTechie
Copy link
Collaborator

XLTechie commented Feb 28, 2024 via email

@XLTechie
Copy link
Collaborator

XLTechie commented Feb 28, 2024 via email

@Nael-Sayegh
Copy link
Contributor

Hello,
I disagree with the idea that an add-on should be installed globally for all users for the same reasons as @mwhapples and that, in addition, it will result in a lot of add-ons installed for all users when not everyone necessarily needs them.
The problem also, even though security is extremely important in companies, I think that the user who has to ask their administrator to authorize the installation and update of each add-on they install will be more of an obstacle than anything else.
The fact of placing the add-ons in the installation directory is that for inexperienced users, access will be more complex if they have to manually edit files as several add-ons require configuring or activating a license, for example.
If they have to search in the installation directory for these files, it will discourage a lot of people.
Finally, I think there are other solutions to strengthen the security of add-ons without having to move them from their directories and make all this management too complicated.

@josephsl
Copy link
Collaborator

josephsl commented Feb 28, 2024 via email

@LeonarddeR
Copy link
Collaborator

As mentioned earlier, add-ons can run at whatever level NVDA is running, including as administrator. i.e. when performing an update or running the com reg fixing tool add-ons can hijack UAC elevated privileges. This is documented in the user guide and corporate page:

Add-ons can indeed request NVDA to elevate, in fact that's what I do in RDAccess. However, I'm pretty sure that an add-on can't elevate if the UAC screen is enabled and a user is prompted. In that stage, it became the responsibility of the user.

With the Windows Store, Microsoft is following a pattern in which applications are installed on a user basis (see %localappdata%\Microsoft\WindowsApps).

I think a difference with this case is these applications are not being run from Program Files, I think the explicit issue is we take code from AppData and execute it from Program Files.

NVDA is certainly not unique in this. In fact, this applies to every application that supports some form of user extensions. I am thinking of applications such as Foobar2000, visual studio code (system wide install), global installations of Python. And how about other screen readers, like JAWS?

Installing third party add-ons (outside the store) should be something that can be disabled without explicitly enabling secure mode. it makes sense to make that the default and explicitly opt-in.

This would be a false sense of security. There is no security difference between add-ons from outside of the add-on store and add-on store add-ons.

In that case, I'd rather see add-on store add-ons made more secure.

I understand that there is a security risk associated with using add-ons, but I'm afraid I cannot support such a drastic change as proposed in this issue. @mwhapples also has a very important point.

@nvdaes
Copy link
Collaborator

nvdaes commented Feb 28, 2024

I tend to agree with people who think that add-ons shouldn't be installed globally but for specific users.
The store provides a warning, in addition to the information included in the user guide and in the corporate environment page on NV Access website, so I don't see too much benefit from this proposal, but this can be anoying if users neeed to disable or enable lots of specific add-ons installed globally.
About the store, @seanbudd wrote:

There is no security difference between add-ons from outside of the add-on store and add-on store add-ons.

This is true, but imo we shouldn't give the feeling that the store has no difference respect external sources, since this would be also a false feeling. NVDA checks sha-256 to check add-ons integrity, and submitting add-ons to the store has requirements that ensure that add-ons are not released as stable for experimental API versions. this is not trivial, so it's not the same to use the store that external sources.
An issue is open in the store to sign add-ons, and personally, for now, I use GPG to sign add-on releases. I think that NV Access may want to work on this issue too. Since this is mentioned here, for reference the corresponding issue is at

nvaccess/addon-datastore#41

@lukaszgo1
Copy link
Contributor

I fully agree with @LeonarddeR and @mwhapples that moving add-ons to program files is not an acceptable solution at all. What @LeonarddeR pointed out about other programs which support extensions is also very true. The difference, of course, is that many of these(JAWS being a prime example) have a proper API boundaries between extensions and the main software, and that should be the direction NVDA moves in.

In that case, I'd rather see add-on store add-ons made more secure.

How exactly should this be implemented? For me this appears pretty impossible to do.

As a middle ground: has it been considered to leave the install location alone and just show UAC screen during add-on installation? This ensures that non admin users cannot install add-ons, yet very little changes other than the additional elevation requirement. I'm not sure how annoying that would be for users though. The truth of the matter is that NVDA is not really usable without add-ons, so if their installation is going to be made any more difficult than it is now, we really need a mindset change where add-ons are required only for the most unusual usages, and most of those which are used commonly are integrated into the main product.

@bramd
Copy link
Contributor

bramd commented Feb 28, 2024

Many valid points have already been raised in this thread. I also don't like the idea to putting all addons in a privileged location by default and fully restrict users from installing addons in non-admin environments. If we go that way, I would like to see a clear policy of what we could and should put in core and why, since some addons provide very useful functionality without accessing any resources outside NVDA. Why are some rarely used appmodules in NVDA for example and does NVDA not provide sensible defaults for some other applications and a feature to keep Bluetooth and bad behaving sound cards open? Especially the latter is clearly needed on many machines and is in JAWS for a few years now. I have seen NVDA in some corporate environments and I think these customers would be better off with:

  1. Group policies to centrally configure what NVDA can and can't do, including running addons and access to the Python console
  2. Allow an addon to either run from the users' directory or program directory. Ask this at install time or put program directory installations behind a command line flag or something, document it for sysadmins. This way some addons can be preinstalled in the program directory for all users and other user-level installations can be blocked
  3. Work on a formal, safe and/or sandboxed NVDA API that addons can use if there is a need for that. Addons using only this API would be safe to install, the API could also provide access to system level resources, but NVDA could prompt to allow this or block it in settings. And yes, I totally see that this is a lot of work and we should consider if it's worth the effort

I would love to hear from more NVDA users in a corporate environment.

@Adriani90
Copy link
Collaborator

a feature to keep Bluetooth and bad behaving sound cards open? Especially the latter is clearly needed on many machines and is in JAWS for a few years now.

This reached now also last NVDA alpha version, so no addon needed for that anymore. It took years to come though.

Reading this very interesting conversation, I think we should really challenge the aspects why so many addons are needed at all. It still contradicts the goal of better accessibility when we start restricting their functionality and though better security is needed.
I tend to agree in principal that we need a better way of handling security in this case, and I think bringing community proven addons into the core is a very good way to do it. However, this still depends on the willingness of the add-on authors to do it and undergo sometimes a very long review process which is needed. NV Access cannot do anything about it if people do not raise pull requests to bring their ideas into the core.

Especially NV Access did never totally deny a functionality proposed for the core if the community was insisting on having it in the core. However, NV Access developers and also contributors challenge these proposals and exchange arguments in a healthy way. Some add-on authors might feel overwelmed by the standards they have to comply with in terms of coding etc. But still this is needed and in the end it is the best security check you can have.

@Adriani90
Copy link
Collaborator

I think also @nvdaes brings also a very valid point. We can work on the add-ons security checks to improve them first before moving totally their installation into system admin's hands who very often do not understand accessibility needs because they are too far away from the community.

@Adriani90
Copy link
Collaborator

Adriani90 commented Feb 28, 2024

  1. Allow an addon to either run from the users' directory or program directory. Ask this at install time or put program directory installations behind a command line flag or something, document it for sysadmins. This way some addons can be preinstalled in the program directory for all users and other user-level installations can be blocked

For this proposal to work properly system amins will have to really digg deep into NVDA's functionality and its add-on environment. At least from my experience, all system admins I ever know in usual companies have very very basic NVDA knowledge. Letting them the decision of what users might need as add-ons to be preinstalled is not really going to work.

  1. Work on a formal, safe and/or sandboxed NVDA API that addons can use if there is a need for that. Addons using only this API would be safe to install, the API could also provide access to system level resources, but NVDA could prompt to allow this or block it in settings. And yes, I totally see that this is a lot of work and we should consider if it's worth the effort

I am working in a development financing institution with very strict security policies, and this approach would probably be the most atractive one for them. However, they still allow me to install add-ons but I have to send a request for that, otherwise they look at every NVDA update if there is an unknown add-on and remove it from my user account.
It is a governance process, but at least they look into it and they are ok so far. I am not sure we should open up a too big Pandora's box here.

@Adriani90
Copy link
Collaborator

Hi, I think this would be the moment to use Group Policy (finaly). That way enterprise admins can toggle features on and off at the individual or corporate level, including allowing add-on installation and enablement.

This requires a very advanced knowledge about the software you apply policies on. I doubt admins will look that deeply into it if there is only a very small amount of people using this software in the company. They will rather block as much as possible to minimize risks and people would have to ask everytime for the release of a specific configuration / setting and explain why it is useful.

@Nael-Sayegh
Copy link
Contributor

I think, like many people in this discussion, that the problem would rather be how to strengthen the security of add-ons rather than moving them from their directory. I do not know how the add-ons were checked before the release of the add-on store, but if there had to be a manual code check of each add-on, a lot of evaluators would need to be put in place for the publishing and updating of add-ons to happen quickly.

I also think that in manual control, the evaluator should not decide whether the add-on should be published or not based on its usefulness, but only on its security, contrary to what has happened with add-ons published on the old store that were refused because they were deemed useless.

I also think that already, to improve the security of add-ons, there should be a stricter way of authenticating oneself as an author so that NVAccess can have reliable author coordinates (name, first name, email address) and not use Github as it is now, where anyone can submit an add-on for someone else and where it is very easy to change one's name or delete information afterwards.

Finally, as @nvdaes and @XLTechie said, we should focus on the security of add-ons rather than disturbing the user with many warnings.

@nvdaes
Copy link
Collaborator

nvdaes commented Feb 29, 2024

I also think that in manual control, the evaluator should not decide whether the add-on should be published or not based on its usefulness, but only on its security, contrary to what has happened with add-ons published on the old store that were refused because they were deemed useless.

But add-ons review is a process made by volunteers, and to check security time is required, so it's understandable that reviewers don't want to waste time in reviewing add-ons not considered useful.

I also think that already, to improve the security of add-ons, there should be a stricter way of authenticating oneself as an author so that NVAccess can have reliable author coordinates (name, first name, email address) and not use Github as it is now, where anyone can submit an add-on for someone else and where it is very easy to change one's name or delete information afterwards.

Currently just 6 persons are considered trusted submitters, i.e., just they can submit add-ons on behalf of other authors. Other submitters need to be approved, and some evidence, like the GitHub URL, is checked to see if the submitter is the owner, of if he or she has permission from the add-on authors to submit it to the store.

@Adriani90
Copy link
Collaborator

Adriani90 commented Feb 29, 2024 via email

@Adriani90
Copy link
Collaborator

Adriani90 commented Feb 29, 2024 via email

@nvdaes
Copy link
Collaborator

nvdaes commented Feb 29, 2024

Re: usefullness and review, I think if people get a reward for it (i.e. through donations), there are certainly enough people who would be willing to do it even if the addon is not useful. We just have to find a bunch of people who are really trusted by the community and reward them to do it

I don't think that rewards have a lot of impact in finding reviewers, and this would bring the problem of donating to people instead of doing it directly to NV Access for NVDA, like it's already done for other projects. Also, reviewers may not be very active, and this should be measured in some way. some kind of certification maybe required, and I don't know if NV Access can do it given other priorities.
In addition, security issues maybe found after an add-on is published. I think that all this is really complex, and for now what we can do is to review add-ons published on the store.

@mwhapples
Copy link
Collaborator

I also think that in manual control, the evaluator should not decide whether the add-on should be published or not based on its usefulness, but only on its security, contrary to what has happened with add-ons published on the old store that were refused because they were deemed useless.

But add-ons review is a process made by volunteers, and to check security time is required, so it's understandable that reviewers don't want to waste time in reviewing add-ons not considered useful.
And define "useful", what is useful to one person may not be useful to another. May be it makes SoftwareX easier to use with NVDA but its not useful to me if I don't use SoftwareX. Some of the work arounds for bluetooth headsets and buggy sound cards, well that isn't useful to someone who doesn't use bluetooth and has a good sound card. My point is where should that boundary be. Yes there probably should be some sort of spam filter to prevent junk with the only purpose of overloading the system.
Reviewers should be prepared that there will be addons which don't interest them but may be of use to others.

@nvdaes
Copy link
Collaborator

nvdaes commented Feb 29, 2024

And define "useful",

My definition of useful in the context of this thread is what reviewers consider that can be enough useful to dedicate time and efforz to review add-ons.
For example, I didn't consider useful to review add-ons consisting on adding functions that can be done from NVDA itself, just changing shortcuts, in short.

@mwhapples
Copy link
Collaborator

And define "useful",

My definition of useful in the context of this thread is what reviewers consider that can be enough useful to dedicate time and efforz to review add-ons. For example, I didn't consider useful to review add-ons consisting on adding functions that can be done from NVDA itself, just changing shortcuts, in short.

That still isn't really a full definition if "useful" is a criteria which can determine if an addon will be reviewed. Is it a reviewers ability to decide whether they feel it is useful based on their own view of what constitutes useful, is there standard criteria to determine useful which the reviewer must use regardless of their own personal opinion, could an addon author appeal a decision that their addon is not useful, etc.
If reviews to treat an addon preferably were introduced, then the process to get that privileged status needs to be clear and applied consistently.
My point about a clear and consistently applied process extends beyond "useful", there may be other criteria for certain standards (eg. would NVAccess want to be seen giving a seal of approval to an addon which may have embedded political messages, etc). Getting the definitions correct for the criteria though would be important. One may even say "security" needs defining, security of what and for whom (eg. is it simply protecting the system from code which may damage it or does it extend to protecting data privacy and code which may only gather data but lets the system function normally).
I admit a bit of a minefield and quite understandable if NVAccess decided not to go there leaving it for the user to determine whether it meets their own criteria for there own needs and priorities. That makes me think another reason why if reviews were done for the need for a clear and consistent process, so the user can assess whether the review even deals with their concerns and needs.

@Adriani90
Copy link
Collaborator

Adriani90 commented Feb 29, 2024 via email

@lukaszgo1
Copy link
Contributor

While it is very likely the discussions about improving add-ons security should be done somewhere else, until it is not I'll post some thoughts in this issue.
I don't think reviews system as implemented in the store currently can be used for a kind of reviews were discussing here. Its intent was, at least to my knowledge, to encourage users to share feedback about the add-on in terms of usage. Assuming I weren't capable of reading Python code, when deciding if the given add-on is okay to install in a high security environment, I don't want / don't have time to read through reviews which will very likely not answer my main question.
What I have suggested is a simple classification - add-on is either reviewed or not, and there is no space for deciding based on user reviews.
The fact that given person is as @Adriani90 put it 'a really trusted by the community' does not mean they are qualified to perform security reviews. I have reviewed some (certainly not as many as other participants here) add-ons in the past, and while I have tried to pay attention to obvious security issues, I don't have any formal training in a subject as complex as security, so it is likely I missed something in that aspect.

Code reviews were pretty useful nevertheless, as they ensured that most pretty badly programmed add-ons, or one whose behavior was totally un-intuitive weren't available via the official sources.
I'd say more promising avenue to explore would be to create a forma API which can be used by add-ons or, alternatively, to introduce some kind of sandboxing for their code. This ensures that as long as the public interface is secure add-ons using it are secure too.

@Adriani90
Copy link
Collaborator

Does anyone know how other types of apps become trusted? I mean if someone downloads an app / or a browser extension in the app store, or in the Microsoft store or chrome web store, how do users or corporates make sure they can trust them?
Is it just because they assume Microsoft or Apple or Google have some security checks in the background?
Or do users reviews influence the decision whether to install it or not in a corporate environment?

At least I could find some useful information.
Here is an overview of risks especially for non-malitious apps:

  1. Insecure network connections
  2. Files stored with insecure file permissions or in an unprotected location
  3. Sensitive information written to a system log
  4. Web browser vulnerabilities
  5. Vulnerabilities in third-party libraries
  6. Cryptographic vulnerabilities, and more.
    https://www.linkedin.com/pulse/how-ensure-mobile-app-security-workplace-prateek-panda

I would say the first 3 points can be quite easily covered in a review without much effort, with a bit of technical knowledge.
However, the last 3 points are tricky to discover I guess if no one has professionally doing security reviews.

For mobile apps and browser extenstions at least, there are some tools where you can throw the code in and they generate a security report.
Since all our add-ons are open-source, we could think about using such a tool where python code is analysed automatically to a certain extent.
For mobile apps there is Appknox for example:
https://www.appknox.com/
For browser extensions there is the system center endpoint protection:
https://software.berkeley.edu/system-center-endpoint-protection-scep-0
and the CRxcavator where you can throw a log of the extension and it analyses it for certain security related aspects:
https://crxcavator.io/docs.html#/

In general it seems following behavior should people have before installing extensions of any kind. This could be rewritten as indications in the user guide and on the corporate environment website:
• Check out the developer’s website to see if it’s a legitimate extension and not a one-off by an unvetted source. (not really relevant for us because most add-ons are on github and still we cannot make sure people upload only trusted content on it)
• Read the description. Look for things that may be questionable, like tracking info or data sharing.
• Check out the reviews. Look for users complaining of oddities happening, speculating on their data being taken, or for anything that strikes you as odd. (this would come into force if enough users reviewed the add-ons. This could be encouraged to do via the in process blog, motivating people on the mailing list or Masterdom or Discord to give a user review etc.)
• Be picky. The more extensions installed, the bigger the attack surface you open up to attackers. Only pick the most useful and delete the ones you don’t need.
• Only install through trusted sources. While not guaranteed safe, security technicians review extensions for malicious content. 
• Review permissions. Review extension permissions closely. If an extension installed suddenly requests new permissions, be wary. If you can’t find a reason for the permission change, it’s probably better to uninstall.
https://security.berkeley.edu/education-awareness/browser-extensions-how-vet-and-install-safely

@Adriani90
Copy link
Collaborator

So in summary from my point of view following could be done to improve the security perception:

  1. Publish indications in the user guide and on the corporate environment website on what users should care about when installing add-ons
  2. Use a tool to detect vulnerabilities in the add-on code (i.e. as a github action in the add-on submission store with CodeqL: https://docs.github.com/en/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning-with-codeql
    here is a list of tools which could help in automating security checks in the add-on submission repository:
    https://owasp.org/www-community/Source_Code_Analysis_Tools
  3. Release the add-on to the add-on store with a review flag to the store depending on the results of the CodeqL or any automated analysis
  4. Encourage users via in process blog and mailing lists and other channels to provide reviews of the add-ons in the add-on store to build up trusted environment
  5. Make a compulsory data requirement in the manifest file which requires add-on authors to describe the permissions an add-on is running on a system, and show this data for every add-on when installing as a warning dialog
    (i.e. this add-on is using an external API to process data, or this add-on saves a log file in temp\add-on-name which contains private data such as what you type on your keyboard. By installing this add-on, you allow the add-on to use the saving and processing permissions on your system. Are you sure you want to install this add-on?
    yes button; no button).

@Adriani90
Copy link
Collaborator

I found a very interesting article on how to build github CI workflows / actions with codeqL and Bandit to analyse python repositories automatically as far as possible. Here is also an example repo with already built in actions to see how it works:
https://github.com/tjtharrison/demo-python-repository?tab=readme-ov-file#readme

I attach the article as a word document, it comes from here:
https://tjtharrison.medium.com/github-ci-workflows-for-python-code-repos-5e13b5538aaa
GitHub CI workflows for Python code repos.docx

@beqabeqa473
Copy link
Contributor

Just words in support of people who are against this issue.
Implementing this would make lot more problems, like was said about multi-user environments, if this is a shared computer, someone could use one of the main thing, a speech synthesizer, and which can have an update, which significantly changes output speech. Some users like that some not, and how you would solve this problem if addons will be shared between user accounts.
As said, this is overkill and i personally vote against this change.
Instead of this, a security mechanism should be implemented, for example signing a trusted addon with some sort of certificate own by nvaccess, which would lift off a question about legitimate source of this particular addon.

@Adriani90
Copy link
Collaborator

I created discussion #16241

I am closing this issue due to lack of support from the community. Further ideas can be discussed in the new discussion.

@Adriani90 Adriani90 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 29, 2024
@josephsl
Copy link
Collaborator

Hi,

I don't think so - if NV Access opened it, surely there could be a reason for it. Just because the community does not support it should not be the reason for closing it unless the original poster (NV Access) agrees with community assessment. Reopening.

Thanks.

@josephsl josephsl reopened this Feb 29, 2024
@nvdaes
Copy link
Collaborator

nvdaes commented Feb 29, 2024

I don't think so - if NV Access opened it, surely there could be a reason for it. Just because the community does not support it should not be the reason for closing it unless the original poster (NV Access) agrees with community assessment. Reopening.

I support Joseph action and arguments here.

@Adriani90
Copy link
Collaborator

Adriani90 commented Feb 29, 2024

If someone creates and issue (even being nV Access,) then it seeks community feedback and advice. If the support is not given on the basis of the arguments in the description or further conversation comments, issues and requests are closed, no matter who creates them. This is the triage policy and the practice we have for years. We cannot put anyone into advantage because it has a certain role. I hope we don't create a community with unhealthy hierarchies.

So in case @seanbudd has further arguments which contribute to support his statement in the description we can reopen the discussion and the community feedback loop. Otherwise there is already an alternative discussion where ideas can be further discussed.

@nvdaes
Copy link
Collaborator

nvdaes commented Feb 29, 2024

I didn't know the mentioned practice about triage, @Adriani90 . Sorry for my quick reply without knowing this.

@josephsl
Copy link
Collaborator

Hi,

When an NV Access staff (or for that matter, members of any organization) posts something, they are speaking on behalf of both themselves and the organization they are part of. Because Sean said there is a pending advisory (see his initial comment), it can be inferred that NV Access staff had an internal discussion about it. I would say this also applies to the wider community - when we comment, we would think about individual and other roles and groups/organizations we are a part of.

The community could close an issue if we haven't heard from original posters for a while, and lack of communication is a strong justification which can both encourage and discourage participation (encouraging folks to answer questions in a timely manner, discouraging because issues might be closed just because we did not hear from people). But if we are dealing with a potential and incoming security advisory on a topic with community-wide consequences, and if the organization responsible for representing product development is deeply interested in this topic, then I think organizational members should be given a chance to respond rather than closing an issue outright due to community opposition. This last sentence is where I'm getting at and I strongly disagree with closing this issue. Besides, several people have offered other justifications and thoughts after Sean posted his responses, and I think it would be better to let Sean and other NV Access staff offer their thoughts to subsequent comments. What I'm offering is a pircutre of a higherarcy, but that of negotiations and dialogue.

Thanks.

@seanbudd seanbudd reopened this Feb 29, 2024
@Emil-18
Copy link
Contributor

Emil-18 commented Mar 2, 2024

The difference, of course, is that many of these(JAWS being a prime example) have a proper API boundaries between extensions and the main software, and that should be the direction NVDA moves in.

@lukaszgo1 I really don't think NVDA should move in that direction. The main thing that makes NVDA addons superior to scripting engines of other screen readers in my opinion is exactly that add-ons can do more or less anything. I think it is ok if NVDA gets a version or mode spesificly designed for secure environments, and it is implemented there, but I don't think add-ons should be restricted in any way in what they can do in the normal version/mode.

From NVDA's product vision document

NVDA provides freedom for add-ons to find innovative solutions by allowing them broad access to NVDA's internals.

@Emil-18
Copy link
Contributor

Emil-18 commented Mar 4, 2024

@XLTechie I noticed that, and corrected the comment. I somehow managed to put your name there instead of @lukaszgo1. Sorry for that

@XLTechie
Copy link
Collaborator

XLTechie commented Mar 4, 2024 via email

@seanbudd
Copy link
Member Author

Thanks for the earlier discussion.
We are closing this due to discovering the change would actually create a wider vector for certain vulnerabilities.

@seanbudd seanbudd closed this as not planned Won't fix, can't repro, duplicate, stale Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-breaking-change blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. needs-triage
Projects
None yet
Development

No branches or pull requests