Closed
Bug 1308261
Opened 8 years ago
Closed 8 years ago
Add hash to mozAddonManager.createInstall() options
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: aswan, Assigned: aswan)
Details
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
kmag
:
review+
bzbarsky
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details |
AddonManager installs can be given the hash of the xpi to be installed that gets verified after downloading but before actually installing an extension. mozAddonManager should expose this as well.
Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8798679 [details] Bug 1308261 Add hash to options for mozAddonManager.createInstall() https://reviewboard.mozilla.org/r/84106/#review82876 ::: dom/webidl/AddonManager.webidl:51 (Diff revision 1) > Promise<void> cancel(); > }; > > dictionary addonInstallOptions { > required DOMString url; > + DOMString hash; `DOMString? hash` ::: toolkit/mozapps/extensions/test/browser/browser_webapi_install.js:211 (Diff revision 1) > - is(addons[0], null, "Addon was uninstalled"); > + is(addons[0], null, "Addon was uninstalled"); > -})); > + }); > +} > + > +add_task(makeRegularTest({url: XPI_URL}, "a basic install works")); > +add_task(makeRegularTest({url: XPI_URL, hash:XPI_SHA}, "install with hash works")); Nit: `hash: XPI_SHA` ::: toolkit/mozapps/extensions/test/browser/browser_webapi_install.js:225 (Diff revision 1) > error: null, > }, > } > ]; > > - yield testInstall(browser, XPI_URL, steps, "canceling an install works"); > + yield testInstall(browser, {url:XPI_URL}, steps, "canceling an install works"); Nit: `url: XPI_URL` (same for the changes below)
Attachment #8798679 -
Flags: review?(kmaglione+bmo) → review+
Comment 3•8 years ago
|
||
Should this be required?
Assignee | ||
Comment 4•8 years ago
|
||
[Tracking Requested - why for this release]: At some point, Test Pilot will being using the mozAddonManager API to install its add-on. As described in bug 1308251, that process is currently broken when certain other extensions that MITM outgoing SSL connections are installed. The underlying AddonManager allows a checksum to be supplied for an add-on installation, since that checksum provides an end-to-end check on the thing being downloaded, it relaxes other rules about a secure download channel. The patch for this bug exposes that capability up to mozAddonmanager, which will allow us to make Test Pilot installation work even when these MITM'ing extensions are present.
status-firefox50:
--- → affected
status-firefox51:
--- → affected
status-firefox52:
--- → affected
tracking-firefox50:
--- → ?
tracking-firefox51:
--- → ?
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Andy McKay [:andym] from comment #3) > Should this be required? It is not required in the underlying AddonManager API, I don't see a good reason to make it any different for mozAddonManager.
Comment hidden (mozreview-request) |
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8798679 [details] Bug 1308261 Add hash to options for mozAddonManager.createInstall() https://reviewboard.mozilla.org/r/84106/#review82934 ::: dom/webidl/AddonManager.webidl:51 (Diff revision 2) > Promise<void> cancel(); > }; > > dictionary addonInstallOptions { > required DOMString url; > + DOMString? hash; You should probably just make this: DOMString? hash = null; so the "not passed" case doesn't need to be worried about on the callee side, since semantically you want null and not passed to act the same anyway. A comment about what null means (and that null and "" are semantically equivalent, because afaict they are) would be a good idea too. That's all assuming you do in fact want null to mean "no hash" as opposed to getting stringified to "null". r=me on the webidl changes with that.
Comment hidden (mozreview-request) |
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8798679 [details] Bug 1308261 Add hash to options for mozAddonManager.createInstall() https://reviewboard.mozilla.org/r/84106/#review82942
Attachment #8798679 -
Flags: review+
Comment 10•8 years ago
|
||
Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0c6ac0ce7b39 Add hash to options for mozAddonManager.createInstall() r=bz,kmag
Comment 11•8 years ago
|
||
Andy, Chuck, Can you please file bugs to begin using this in AMO and Test Pilot?
Flags: needinfo?(charmston)
Flags: needinfo?(amckay)
Comment 12•8 years ago
|
||
https://github.com/mozilla/testpilot/issues/1555
Flags: needinfo?(charmston)
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0c6ac0ce7b39
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 14•8 years ago
|
||
https://github.com/mozilla/addons-frontend/issues/1208
Flags: needinfo?(amckay)
Comment 15•8 years ago
|
||
Tracking to avoid some issues with system addons and MITM software. Please fill the uplift request, Ritu will decide whether to take this for 50.
Assignee | ||
Comment 16•8 years ago
|
||
Comment on attachment 8798679 [details] Bug 1308261 Add hash to options for mozAddonManager.createInstall() Approval Request Comment [Feature/regressing bug #]: mozAddonManager [User impact if declined]: This patch allows AMO and testpilot to add an additional layer of protection against a potential MITM attacker modifying addons to be installed. If uplift is declined, we won't have that additional protection. [Describe test coverage new/current, TreeHerder]: Tests are included in this patch [Risks and why]: see above [String/UUID change made/needed]: none
Flags: needinfo?(aswan)
Attachment #8798679 -
Flags: approval-mozilla-beta?
Attachment #8798679 -
Flags: approval-mozilla-aurora?
Comment on attachment 8798679 [details] Bug 1308261 Add hash to options for mozAddonManager.createInstall() This is needed to make TestPilot experiments more safe, stabilized on Nightly for a week, Aurora51+, Beta50+
Attachment #8798679 -
Flags: approval-mozilla-beta?
Attachment #8798679 -
Flags: approval-mozilla-beta+
Attachment #8798679 -
Flags: approval-mozilla-aurora?
Attachment #8798679 -
Flags: approval-mozilla-aurora+
Comment 18•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/27874e8f6911
Comment 19•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4b1f9598c7e1
You need to log in
before you can comment on or make changes to this bug.
Description
•