Open Bug 1590717 Opened 6 years ago Updated 1 year ago

macOS codesign verify failing on Firefox (70) with sealed resource invalid

Categories

(Release Engineering :: Release Automation, defect)

x86_64
macOS
defect

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: xdavid, Unassigned)

References

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

Steps to reproduce:

  1. Install Firefox 70.0 in macOS
  2. Issue the below command in terminal:
    codesign --verify --verbose /Applications/Firefox.app

Actual results:

Above codesign command failing with the below error:
/Applications/Firefox.app: a sealed resource is missing or invalid
file modified: /Applications/Firefox.app/Contents/Resources/defaults/pref/channel-prefs.js

Expected results:

The output should be:
/Applications/Firefox.app: valid on disk
/Applications/Firefox.app: satisfies its Designated Requirement

Component: Untriaged → Installer
OS: Unspecified → macOS
Hardware: Unspecified → x86_64

I am seeing the expected output with a fresh download of 70.0. Can you post the contents of that channel-prefs.js file?

Component: Installer → Release Automation: Signing
Flags: needinfo?(xdavid)
Product: Firefox → Release Engineering
QA Contact: aki
Version: 70 Branch → unspecified

Hi,

This is the content of the channel-prefs.js:

/* 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", "release");
Flags: needinfo?(xdavid)

I am seeing the same on my system. If I had to make a guess, I'd suspect that this broke during an update. We should make sure that updates don't break code signatures. If we could get a test written for this, even better. codesign -vvvv Firefox.app should always pass, including after updates.

Molly, could you direct this to someone to take a look?

Flags: needinfo?(mhowell)

Okay, I see what happened; the file is different because we added this comment, and channel-prefs.js is specifically not touched during updates so that QA can use it to change the update channel and also to make release candidates possible. Meaning, we're updating the signatures because those are in a separate file, but we're not updating channel-prefs.js, so its signature stops matching. As the comment says, there are more details in bug 1431342, where something like this has happened before.

I have no idea how to fix this. We could make the problem go away for this specific report by removing the comment, but that seems like a bad idea, plus it would actually make the same thing start happening for any installs created since bug 1576546. I doubt it's possible to alter just that one signature dynamically during the update. And we can't start actually updating the file without breaking QA's workflows and completely changing how release candidates work. I'm not even sure how to test for this other than adding some kind of lint check that bans all changes to the channel-prefs.js template. It's... not a good situation.

Flags: needinfo?(mhowell)

So if I'm understanding correctly, this is a one-time issue due to this added comment? Any new installs going forward shouldn't have this problem since they will get the file with the new comment? If that's the case, we should get a test written that will verify our code signatures in our update tests to prevent this from happening in the future. We really need to stop breaking our own signatures because it is very much possible that macOS will check code signatures on every start at some point in the future. If we can't make any changes to channel-prefs.js (and any similar files) in the future because it would break signatures, we should have some way of enforcing this.

Flags: needinfo?(mhowell)

(In reply to Stephen A Pohl [:spohl] from comment #5)

Any new installs going forward shouldn't have this problem since they will get the file with the new comment?

Correct, any install created since that change landed will not be seeing this problem.

If we can't make any changes to channel-prefs.js (and any similar files) in the future because it would break signatures, we should have some way of enforcing this.

Then maybe some kind of lint or other hook that reviewbot runs that prevents any changes to that file at all really is the way to go. I... don't know how to do that or who might know how to do that, I'd have to poke around some.

Flags: needinfo?(mhowell)
QA Contact: mozilla → jlorenzo
See Also: → 1799332
Severity: normal → S3
QA Contact: jlorenzo
Component: Release Automation: Signing → Release Automation
You need to log in before you can comment on or make changes to this bug.