canmove, Confirmed users
432
edits
User:Dolske/Password Manager Security Review: Difference between revisions
User:Dolske/Password Manager Security Review (view source)
Revision as of 18:01, 31 July 2007
, 31 July 2007Update more stuff.
(mid-edit checkpoint...) |
(Update more stuff.) |
||
| Line 5: | Line 5: | ||
'' Has a design review been completed?'' | '' 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'' | '' 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. | Password Manager was an existing feature. The most significant update (the JS rewrite) landed for Alpha 5. Other modifications have been landing since then. | ||
Remaining work: notification bar, UI cleanup | Remaining work: notification bar, UI cleanup for M8. | ||
= Overview = | = Overview = | ||
''Describe the goals and objectives of the feature here.'' | ''Describe the goals and objectives of the feature here.'' | ||
* 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. | |||
== Use Cases == | == Use Cases == | ||
| Line 67: | Line 73: | ||
* Explain the significant file formats, names, syntax, and semantics. | * Explain the significant file formats, names, syntax, and semantics. | ||
signons2.txt: | 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: | |||
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? | * Are the externally visible interfaces documented clearly enough for a non-Mozilla developer to use them successfully? | ||
| Line 76: | Line 83: | ||
* Does it change any existing interfaces? | * Does it change any existing interfaces? | ||
Yes. It completely replaces all of the existing Password | Yes. It completely replaces all of the existing Password Manager interfaces. They were poorly designed and not well documented. | ||
== Web Compatibility == | == Web Compatibility == | ||
| Line 91: | Line 98: | ||
* What are the performance goals of the project? How were they evaluated? What is the test or reference platform and baseline results? | * 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)? | * 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 == | == Reliability == | ||
* What failure modes or decision points are presented to the user? | * 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? | * 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. | 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 == | == l10n and a11y == | ||
| Line 104: | Line 119: | ||
* are all UI elements available through accessibility technologies? | * are all UI elements available through accessibility technologies? | ||
Yes. (notification bar | Yes. (I'm assuming the notification bar is?) | ||
== Installation, Upgrade/Downgrade/Sidegrade, and platform requirements == | == Installation, Upgrade/Downgrade/Sidegrade, and platform requirements == | ||
| Line 124: | Line 139: | ||
*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) | *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 == | == configuration == | ||
* Can the end user configure settings, via a UI or about:config? Hidden prefs? Environment variables? | * 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.] | * Are there build options for developers? [#ifdefs, ac_add_options, etc.] | ||
| Line 133: | Line 153: | ||
* What ranges for the tunable are appropriate? How are they determined? | * What ranges for the tunable are appropriate? How are they determined? | ||
No tunables | No tunables; all prefs are boolean on/off. | ||
* What are its on-going maintenance requirements (e.g. Web links, perishable data files)? | * What are its on-going maintenance requirements (e.g. Web links, perishable data files)? | ||
| Line 141: | Line 161: | ||
== Relationships to other projects - are there related projects in the community? == | == 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? | * If so, what is the proposal's relationship to their work? Do you depend on others' work, or vice-versa? | ||
* 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? | |||
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 == | == Documentation == | ||
* Do built-in Help pages need modified? | * 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? | * Documentation for developer.mozilla.org? | ||
Complete. | Complete: | ||
[http://developer.mozilla.org/en/docs/nsILoginManager nsILoginManager] | |||
[http://developer.mozilla.org/en/docs/nsILoginInfo nsILoginInfo] | |||
[http://developer.mozilla.org/en/docs/Using_nsILoginManager Using nsILoginManager] | |||
== Other == | == Other == | ||