Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#32498 closed task (fixed)

Consider updating MAR_CHANNEL_ID for nightly build (and maybe alpha too)

Reported by: boklm Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, tbb-update, TorBrowserTeam201911R
Cc: mcs, brade, tbb-team Actual Points: .3
Parent ID: #18867 Points:
Reviewer: Sponsor:

Description

In browser/confvars.sh (from tor-browser.git) we currently set MAR_CHANNEL_ID to torbrowser-torproject-release in all cases.

I see that Mozilla is using a different MAR_CHANNEL_ID for each of their channel (previously by updating browser/confvars.sh, and now by setting it from taskcluster: https://hg.mozilla.org/releases/mozilla-release/rev/66f52bda7e14e26235bd0a43bb68ad11775046e4).

So I am wondering if we should do the same, and use a different MAR_CHANNEL_ID for nightly, and maybe for the alpha too.

Child Tickets

Attachments (1)

0001-fixup-Bug-4234-Use-the-Firefox-Update-Process-for-To.patch (1.5 KB) - added by boklm 11 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 months ago by mcs

Using different MAR channel IDs would prevent the updater from accepting a mar file from a different channel (probably better from a security point of view). If I remember correctly, doing so would also prevent use of MAR tools such as signmar across releases. That would probably be OK, but might lead to some confusion for developers.

If we do switch the MAR channel for in our alpha series we need to think about how to make the transition. I believe that such a transition will require a "watershed" update, but I have not spent a lot of time thinking about it.

comment:2 in reply to:  1 Changed 11 months ago by boklm

Replying to mcs:

Using different MAR channel IDs would prevent the updater from accepting a mar file from a different channel (probably better from a security point of view). If I remember correctly, doing so would also prevent use of MAR tools such as signmar across releases. That would probably be OK, but might lead to some confusion for developers.

Preventing an attacker from being able to switch stable users to alpha seems useful. Although that does not seems to be a major threat, so it is probably not urgent to do it.

Looking at modules/libmar/tool/mar.c, I see that some of the commands have a -H MARChannelID option (for example the one to create a MAR file), but it seems the signing one does not have that option. We normally use the martools from the corresponding version when generating mar and incremental mars, so this should not be an issue.

If we do switch the MAR channel for in our alpha series we need to think about how to make the transition. I believe that such a transition will require a "watershed" update, but I have not spent a lot of time thinking about it.

As there is no urgency to do the switch, maybe we could have an ACCEPTED_MAR_CHANNEL_IDS containing both channels for something like 9 months, before doing the switch without a watershed update (or taking advantage the watershed update to the next ESR if one is needed). This would break update for alpha users who did not update in a few months, but maybe there are not so many users of 9 month-old alpha versions.

For the nightly, switching channels is already prevented by using different signing keys, but since there is no transition needed, maybe we can use a separate channel ID from the beginning.

comment:3 Changed 11 months ago by boklm

Keywords: TorBrowserTeam201911R added; boklm201911 TorBrowserTeam201911 removed
Status: newneeds_review

I attached a patch updating the nightly channel name.

For the alpha, the patch adds torbrowser-torproject-alpha to ACCEPTED_MAR_CHANNEL_IDS, but does change MAR_CHANNEL_ID yet.

comment:4 Changed 11 months ago by mcs

The patch looks good to brade and me. Our only question is: Have you confirmed that configuring a comma-separated list for ACCEPTED_MAR_CHANNEL_IDS works correctly? It looks like it is supported, but it would be very bad to upgrade people to an alpha that cannot handle any future updates ;)

comment:5 in reply to:  4 ; Changed 11 months ago by boklm

Replying to mcs:

The patch looks good to brade and me. Our only question is: Have you confirmed that configuring a comma-separated list for ACCEPTED_MAR_CHANNEL_IDS works correctly? It looks like it is supported, but it would be very bad to upgrade people to an alpha that cannot handle any future updates ;)

I have not confirmed that it works, but I saw an example of comma-separated list in testing/mozharness/configs/merge_day/central_to_beta.py.

I can try tomorrow changing an update-settings.ini in a previous alpha and see if the update still works.

comment:6 in reply to:  5 ; Changed 11 months ago by boklm

Replying to boklm:

Replying to mcs:

The patch looks good to brade and me. Our only question is: Have you confirmed that configuring a comma-separated list for ACCEPTED_MAR_CHANNEL_IDS works correctly? It looks like it is supported, but it would be very bad to upgrade people to an alpha that cannot handle any future updates ;)

I have not confirmed that it works, but I saw an example of comma-separated list in testing/mozharness/configs/merge_day/central_to_beta.py.

I can try tomorrow changing an update-settings.ini in a previous alpha and see if the update still works.

So I tried updating 9.5a1 to 9.5a2, after changing update-settings.ini to set ACCEPTED_MAR_CHANNEL_IDS=torbrowser-torproject-alpha, and as expected the updated failed.

When setting ACCEPTED_MAR_CHANNEL_IDS=torbrowser-torproject-alpha,torbrowser-torproject-release in update-settings.ini the update worked.

comment:7 in reply to:  6 Changed 11 months ago by mcs

Replying to boklm:

So I tried updating 9.5a1 to 9.5a2, after changing update-settings.ini to set ACCEPTED_MAR_CHANNEL_IDS=torbrowser-torproject-alpha, and as expected the updated failed.

When setting ACCEPTED_MAR_CHANNEL_IDS=torbrowser-torproject-alpha,torbrowser-torproject-release in update-settings.ini the update worked.

Thanks for performing those tests. I think this fix is ready to be merged.

comment:8 Changed 11 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Okay, let's do it. This is on tor-browser-68.2.0esr-9.5-1 now (commit 9579237949269ef391e474f222011fa218608fae).

comment:9 Changed 11 months ago by gk

boklm: don't forget to set your points.

comment:10 Changed 11 months ago by boklm

Actual Points: .3

I opened #32612 to change the MAR_CHANNEL_ID for the alpha.

Note: See TracTickets for help on using tickets.