1623878 - Port bug 1617047: Add codesign mapping files for Thunderbird

Status: RESOLVED INCOMPLETE

(Thunderbird :: Security, task, P1)

Description

[Rob Lemley [:rjl]]

Not sure how urgent this is for Thunderbird, since we are single process yet, but I suspect that we will need to rearrange some files and produce a map file when the codesigning scriptworkers get updated.

Comment 1

[Haik Aftandilian [:haik]]

(In reply to Rob Lemley [:rjl] from comment #0)

Not sure how urgent this is for Thunderbird, since we are single process yet, but I suspect that we will need to rearrange some files and produce a map file when the codesigning scriptworkers get updated.

We should be able to support using either a map file or the older codesigning method at first and then switch Thunderbird over to using a map file after Firefox. The automation changes we're working on now work this way so there should not be any breakage for Thunderbird when we enable them.

Comment 2

[Rob Lemley [:rjl]]

I think this will be a good one for you Dave (especially since you have a Mac) ;) But before doing too much, check with Haik. This has been sitting for a while now, and I don't know what the current plan is.

Our codesign files are under comm/build/macosx/hardenedruntime. You'll need to create a production and developer json map similar to Firefox's in that same directory. On macOS there's the Spotlight integration plugin that you'll need to make sure gets added to the json file appropriately.

Then the other part I just noticed is that in packager.mk, that bit of code to create the ZIP file with all of the entitlement files and the JSON map files is not going to work for Thunderbird. It's probably best to make the location of the codesigning entitlement files a variable. Since packager.mk is included from comm/mail/installer/Makefile.in the variable can be set there (and also the equivalent file under browser/ for Firefox.) See https://searchfox.org/comm-central/rev/612de82269d1ce23b9f393f40eafcec00078e31a/mail/installer/Makefile.in#114-119,133

Comment 3

[Dave Miller [:justdave]]

Sounds good. :haik, what can you tell me?

Comment 4

