Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • Tor Browser Tor Browser
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 875
    • Issues 875
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 14
    • Merge requests 14
  • Deployments
    • Deployments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • Repository
  • Wiki
    • Wiki
  • Activity
  • Graph
  • Create a new issue
  • Commits
  • Issue Boards
Collapse sidebar

Admin message

GitLab has been upgraded GitLab 16, please report any issues!

  • The Tor Project
  • Applications
  • Tor BrowserTor Browser
  • Issues
  • #41668
Closed
Open
Issue created Mar 07, 2023 by Pier Angelo Vendrame@pierovMaintainer

Move part of the updater patches to base browser

Currently, we have 3 commits for changes to the updater in Tor Browser:

  • Bug 4234: Use the Firefox Update Process for Tor Browser.
  • Bug 13379: Sign our MAR files.
  • Bug 32658: Create a new MAR signing key

Bug 32658: Create a new MAR signing key

Let's start from the last one, but also the easiest one: we replace the Firefox's MAR signing keys with our keys.

This commit should stay in Tor Browser, but we might want a set of keys also for Base Browser.

This has two possibilities:

  1. we add a commit only for base-browser and we do not include it in Tor Browser (it must always be the last one, or part of a tail of commits that aren't included in TBB)
  2. we include the commit also in TBB without caring too much, since we overwrite the contents anyway.

I don't mind the second option: it allows to tag -build1 at the commit that we use as a fork point, and we can use it for diffs or other interesting git tricks.

Also, it allows to use only one MR to do the rebases.

Actually, there's a third alternative: we can inject the base-browser key in tor-browser-build, since I guess we'd use it only for nightlies (see https://gitlab.torproject.org/tpo/applications/tor-browser/-/wikis/Updating).

Bug 13379: Sign our MAR files.

This commit is really two parts:

  1. #13379 (closed): Sign our MAR files. Use NSS for the mar tools
  2. #19121 (closed): reinstate the update.xml hash check

It also refers #18900 (closed), but it is no longer included, and we should remove it from the description.

I'm up for splitting it again.

Use NSS for MARs

Using NSS for MARs was discussed also by Mozilla, and they decided not to on Windows and macOS, but to prefer system crypto.

We were of the opposite opinion (and fwiw, I agree with the choice). TL;DR: we prefer trusting open source code instead of OS opaque boxes (well, one could say that if you don't trust the OS you should not run TBB there), and NSS is production ready and used on Linux anyway.

I think we could even add a configure option, and then upstream to Moz.

Check hashes for MARs

This reverted part of https://bugzilla.mozilla.org/show_bug.cgi?id=862173.

Moz's reasons were to reduce I/O usage, and that checking signatures were already sufficient.

I tend to agree, I don't know if it actually makes sense to keep checking hashes if files are signed.

Enabling the check is easier, whereas disabling it is much more involved: we'd need to keep the hashes in the XML for a long time, until it's okay for us having old clients failing to do even full upgrades, just because we're missing a property in the XML.

Moreover, uplifting this is more involved, because Mozilla wanted to get rid of it, and we'd need at least to restore tests.

In any case, we could update the function to create the hex representation of the hash, by taking the snippet of code from here (and using padStart instead of their toHexString).

Bug 4234: Use the Firefox Update Process for Tor Browser.

General overview: here we have also several version-related (and channel-related!) changes. The borders of the changes are not actually well defined. That's in part also what I meant in !568 (merged) (and related to #41647 (closed)).

I think we could keep this commit for actual Tor Browser customization (because of the subject), but create a new commit for the Base Browser stuff.

  • browser/app/Makefile.in, browser/installer/package-manifest.in: this is very interesting, because it's related to LaunchServices, which are a similar thing to remoting. In this case it is for stuff about the updater, but still, somethign we need to explore a little bit more
  • browser/app/profile/firefox.js: we can bring this change to 001-base-profile.js. The comment doesn't apply to Tor Browser because we modify the behavior of getAppUpdateAutoEnabled, but we can add a comment about this on the 001-base-profile.js, instead of modifying firefox.js, I think
  • browser/base/content/aboutDialog-appUpdater.js, browser/base/content/aboutDialog.js, browser/components/preferences/main.js: related to versions and channels. We could move to a commit on their own?
  • browser/components/BrowserContentHandler.jsm: homepage override, I need to investigate it
  • browser/components/customizableui/content/panelUI.inc.xhtml: removes the message that tabs will be restored after the update, it can remain here
  • browser/confvars.sh: channel names, they include torbrowser/torproject. We should have a commit for Tor Browser, and one for Privacy Browser. We could still customize something for Base Browser here, then change it.
  • build/application.ini.in: URL of the updater, so like the previous file.
  • build/moz.configure/init.configure: version-related. Also, a1 is treated as a nightly, which is sorta okay, since it's the first alpha of the series, so things are expected to be very temporary. Still, we should remove it, if we wanted to do things properly.
  • config/createprecomplete.py: things for the default profile, they are Tor Browser-only. Also, references to the symlinks, that we should remove for #34319
  • devtools/client/aboutdebugging/src/actions/runtimes.js: this should go to the branding commit. Also, we're using the purple logo for all the channels, maybe we could differentiate them. I still haven't looked where this is shown.
  • toolkit/modules/UpdateUtils.jsm: what if we changed PER_INSTALLATION_PREFS_PLATFORMS at the beginning of the file, instead?
  • toolkit/mozapps/extensions/AddonManager.jsm: can we simplify the code, since we had the 12.0 watershed, and we're starting from new? Okay for the updater, but could be part of a commit only of things that need to happen when we detect a new version.
  • toolkit/mozapps/extensions/test/browser/head.js, toolkit/mozapps/extensions/test/xpcshell/head_addons.js: version-related stuff (and in tests! We don't run them, but since it's a knwon possible modification, we could keep it)
  • toolkit/mozapps/update/UpdateService.jsm: several types of changes!
    • make the updater wait for the bootstrap: this should go on the Tor Browser side
    • stop privilege elevation and the Windows service (about the Windows service: if we end up doing the system installation, we might enable it?)
    • version stuff
    • things to analyze requirements on the client side, and avoid sending them on the server side
    • disable 32 to 64 bits on Windows (not a problem for PB, but I'm quite curious to know why we didn't want to make people migrate)
  • toolkit/mozapps/update/UpdateServiceStub.jsm: update agent stuff, it's okay to keep it here, maybe we could switch from AppConstants.TOR_BROWSER_UPDATE to BASE_BROWSER_VERSION
  • toolkit/mozapps/update/common/updatehelper.cpp: let's keep it, and see in a separate issue for PB
  • toolkit/mozapps/update/updater/launchchild_osx.mm, toolkit/xre/MacLaunchHelper.h, toolkit/xre/MacLaunchHelper.mm: disable elevation, let's keep it
  • toolkit/mozapps/update/updater/moz.build: versioning stuff, but related to the update keys
  • toolkit/mozapps/update/updater/updater.cpp:
    • symlink (see above, to remove)
    • remove elevation
    • something about the sockets, for IPC stuff (maybe Tor Browser only?)
    • skip profile/Tor files (only Tor Browser)
    • something under TOR_BROWSER_DATA_OUTSIDE_APP_DIR but only for macOS, we're already removing it in !568 (merged)
  • (to be continued)

TODO (even more involved than the previous one)

Edited Mar 09, 2023 by Pier Angelo Vendrame
Assignee
Assign to
Time tracking