Opened 3 months ago

Closed 3 months ago

#26127 closed defect (fixed)

Make sure Torbutton and Tor Launcher are not treated as legacy extensions

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff60-esr, TorBrowserTeam201805R
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We should avoid that Tor Launcher and Torbutton get treated as second class add-ons due to them still being based on XPCOM (e.g. getting shown on a separate add-on panel and not listed on the default about:addons page).

Setting extensions.legacy.enabled to true and adding the extensions IDs to extensions.legacy.exceptions should fix that.

Child Tickets

Change History (5)

comment:1 Changed 3 months ago by tom

I was nervous about this, so I checked with aswan.

This pref just changes the UI, so that's okay.

TB will (probably) want to build with MOZ_REQUIRE_SIGNING enabled; otherwise users can install legacy extensions at will.

When we bake a new certificate into the browser to sign our own legacy extensions, we'll need to set the OU to match what's in https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIInstall.jsm#941 to make sure it gets the correct treatment.

comment:2 in reply to:  1 Changed 3 months ago by gk

Keywords: ff60-esr added

Replying to tom:

I was nervous about this, so I checked with aswan.

This pref just changes the UI, so that's okay.

TB will (probably) want to build with MOZ_REQUIRE_SIGNING enabled; otherwise users can install legacy extensions at will.

Yes.

When we bake a new certificate into the browser to sign our own legacy extensions, we'll need to set the OU to match what's in https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIInstall.jsm#941 to make sure it gets the correct treatment.

Good point. I think the signing bits are for #22971, though (even though it seems we need to slightly repurpose that bug).

comment:3 Changed 3 months ago by gk

Keywords: TorBrowserTeam201805R added
Status: newneeds_review

comment:4 in reply to:  3 ; Changed 3 months ago by arthuredelstein

Replying to gk:

A fix for this bug is up in my bug_26127 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_26127&id=4eaeeb9b41e6509b97e3dd216a2e92572fc9b37b). Please review.

I noticed it's possible to set "extensions.legacy.enabled" to false and you can still whitelist torbutton and tor-launcher using "extensions.legacy.exceptions". This might be a little safer by still providing a warning for any other legacy extensions installed.

So I'd suggest removing the "extensions.legacy.enabled" line. The other two lines of code look good to me.

Last edited 3 months ago by arthuredelstein (previous) (diff)

comment:5 in reply to:  4 Changed 3 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to arthuredelstein:

Replying to gk:

A fix for this bug is up in my bug_26127 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_26127&id=4eaeeb9b41e6509b97e3dd216a2e92572fc9b37b). Please review.

I noticed it's possible to set "extensions.legacy.enabled" to false and you can still whitelist torbutton and tor-launcher using "extensions.legacy.exceptions". This might be a little safer by still providing a warning for any other legacy extensions installed.

So I'd suggest removing the "extensions.legacy.enabled" line. The other two lines of code look good to me.

Thanks. I did so and change the exception line and the comment a bit taking into account that we don't want to have a warning for the default them showing up as this is confusing. I removed the whitespace between the extension IDs as sukhbir suggested even though that does not matter.

Applied to tor-browser-60.0.1esr-8.0-1 as commit 30544586f8a98e985e0038b2e905bfddb14e999e.

Note: See TracTickets for help on using tickets.