1379718 - Possible update generation vulnerability

Status: NEW

(Release Engineering :: Release Requests, defect, P3)

Description

[hwine]

Currently, we do not not sign or otherwise protect the integrity some of the MAR tools.

There are 2 mar tools, "mar" & "mbsdiff", which need to be built against "their" version of Firefox. These tools are built for all platforms. [1]

These binaries are not used in their release build process, but only when updaters are being created from their version to some newer version. For example, the mar tool binaries built during the release build of Fx45 are used in producing the updaters for subsequent versions such as Fx45 -> Fx46, and Fx45 -> Fx47.

Between the time these executables are built, and the time they are used to build updaters, they are stored on archive.mozilla.org (which is S3). If someone managed to modify these tools on S3, we currently have no way to detect that modification.

There are several open questions at this point:
 a) Is the "build now, use later" statement correct?

 b) What are the possible impacts of exploiting this vulnerability?

 c) What mitigations do we already have around this issue?


[1] https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/mar-tools/

Comment 1

[Rail Aliiev [:rail]]

a) It's a bit different.

To generate complete mar files we use the "current" mar tools.
For partials we use mozilla-central (!) mar tools. We should use the same mar tools here.

b) with modified mar one can force us to generate evil updates. However our update verification test should catch this issue.
Also, we should probably gpg sign the binaries and verify them as a part of automation

c) for releases we already have update verification tests. Nothing in place for nightly.

Comment 2

[Rail Aliiev [:rail]]

Also https://reviewboard.mozilla.org/r/108864/diff/3#index_header will require use to fix a) and unify the locations.

Comment 3

[hwine]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #1)
> a) It's a bit different.
> 
> To generate complete mar files we use the "current" mar tools.
> For partials we use mozilla-central (!) mar tools. 

I vaguely remember some technical reason for this - perhaps to avoid creating updater watersheds? (It came up during fledgling's initial work on funsize.)

> We should use the same
> mar tools here.

By "same" do you mean "current" or "mozilla-central"?

If it's not possible to be "current", then they must be signed (or otherwise marked to ensure integrity prior to re-use).

Is it correct that 'fun size' (mar generation service) could use these binaries are some (arbitrary) later point in time to generate a new updater? Or, where does funsize retrieve the executables from?

> 
> b) with modified mar one can force us to generate evil updates. However our
> update verification test should catch this issue.

So that means we have both:
 - update verification for every mar file (5 platforms * 96 locales * from 2 last releases ~= 960 minimum tests)
 - update verification tests for binary equivalence of the entire app (directory tree in case of macOS)

Is that right?

> Also, we should probably gpg sign the binaries and verify them as a part of
> automation

Signing would be good, but then we should sign for all platforms (not just windows). If the binaries are used "across release builds", we should mark them somehow so the s3 checker[1] will verify them as well.

> 
> c) for releases we already have update verification tests. Nothing in place
> for nightly.

Now that nightly builds are actively distributed (released), we should enable the updater tests. 


[1] https://github.com/mozilla-services/fx-sig-verify

Comment 4

[[e-mail] (:bhearsum)]

(clarify subject - "updater" is the term we use for the binary that applies updates - not the actual MARs themselves.)

Comment 5

[Rail Aliiev [:rail]]

(In reply to Hal Wine [:hwine] (use NI) from comment #3)
> (In reply to Rail Aliiev [:rail] ⌚️ET from comment #1)
> > a) It's a bit different.
> > 
> > To generate complete mar files we use the "current" mar tools.
> > For partials we use mozilla-central (!) mar tools. 
> 
> I vaguely remember some technical reason for this - perhaps to avoid
> creating updater watersheds? (It came up during fledgling's initial work on
> funsize.)

I don't think we have any technical issues to download a branch/version spcific mar/mbsdiff at least for releases. We already use this approach in release l10n repacks (to generate complete mars).

> 
> > We should use the same
> > mar tools here.
> 
> By "same" do you mean "current" or "mozilla-central"?

I'd say "use mar/basdiff, generated by a build on the same branch and revision". This way we would avoid pinning to "latest" or a particular revision. This may be a bit tricky for Nightly, but not impossible.

> If it's not possible to be "current", then they must be signed (or otherwise
> marked to ensure integrity prior to re-use).
> 
> Is it correct that 'fun size' (mar generation service) could use these
> binaries are some (arbitrary) later point in time to generate a new updater?
> Or, where does funsize retrieve the executables from?

