Open Bug 1359182 Opened 7 years ago Updated 2 years ago

Master password API for WebExtensions

Categories

(WebExtensions :: General, enhancement, P5)

enhancement

Tracking

(Not tracked)

People

(Reporter: htamas, Unassigned)

Details

(Whiteboard: [design-decision-needed][triaged] [needs-follow-up])

I would like to port my add-on StartupMaster to the new WebExtension framework. At the moment there is no API for master password management, so I'm proposing one in comment 1.
The main aspects I considered when designing the API were:

- It should provide the necessary functions for currently available add-ons, most notably StartupMaster[1] and Master Password+[2], but also some others [3][4][5], exposing parts of nsIPK11Token that are actually used. This deliberately excludes functions to block or intercept master password popups, which are used by [2] and [6], but would better suit the login storage API proposed in bug 1344788. It also obviously excludes XUL overlays such as the ones used in [2] and [7].

- It should not be a direct wrapper for nsIPK11Token/nsIPK11TokenDB. Bug 1271851 proposes decoupling the master password UI from the PSM and rewriting it from scratch, and I would like this API to remain forward-compatible. Therefore the functions should pertain to the master password and not security devices as in nsIPK11Token. This change also explicitly restricts operation to the internal key token ("software security device").

- Any of the potentially destructive operations like setting up, changing, removing or resetting the master password should pop up a modal chrome dialog for the corresponding operation instead. This makes the API permission safer to grant and covers usage by existing add-ons.

- Keep it simple. I would like to land it before Firefox 57 is frozen, otherwise it will hit hard on everyone affected by bug 177175 who rely on [1] or [2] as a workaround.

- There is one exception where I went against the "keep it simple" rule, by adding a direct option to ask for the master password on startup. This requires blocking on the main thread before other threads are forked off, and WebExtensions are (or soon will be) loaded in a separate thread, which is too late.


Based on the above points, I would like to create a WebExtension API with the following functions:

- query if a master password is set

- query if the password store is unlocked (i.e. the master password was entered)

- ask the user to unlock the password store by entering the master password
    * option: ask even if the password store is already unlocked

- lock the password store (require master password reentry on next use)

- show the dialog for setting up a master password

- show the dialog for changing the master password
    (ATM it's the same one, but let's not hard-code that)

- show the dialog for removing the master password

- show the dialog for resetting the master password

- set a switch to ask the master password on app startup
    * option: quit if the dialog is canceled


Later, an event handler could be added that fires whenever the password store changes state, but it's not a priority as none of the add-ons use it at the moment.

I'm happy to write the patches, but I would first like some review or triage of the proposed interface.


[1] https://addons.mozilla.org/en-US/firefox/addon/startupmaster/
[2] https://addons.mozilla.org/en-US/firefox/addon/master-password/
[3] https://addons.mozilla.org/en-US/firefox/addon/master-password-timeout/
[4] https://addons.mozilla.org/en-US/firefox/addon/master-password-reset/
[5] https://addons.mozilla.org/en-US/firefox/addon/master-password-timeout-upd/
[6] https://addons.mozilla.org/en-US/firefox/addon/masterpassword-firefox/
[7] https://addons.mozilla.org/en-US/firefox/addon/show-password-on-input/
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [design-decision-needed][triaged]
cc'ing in security for some help on this one.
Hi Tamas and Paul, this has been added to the agenda for the June 20 WebExtenion APIs triage meeting. Would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1s2j85VfYKTDftppFU-K7pgRs6sEuMTHVXqf3RL5iYvs/edit#
Would it make sense to get some input from security about this first?
SGTM, andym.
My bad, I should have checked in before adding this to the agenda. 

