User:Dolske/Password Manager Security Review

From MozillaWiki
Jump to: navigation, search
Nutshell review results:
Attended: dolske, dveditz, ashughes, juanb, tomcat.
Filed bug 394610, bug 394611, bug 394612.
Will evaluate doing something about bug 359675 for FF3.

Status

Feature tracking bug
  • bug 371000 - Password Manager update metabug
  • bug 374723 - Password Manager should be a JavaScript-based component

Has a design review been completed?

Yes, as part of the review for 374723. It was bounded by being a reimplementation of an existing feature.

When do you anticipate the feature landing

Password Manager was an existing feature. The most significant update (the JS rewrite) landed for Alpha 5. Other modifications have been landing since then. Notable remaining work:

  • bug 362576 autocomplete="off" should prevent filling passwords in addition to remembering passwords
  • bug 226735 replace modal pre-submit save password dialog with post-submit bar

Overview

Describe the goals and objectives of the feature here.

Goals for the current work:

  • Simplify existing code
  • Refactor code to allow for adding new features and fixing long-outstanding bugs
  • Switch to JS to improve security and maintainability.
  • UI changes per PRD.

Goals for feature in general:

  • Allow users to store site logins so they don't have to remember them
  • Encourage, as a result, using stronger passwords
  • Keep stored logins up to date by observe authentications (to watch for password changes)

Use Cases

Describe the primary use cases for the feature here.

Requirements

PRD: http://wiki.mozilla.org/Firefox3/Product_Requirements_Document#Password.2C_Identity

Schedule

JS rewrite landed in Alpha 5. Remaining work is just bug-by-bug, setting Target Milestone as needed.

UI Design Documentation

Alex Faaborg's blog entry on redesigning notification, although this specific design was rejected in favor of just using a notification bar (like popup notifications).

Design Impact

Security and Privacy

  • What security issues do you address in your project?
  • Is system or subsystem security compromised in any way if your project's configuration files / prefs are corrupt or missing?
  • Include a thorough description of the security assumptions, capabilities and any potential risks (possible attack points) being introduced by your project.

Exported APIs

  • Please provide a table of exported interfaces (APIs, ABIs, protocols, UI, etc.)

nsILoginInfo.idl nsILoginManager.idl nsILoginManangerStorage.idl

In user's profile: signons2.txt key3.db (dependancy)

Current UI: In preferences, under "Security" tab, a "Passwords" section...

  • "Remember password for sites" checkbox
  • "Use a master password" checkbox
  • "Change master password" button [PSM]
  • "Show Passwords" button
    • Popup window with a list of stored logins
  • "Exceptions" button
    • Popup window with a list of sites for which logins should not be saved

Prompts for HTTP/FTP authentication, changing passwords.

Notification bar to allow saving a login (for a HTTP/FTP authentication, or submission of a form-based login).

  • Does it interoperate with a web service? How will it do so?

No. A simple "store-my-passwords.com" could, by implementing a nsILoginManagerStorage module. More complex authentication schemes (eg OpenID, CardSpace) would require deeper changes to Mozilla.

  • Explain the significant file formats, names, syntax, and semantics.

signons2.txt: Stored encrypted usernames and passwords, along with data used to identify where the login can be used (site, protocol, HTTP realm, form submit URL, etc.). See storage-legacy.js for documentation of the format.

key3.db: Stores the encryption key used in signons2.txt. [The key itself is encrypted with a key derived from the master password.]

  • Are the externally visible interfaces documented clearly enough for a non-Mozilla developer to use them successfully?

Yes. (IDL, devmo, straightforward)

  • Does it change any existing interfaces?

Yes. It completely replaces all of the existing Password Manager interfaces. They were poorly designed and not well documented.

Web Compatibility

  • Does the feature had any impact on Web compatibility?

Yes. There is no standard for programatically locating logins in a document's form, so there is a certain degree of fuzzy logic involved in doing this. Password fields are easy to find, but the Password Manager must try to determine is a username field is present (and where it is), and how to handle multiple password fields. As there is no standard, login forms can come in many different flavors.

The previous password manager usually handled the most common cases, but was not very robust. The new code is more robust and handles more cases, and has a test suite to exercise many variations. There is some risk that the change in algorithms could break a small number of sites, but overall more sites should work.

Performance

  • How will the project contribute (positively or negatively) to "perceived performance"?

There should not be any effect on performance-sensitive areas.

  • What are the performance goals of the project? How were they evaluated? What is the test or reference platform and baseline results?

No specific performance goals.

  • Will it require large files/databases (for example, browsing history)?

No. For normal usage, signons2.txt should not be very large. I tested with an absurdly large file (~1000 stored logins), and performance was not noticeably affected.

Reliability

  • What failure modes or decision points are presented to the user?

When a new login is detected, the user can opt to save to the login (or not). Password changes to existing logins are generally handled automatically, but in some cases the password manager will prompt the user to confirm an ambigious action.

  • Can its files be corrupted by failures? Does it clean up any locks/files after crashes?

The only file it writes to, signons2.txt, is edited using a nsISafeOutputStream. The original file is only updated (atomically) upon success. There is currently an outstanding bug to have it better deal with a pre-existing corrupted file.

l10n and a11y

  • are any strings being changed or added?

There may be some string changes as the UI is polished, but the major work to date has been accomplished with the existing strings.

  • are all UI elements available through accessibility technologies?

Yes. (I'm assuming the notification bar is?)

Installation, Upgrade/Downgrade/Sidegrade, and platform requirements

  • Does it equally support all Tier-1 platforms?

Yes.

  • Does is have a hardware requirement (or increase minimum requirements)?

No.

  • Does it require changes to the installer?

No.

  • Does it impact updates?

No.

  • list the expected behavior of this feature/function when Firefox is upgraded to a newer minor release, downgraded by installation of an earlier revision, or re-installed (same version)

No new changes. A change introduced in Firefox 2.0.0.2 bug 360493 migrates the signons.txt file to signons2.txt, due to incompatible changes required for a security issue.

configuration

  • Can the end user configure settings, via a UI or about:config? Hidden prefs? Environment variables?

Prefs are exposed in the Firefox Preferences UI. (One pref, signons.autofillForms, is only in about:config. It was added in bug 360493 as a workaround to prevent autofilling forms without disabling the password manager entirely.)

  • Are there build options for developers? [#ifdefs, ac_add_options, etc.]

The signons.debug pref is available for logging extra debug information to the console, to help with runtime diagnosis of problems.

  • What ranges for the tunable are appropriate? How are they determined?

No tunables; all prefs are boolean on/off.

  • What are its on-going maintenance requirements (e.g. Web links, perishable data files)?

None.

Relationships to other projects - are there related projects in the community?

  • If so, what is the proposal's relationship to their work? Do you depend on others' work, or vice-versa?

There is community interest in having integrated support for platform-specific password manangement, such as OS X's Keychain (bug 106400)and Gnome's Key Ring (bug 309807). A goal of the current work was to make it easier to such support.

  • Are you updating, copying or changing functional areas maintained by other groups? How are you coordinating and communicating with them? Do they "approve" of what you propose?

No. There is a small amount over overlap with NSS/PSM (which contain the code responsible for the master password and encryption), but this functionality is not changing.

Documentation

  • Do built-in Help pages need modified?

No. The existing text describing saving passwords and using a master password remains accurate.

  • Documentation for developer.mozilla.org?

Complete:

nsILoginManager

nsILoginInfo

Using nsILoginManager

Other

any other implementation or design related documentation

Discussion & Implications

Caveats / What We've Tried Before

links to previous design documents, discussions, etc.

References

links to external documents that could inform the design of the feature