I'm not sure if I understand this correctly...

> > 
> > b) with modified mar one can force us to generate evil updates. However our
> > update verification test should catch this issue.
> 
> So that means we have both:
>  - update verification for every mar file (5 platforms * 96 locales * from 2
> last releases ~= 960 minimum tests)
>  - update verification tests for binary equivalence of the entire app
> (directory tree in case of macOS)
> 
> Is that right?

Correct. This is the bare minimum. We also go back for some randomly selected versions without partial updates to verify the complete updates.


> > Also, we should probably gpg sign the binaries and verify them as a part of
> > automation
> 
> Signing would be good, but then we should sign for all platforms (not just
> windows). If the binaries are used "across release builds", we should mark
> them somehow so the s3 checker[1] will verify them as well.

Yeah, definitely for all platforms. FWIW funsize uses linux64 only binaries.

> > 
> > c) for releases we already have update verification tests. Nothing in place
> > for nightly.
> 
> Now that nightly builds are actively distributed (released), we should
> enable the updater tests. 
> 
> 
> [1] https://github.com/mozilla-services/fx-sig-verify

I forgot to mention, that QE runs UI update verification tests on nightly. Basically what it does:
1) download one of the previous versions
2) check for updates
3) try to apply update
4) verify buildid

Comment 6

[hwine]

Rail helped me get unconfused here. The question is better phrased as "are the mar tool executables ever downloaded from S3".

 - the linux version of the mar utilities are downloaded by fun-size and used in updater generation

 - the mar tools for other platforms are never downloaded

 - the gpg signing and fun-size verification of that signature proposed in comment 1 addresses the issue.

Rail: can you link the gpg signing work bugs here, please?

Comment 7

[Rail Aliiev [:rail]]

I filed Bug 1383857 and bug 1383858.

Comment 8

[Firefox Bug Husbandry Bot]

Bulk change of QA Contact to :jlund, per https://bugzilla.mozilla.org/show_bug.cgi?id=1428483

Comment 9

[Aki Sasaki (not active)]

Now that everything's on Taskcluster, we could use the chainOfTrust.json.asc artifact's shas and download from a task's artifacts rather than ftp.m.o. Unless we're checking the cot artifact's signature, though, there's the chance someone's modified both.

Comment 10

[hwine]

:jlund Has anything changed here?

Comment 11

[Jordan Lund (:jlund)]

good question, @rail - does this bug and the deps still apply? Do we sign the mar tools? Could we use cot based artifacts instead (comment 9)?

How important is this 2 year old bug? It's unclear to me what the vuln severity is.

Comment 12

[Rail Aliiev [:rail]]

(In reply to Jordan Lund (:jlund) from comment #11)

does this bug and the deps still apply?

Yes, we still use the same approach.

Do we sign the mar tools?

Not that I know. mar-tools are also excluded from the checksums file by beetmover, so we can't use SHA512SUMS and SHA512SUMS.asc to verify them.

Could we use cot based artifacts instead (comment 9)?

I think so. It will require some changes in funsize.py.

Comment 13

[Tom Prince [:tomprince]]

I'll note that we already build mar/mbsdiff as a toolchain, and use it[1] for making complete-mars. So it is just the partial generation that needs to be updated to use the toolchain.

[1] Though we don't verify CoT. I think it would be better to do better end-to-end verification than bake it into any particular task.