1431342 - Latest aurora channel update has invalid macOS code signature

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[lucideer]

Attachment: Screen Shot 2018-01-18 at 08.55.26.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Safari/604.1.38

Steps to reproduce:

Tried to run Firefox Developer Edition after autoupdate on macOS Sierra


Actual results:

 - Firstly, my Little Snitch install automatically blocked all network requests from the new Firefox install stating the code signature was invalid (See attached screenshot)

- Secondly, when starting Firefox, I was immediately prompted with a dialog saying something like "Firefox Developer Edition is not your default browser. Do you want to set it?". Firefox Developer Edition has always been my default browser, since I bought this machine. I have never before now seen this dialog on update.


Expected results:

Updated package should not be flagged and blocked as as invalid

Firefox should remain my default browser if I have set it as such before the update

Comment 1

[lucideer]

Attachment: Screen Shot 2018-01-18 at 08.56.11.png

Comment 2

[lucideer]

Attachment: Screen Shot 2018-01-18 at 09.32.17.png

Comment 3

[lucideer]

Attachment: Screen Shot 2018-01-18 at 09.49.36.png

Comment 4

[lucideer]

Further info:

Running

> $ codesign --verify --deep --verbose /Applications/FirefoxDeveloperEdition.app

gives


> /Applications/FirefoxDeveloperEdition.app: a sealed resource is missing or invalid
> file modified: /Applications/FirefoxDeveloperEdition.app/Contents/Resources/defaults/pref/channel-prefs.js

and

