Closed Bug 1431342 Opened 2 years ago Closed 2 years ago

Latest aurora channel update has invalid macOS code signature

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set

Tracking

(firefox-esr52 unaffected, firefox58+ fixed, firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox58 + fixed
firefox59 --- fixed

People

(Reporter: bugzilla, Assigned: chmanchester)

References

Details

(Keywords: regression)

Attachments

(5 files)

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
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");
Component: Untriaged → Application Update
OS: Unspecified → Mac OS X
Product: Firefox → Toolkit
Hardware: Unspecified → x86
Component: Application Update → General
Product: Toolkit → Release Engineering
Version: 58 Branch → other
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.
I verified that this was caused by bug 1415742
Assignee: nobody → cmanchester
Blocks: 1415742
Component: General → Build Config
Keywords: regression
Product: Release Engineering → Core
Version: other → unspecified
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?
Flags: needinfo?(robert.strong.bugs)
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.
Flags: needinfo?(robert.strong.bugs)
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.
Why not just do hg backout 44ad5267ace9 ?
(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 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
Attachment #8943772 - Flags: review?(robert.strong.bugs) → review+
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.
Flags: needinfo?(catlee)
Flags: needinfo?(bhearsum)
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be1ce4548b2e
Move channel-prefs.js processing back to Makefile.in to avoid changing its content. r=rstrong
https://hg.mozilla.org/mozilla-central/rev/be1ce4548b2e
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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
Attachment #8943772 - Flags: approval-mozilla-beta?
(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)?
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(catlee)
Flags: needinfo?(bhearsum)
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.
Attachment #8943772 - Flags: approval-mozilla-beta? → approval-mozilla-release?
> 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.
(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.
Flags: needinfo?(robert.strong.bugs)
Tracked. We'd like to include this in a 58.x dot release as a ride-along.
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
Attachment #8943772 - Flags: approval-mozilla-release? → approval-mozilla-release+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.