[Haik Aftandilian [:haik]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #3)

Sounds good. :haik, what can you tell me?

Here's some background on the code signing improvements we've been working on in Firefox (which are a bit stalled at the moment.) At present, we codesign Firefox.app using the entitlement list production.entitlements.xml. The way we codesign the different files within the bundle is hardcoded in automation code[1]. The automation code executes the codesign command and uses the entitlement list for any codesigning commands that use entitlements.

For Firefox, being a multi-process application using different executables from within the Firefox.app bundle, we can get some hardening benefits by using different entitlements for different processes. Content processes use an executable from plugin-container.app within Firefox.app while the parent process uses the firefox executable. We could turn on some stricter entitlements for the parent process if our codesigning supported different entitlement files for different executables with the Firefox.app bundle. For processes that never run Javascript, we could turn on stronger runtime page signing restrictions.

However, rather than hardcode this knowledge in the scriptworker-scripts automation code, the plan is to have a config file in mozilla-central that specifies how to codesign the Firefox.app bundle including specifying what entitlements to use for each executable. I wrote a python utility[2] to consume the codesigning specification as a json file and run the necessary codesign commands. I have some patches to integrate this into the scriptworker-scripts signing code[1] that runs in automation, but it's not ready to integrate yet.

For Thunderbird, this might be worthwhile to adopt if you see a benefit from having a config file that specifies how to sign all the files within the Thunderbird.app bundle. Separating the codesigning out of the automation code means that the codesigning entitlements can be changed from the project repo (e.g., mozilla-central) without having to push any changes to the automation code. For Firefox this makes sense because we are multi-process and use different executables and we can leverage this to use stronger entitlements for different executables. Hope that helps and feel free to reach out if you have any questions.

1. https://github.com/mozilla-releng/scriptworker-scripts/blob/47a9c02832bbddb6cd4f96d5019a740b5c107fc5/iscript/src/iscript/mac.py#L195
2. https://github.com/hafta/codesign-tree

Comment 5

[Dave Miller [:justdave]]

OK, so just to make sure I understand this... after reading the above stuff and poking around a bit at the code, it looks like this has not actually landed yet in Firefox, but when it does it will probably break TB's signing process, so we need to have the equivalent ready to go when Firefox lands theirs?

From my first pass, it looks like all we need to do is add TB equivalents for the two json files, and change the two lines in comm/taskcluster/ci/config.yml to point at them.

Question for :haik :
The config.yml pointers are becoming just filenames and not full paths. None of the other changes I'm seeing seem to indicate passing a directory. Where it's getting the directory from to look for those files in? (How will it know to look in a different place for TB's files?)

Comment 6

[Dave Miller [:justdave]]

(In reply to Rob Lemley [:rjl] from comment #2)

Then the other part I just noticed is that in packager.mk, that bit of code to create the ZIP file with all of the entitlement files and the JSON map files is not going to work for Thunderbird. It's probably best to make the location of the codesigning entitlement files a variable. Since packager.mk is included from comm/mail/installer/Makefile.in the variable can be set there (and also the equivalent file under browser/ for Firefox.) See https://searchfox.org/comm-central/rev/612de82269d1ce23b9f393f40eafcec00078e31a/mail/installer/Makefile.in#114-119,133

Is this broken currently? I'm not seeing how the change being made in that patch would work any different than what it's replacing, other than depositing the output one deeper level in the tree. It's still specifically doing Firefox's version in the current code, which means if that's broken for TB after the patch, it should already be broken now. (unless I'm reading that wrong)

Comment 7

[Rob Lemley [:rjl]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #6)

Is this broken currently? I'm not seeing how the change being made in that patch would work any different than what it's replacing, other than depositing the output one deeper level in the tree. It's still specifically doing Firefox's version in the current code, which means if that's broken for TB after the patch, it should already be broken now. (unless I'm reading that wrong)

Yes, currently the generated zip file is wrong for Thunderbird. However, it's not being used yet. The iscript scriptworker is downloading the entitlements files right from Mercurial. There's some magic in one of the transforms to turn that path into a proper URL.

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #5)

Question for :haik :
The config.yml pointers are becoming just filenames and not full paths. None of the other changes I'm seeing seem to indicate passing a directory. Where it's getting the directory from to look for those files in? (How will it know to look in a different place for TB's files?)

The path is hardcoded in packager.mk: https://searchfox.org/mozilla-central/rev/3d53187b90605ccef321c9cadbba762ad06a6381/toolkit/mozapps/installer/packager.mk#94-97

packager.mk is included from comm/mail/installer/Makefile.in and browser/installer/Makefile.in. (Possibly other places) A variable can be set in those Makefiles and used in packager.mk. You could also set it only in Thunderbird's Makefile.in and then in packager.mk use a conditional assignment.

CODESIGN_PATH ?= security/mac

Comment 8

[Haik Aftandilian [:haik]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #5)

OK, so just to make sure I understand this... after reading the above stuff and poking around a bit at the code, it looks like this has not actually landed yet in Firefox, but when it does it will probably break TB's signing process, so we need to have the equivalent ready to go when Firefox lands theirs?

None of this is used for Firefox yet. The scriptworker changes I have been working on (that are still in progress) are meant to be backwards compatible so no changes to Thunderbird should be required. It would probably be easiest to wait until we have this working for Firefox and then add a map file for Thunderbird to use. The scriptworker changes[1] fall back to the old mode if a map file is not present and I don't want to break the old mode in case we need to fall back to it. And I don't want to break Thunderbird's codesigning.

From my first pass, it looks like all we need to do is add TB equivalents for the two json files,
and change the two lines in comm/taskcluster/ci/config.yml to point at them.

Question for :haik :
The config.yml pointers are becoming just filenames and not full paths. None of the other changes I'm seeing seem to
indicate passing a directory. Where it's getting the directory from to look for those files in? (How will it know to look in a
different place for TB's files?)

With the fix for bug 1606746, we create a zip file build artifact that includes the entitlements. The releng team is going to help wire all this up, but my understanding is the build config will include a map file path if one is present in the tree and a path to the zip file artifact. (I plan to make a change to this in the future so that the files are in the root of the zip file and not in a "hardenedruntime" directory as shown on the bug.)

1. https://github.com/hafta/scriptworker-scripts/blob/master/iscript/src/iscript/mac.py#L1159

Comment 9

[Dave Miller [:justdave]]

OK, so basically this can just sit here until bug 1593072 is fixed, and nothing will break when it lands so we don't have to rush to have it ready.

Comment 10

[Rob Lemley [:rjl]]

Upstream took a different direction, this bug is no longer necessary.