> $ cat /Applications/FirefoxDeveloperEdition.app/Contents/Resources/defaults/pref/channel-prefs.js```

gives

> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

> pref("app.update.channel", "aurora");

Comment 5

[Molly Howell (she/her) (no longer active)]

I can reproduce this signature error myself when updating from 58.0b2, so I think I see what's going on. I don't know exactly how this happened, but it looks like starting with 58.0b3, the way that channel-prefs.js is generated changed so that it started including a comment with the path to the source file. The Mac version of channel-prefs.js should have this comment right before its last line of text:

> //@line 6 "/builds/worker/workspace/build/src/browser/app/profile/channel-prefs.js"

But the updater isn't making that change, apparently because channel-prefs.js is on the add-if-not list, meaning the updater doesn't touch that file unless it wasn't there at all before. So the functional content of the file is the same, but we're not updating it to exactly the same version as in the app bundle that we're signing.


Reporter, you should be able to fix this by just downloading a fresh copy of the browser and overwriting yours. And thanks for letting us know about this.

Comment 6

[Robert Strong (they/them - no direct email)]

This might be due to bug 1415742

Comment 7

[Robert Strong (they/them - no direct email)]

I verified that this was caused by bug 1415742

Comment 8

[Chris Manchester (limited bugmail, email directly)]

I can figure out how to stop generating that comment, but I don't think I understand this issue, or whether that's in fact the correct way to fix this. From the description it seems like we will encounter this if we ever change that file.

Robert, can you offer some advice here?

Comment 9

[Robert Strong (they/them - no direct email)]

The file is there so QA can manually change the value when verifying updates. For the typical user case the file is never changed which allows OS X v2 signing to continue to verify after an app update. When we have had to change the file releng will create an update that updates that file instead of only adding it if it doesn't exist already.

Comment 10

[Chris Manchester (limited bugmail, email directly)]

Adding "-Fsubstitution" to all of our preprocessed final target files is an idea, but we don't have a way to pass pp flags to the faster make backend, so this ends up creating a new problem there, either because making it aware of channel-prefs.js without any directives in it will cause an error, or because we can't pass pp flags through install manifests.

We should just do a partial backout of bug 1415742 until we can figure out the interaction between PP flags and mozbuild.

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1431342 - Move channel-prefs.js processing back to Makefile.in to avoid changing its content.

Review commit: https://reviewboard.mozilla.org/r/214154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214154/

Comment 12

[Robert Strong (they/them - no direct email)]

Why not just do hg backout 44ad5267ace9 ?

Comment 13

[Chris Manchester (limited bugmail, email directly)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12)
> Why not just do hg backout 44ad5267ace9 ?

The other things in that patch aren't causing problems and block bug 1255485.

Comment 14

[Robert Strong (they/them - no direct email)]

Comment on attachment 8943772 [details]
Bug 1431342 - Move channel-prefs.js processing back to Makefile.in to avoid changing its content.

https://reviewboard.mozilla.org/r/214154/#review219888

Looks fine

Comment 15

[Robert Strong (they/them - no direct email)]

bhearsum, catlee, rail,
Any clients that installed Firefox after bug 1415742 landed will have a different channel-prefs.js file which will make OS X signature verification fail when launching Firefox. As I understand it, the signature verification will occur after an OS X update though I am by no means all that knowledgeable about OS X signing. It might be a good thing to create a mar that force adds the channel-prefs.js file for these clients though the workaround of just reinstalling Firefox might be ok.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be1ce4548b2e
Move channel-prefs.js processing back to Makefile.in to avoid changing its content. r=rstrong

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/be1ce4548b2e

Comment 18

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8943772 [details]
Bug 1431342 - Move channel-prefs.js processing back to Makefile.in to avoid changing its content.

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1415742 (build system refactoring)
[User impact if declined]:
Signature verification failure (see comment 0) 
[Is this code covered by automated tests?]:
No
[Has the fix been verified in Nightly?]:
No
[Needs manual test from QE? If yes, steps to reproduce]: 
No
[List of other uplifts needed for the feature/fix]:
None
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
This reverts a change made in bug 1415742 by removing the comment that introduced the content mismatch and therefore the error. I have verified the content of channel-prefs.js in automation builds on mozilla-central following this change.
[String changes made/needed]:
None

Comment 19

[[e-mail] (:bhearsum)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #15)
> bhearsum, catlee, rail,
> Any clients that installed Firefox after bug 1415742 landed will have a
> different channel-prefs.js file which will make OS X signature verification
> fail when launching Firefox. As I understand it, the signature verification
> will occur after an OS X update though I am by no means all that
> knowledgeable about OS X signing. It might be a good thing to create a mar
> that force adds the channel-prefs.js file for these clients though the
> workaround of just reinstalling Firefox might be ok.

Do we have any idea how many clients are affected? How would we target them (buildid I guess)?

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 8943772 [details]
Bug 1431342 - Move channel-prefs.js processing back to Makefile.in to avoid changing its content.

58 is on release, moving flag in case we end up taking this for a dot release.  From chatting with mhowell it doesn't sound like this is cause for panic the day before we ship 58, as this has been on nightly and beta for a while before anyone noticed.

Comment 21

[lucideer]

> it doesn't sound like this is cause for panic

Just to add some context to this discussion in terms of urgency/panic:

- I run separate, 3rd-party, firewall software (Little Snitch) that performs extra signing checks and blocks any connections from binaries that fail checks. Apple's own in-built signing checks did not catch this, and Firefox continued to open and operate fine (albeit without network access). So I don't think this will affect many users unless they're running Little Snitch.

- Firefox did however somehow "lose" its placement as the "Default browser" for my system - I'm unsure how/why this happened.

Comment 22

[Robert Strong (they/them - no direct email)]

(In reply to Ben Hearsum (:bhearsum) from comment #19)
><snip>
> Do we have any idea how many clients are affected?
I do not but perhaps someone else does.

> How would we target them
> (buildid I guess)?
I suspect that would suffice.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked. We'd like to include this in a 58.x dot release as a ride-along.

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8943772 [details]
Bug 1431342 - Move channel-prefs.js processing back to Makefile.in to avoid changing its content.

This fix has been in Nightly and Beta for ~2 weeks now. Taking it as a safe ride-along in 58.0.2

Comment 25

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-release/rev/67e1226dbe40