Paul, would you have time to take a look at this before Tuesday at 10am Pacific? If not, no worries -- we will reschedule this for an upcoming meeting after the All Hands.
I'm afraid I'll be right in the middle of a flight. Is there anything I can contribute in advance?
Kev is also suggesting including Sandy Sage from the lockbox team in this discussion in case there's a touchpoint with this.
Flags: needinfo?(ssage)
Flags: needinfo?(ssage)
I just discussed this with Devin Reams and he will follow up accordingly. I don't think there is currently a touchpoint with us yet but we're still actively watching this thread.
[drum fill]
Last Nightly update finally killed off the last working code: MasterPassword+. I have no further working master password timeout addons (from now on the master password doesn't inactivate in 5 minutes, sending otherwise good security down the dumps).
Priority: -- → P5
Just to clarify, andym, does P5 mean I can start working on a patch?
It's still [design-decision-needed] meaning we aren't sure if it will be accepted. You can start on a patch, but without a comment from a peer you are taking on the risk of working on something that might be rejected. Of course, a proof of concept might persuade people that its a good idea.

cc'ing dreams from comment 9
Flags: needinfo?(dreams)
As the engineering manager of a new team at Mozilla focused on these kinds of things: the initial proposal is a good starting point to discuss.

And while I hate to be discouraging I'll echo the risk andym highlighted: the scrutiny around design details (especially from a security perspective) for this kind of interface will be high.

Worth adding: many of us recognize Master Password's current implementation is not ideal (documented in many other bugs not worth going into here). Thus, all of Master Password is subject to change as our team (among others) plan to explore this. We'd appreciate input like this once that planning gets started. 

This is worth considering: valuable time spent now, on an API folks would come to rely upon, could be wasted (or lead to breaking changes and re-work) based on decisions we start to make on a parallel path. 

That all said, we're very interested in helping make something like this happen and want to work on it together with you all in a thoughtful, secure way. We all sympathize with the need to have _something_ once 57 lands. But, for what its worth, my team would not be able to help contribute here for at least a few months.
Flags: needinfo?(dreams)
To be honest, I think Startup Master a "crude" workaround for a problem which should not exist in the first place. If Firefox would not spam 10 master PW popups in rapid succession after opening (esp. with Sync enabled), there would be no need for this plugin. This is incredibly annoying behaviour and I really can't believe this has not been fixed yet.

Sadly enough, all the other bugs seem to be stale or are being ignored, e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=177175

If you (@Mozilla) really decide to kill this functionality with 57, PLEASE make sure those bugs are either fixed or re-enable the functionality needed for Startup Master in WebExt. Otherwise you'll probably loose a lot of the 21k users this addon has.

Thanks @htamas for maintaining this plugin though, you improved the UX for everyone using a master password a lot over the years.

Sorry for some off-topic.
(In reply to Carsten from comment #14)
> To be honest, I think Startup Master a "crude" workaround for a problem
> which should not exist in the first place.
I partly agree, because this was the main issue why I installed StartupMaster some years ago.
But then I found the option to quit on cancel being very useful!
Of course, it's no real security protection, because one could still run the program deactivating all addons - but it's at least a good way to keep children or low-skilled guys from opening my browser without permission...

(In reply to Tamas Hubai [:htamas] from comment #1)
> - There is one exception where I went against the "keep it simple" rule, by
> adding a direct option to ask for the master password on startup. This
> requires blocking on the main thread before other threads are forked off,
> and WebExtensions are (or soon will be) loaded in a separate thread, which
> is too late.
I had some quick idea how to avoid blocking (at least) all other threads:
Inside the "ask the user to unlock the password store by entering the master password" there could be a check whether any other instance of this dialog is already shown and to focus to that instead of opening a new one. BTW this would solve bug #177175 implicitly.

> - set a switch to ask the master password on app startup
>     * option: quit if the dialog is canceled
Maybe another solution would be to integrate this option in the default preferences, where you could set the master password.
Ending up in integrating Startup Master in the default code base.
But as this does not remove the need for a Master password API, this comment is rather related to bug #177175.
Whiteboard: [design-decision-needed][triaged] → [design-decision-needed][triaged] [needs-follow-up]
Our current solution for restricting access to Firefox by asking the master password on startup (and to all other add-on compatibility issues) is to use Firefox ESR release.
According the current release schedule [1], Firefox ESR 60.0 is about to be released in 3 months.

Are there any plans to have the proposed master password API or any other workaround ready until this release?


[1] https://www.mozilla.org/en-US/firefox/organizations/
Product: Toolkit → WebExtensions
(In reply to Devin Reams (dreams) from comment #13)
> That all said, we're very interested in helping make something like this
> happen and want to work on it together with you all in a thoughtful, secure
> way. We all sympathize with the need to have _something_ once 57 lands. But,
> for what its worth, my team would not be able to help contribute here for at
> least a few months.

ESR Support for 52 is going to end soon, 60 is already out - one more year to land ESR 68...
To tell the truth - I kept using 56.0.2 ignoring all the warnings about "old and unsupported browser". Maybe I'm one of the last, if not even THE last of 21k users that are unable to use the current implementation with a Master Password set.

So, Devin, how long will "at least a few months" last? Did anything happen in the past 11 months?
Should I simply switch to another browser or use a third party password manager?
I'd prefer none of both, as a separate password manager still wouldn't keep my children from starting my browser. Maybe I should go for a separate bookmark manager too?! (sorry for off-topic)
Hey Sven, I chatted a bit with the teams involved at Mozilla as there's a lot of interrelated work in this space.

And while I don't speak entirely on behalf of everyone (Security Engineering, Product Integrity, WebExtensions APIs, Lockbox) I'd like to summarize our thinking to-date:

1. We have not committed to providing a WebExtension API to the master password nor logins, largely for user security and also future reliance (if we create an API and have to change or remove it, we squarely end up back right where we are now, so want to be especially mindful of what is offered)

2. Many agree the technology powering master password (storage, encryption) should be different and there are a few initiatives to evaluate and potentially change or replace some of this.

Therefore, this is all still a moving target and blocked by other work. As I understand it, we may have more to share on the latter point later this year. 

I hope that's helpful and shows a bit more of our thinking...
Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958
Component: Untriaged → General
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.