1047728 - Deal with move of *.app/Contents/MacOS/defaults/pref/channel-prefs.js to *.app/Contents/Resources/defaults/pref/channel-prefs.js

Status: RESOLVED WORKSFORME

(Toolkit :: Application Update, defect)

Description

[Stephen A Pohl [:spohl]]

As part of bug 1047584 we will be moving defaults/pref/channel-prefs.js from Contents/MacOS to Contents/Resources. We will need to make sure that things still work properly with the file in this new location.

Note that this file cannot be modified after the .app has been signed, since this would break the signature on the .app.

Comment 1

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

dveditz, can I get your take from a security point of view or point me to someone on the security team to get input. What I am hoping to do is move this file into the omni.ja where all other default preferences live and to remove the restriction to only read the update channel value from the default app.update.channel preference branch so it can be set as a user preference. This will allow QA to test updating to release from beta.

Please note: this is not currently public knowledge and Apple is holding us to our NDA with them regarding this issue. As such, all the bugs are currently Mozilla confidential.

Comment 2

[[e-mail] (:bhearsum)]

This is going to cause issues for QA. The way they do prerelease update tests is to modify the update channel by touching this file, I believe. Maybe we can make it possible to do this in the profile instead?

Comment 3

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

Right and hence comment #1.

Comment 4

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

Nick, I don't see a clean way to do this and still support updating beta to release while retaining the beta channel. I guess we could only make this change to OS X but I hate the idea of using one method for OS X and a different method for other platforms. Thoughts?

Comment 5

[Stephen A Pohl [:spohl]]

Robert, it turns out that we're seemingly able to modify files in the root of the .app bundle, so we could have Firefox.app/defaults/pref/channel-prefs.js and still modify it after signing the bundle. Would this solve our problem here and the one regarding distribution partners in bug 1047738?

Ben, do you see any potential problems with a bundle structure that has additional files in the root of the bundle with regards to our signing server etc?

Comment 6

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

Seemingly it would ;)

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #4)

Putting channel-prefs.js in omni.ja would definitely be a problem for release builds on beta, but sounds like we can put it in the top-level of the .app to avoid that. I'd wouldn't lose any sleep if we lost the defaults/pref/ prefix on the way.

update-settings.ini would be another file that could be problematic. For cases like 31.0 --> 32.0b1 QA needs to modify ACCEPTED_MAR_CHANNEL_IDS inside that. Can it move to the top-level and the updater still use it ?  

For both files we would need to be careful with the first update to new layout. I think it would be preferable for a REMOVE/ADD instruction pair to run before an ADD-IF-NOT (the fallback). That means moving the check_for_add_if_not_update check lower in tools/update-packaging/make_incremental_update.sh and make_incremental_updates.py.

QA may also be inserting preferences (like app.update.log) via a .js file, which may or may not be done inside the app directory. Adding whimboo, who owns the mozmill system QA uses.

Comment 8

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

Stephen, what is your confidence level that this will work? If it isn't a high level of confidence what can be done to verify that it will work? Perhaps a contact at Apple can confirm?

Clearing request to dveditz in the hope this will work out.

Comment 9

[Stephen A Pohl [:spohl]]

Right now, my confidence is pretty high that this will work for the following reasons:
1. We already have a file ('precomplete') sitting next to Contents/ in the top-level directory. We have not had any issues signing with v2 signatures due to this file, so other files and directories shouldn't be a problem either.
2. All my local testing seemed to confirm that the v2 signature seal only applies to Contents/ and its descendants. Files can be added/modified/removed in the root of the .app without affecting the signature seal, as long as they don't touch Contents/.

As with all the other v2 signature bugs, we won't be able to confirm 100% until we have a build of OSX that has the v2 signature enforcement enabled in Gatekeeper, or we have a manual way of turning it on. The latest 10.9.5 preview build still does not have this enabled. We will also need a Firefox.app bundle that tries to access the resources in the root of the .app bundle, of course.

What would be best to know from Apple right now is when we could expect a preview build that has the new v2 enforcements enabled in Gatekeeper. Alternatively, it would be equally helpful to know if there is a way to manually turn this enforcement on in the current 10.9.5 preview build.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Nick Thomas [:nthomas] from comment #7)
> QA may also be inserting preferences (like app.update.log) via a .js file,
> which may or may not be done inside the app directory.

All changes to preferences happen in the prefs.js file which is located inside the profile. We do not mess around with any other files as the ones mentioned above inside the bundle.

Once we know how we want to solve this bug, I can certainly work on the solution for our Mozmill automation scripts.

Comment 11

[Stephen A Pohl [:spohl]]

We're most likely going to move this file to *.app/MozResources/defaults/pref/channel-prefs.js since this doesn't break the signature seal on the .app bundle.

Comment 12

[[e-mail] (:bhearsum)]

(In reply to Stephen Pohl [:spohl] from comment #5)
> Robert, it turns out that we're seemingly able to modify files in the root
> of the .app bundle, so we could have
> Firefox.app/defaults/pref/channel-prefs.js and still modify it after signing
> the bundle. Would this solve our problem here and the one regarding
> distribution partners in bug 1047738?
> 
> Ben, do you see any potential problems with a bundle structure that has
> additional files in the root of the bundle with regards to our signing
> server etc?

I might need to tweak my existing patch, but the signing server should cope fine. Given Henrik's comment, this may not be an issue for QA, but I still think we should do it. I suspect there's a number of systems and/or people that we're not aware about that rely on this ability.

Comment 13

[Stephen A Pohl [:spohl]]

MozResources is no longer an option. We will have to move channel-prefs.js to Contents/Resources.

Juan, we will need to modify our installer/updater test instructions. I'll give a high-level outline of what needs to change, but please let me know if you have any questions.

1. Before we run the .app, we need to ensure that the signature is valid. The simplest way to do this is to run spctl -a -v <bundlename>.app.
2. When channel-prefs.js needs to be modified, the app should be run a first time after being downloaded. Only then should channel-prefs.js be modified. Note that with a modified channel-prefs.js, we can no longer test for valid signatures.
3. After update tests that don't modify channel-prefs.js, we will need to make sure that the signature on the .app bundle is still correct. Again, the easiest way to do this is with the command given in step 1.
4. If channel-prefs.js needs to be modified for update tests, it will need to be changed back before checking the signature.

Would you be able to update these tests? Please let me know if anything's unclear.

Comment 14

[juan becerra [:juanb]]

(In reply to Stephen Pohl [:spohl] from comment #13)
> MozResources is no longer an option. We will have to move channel-prefs.js
> to Contents/Resources.
> 
> Juan, we will need to modify our installer/updater test instructions. I'll
> give a high-level outline of what needs to change, but please let me know if
> you have any questions.
> 
> 1. Before we run the .app, we need to ensure that the signature is valid.
> The simplest way to do this is to run spctl -a -v <bundlename>.app.
> 2. When channel-prefs.js needs to be modified, the app should be run a first
> time after being downloaded. Only then should channel-prefs.js be modified.
> Note that with a modified channel-prefs.js, we can no longer test for valid
> signatures.
> 3. After update tests that don't modify channel-prefs.js, we will need to
> make sure that the signature on the .app bundle is still correct. Again, the
> easiest way to do this is with the command given in step 1.
> 4. If channel-prefs.js needs to be modified for update tests, it will need
> to be changed back before checking the signature.
> 
> Would you be able to update these tests? Please let me know if anything's
> unclear.

Checking these manually seems straightforward, but I will defer to Henrik to see if this is doable in the Mozmill updates automation, which is what we use to verify updates on our side.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

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

Ok, so here some feedback in regards of our Mozmill update tests...

> MozResources is no longer an option. We will have to move channel-prefs.js
> to Contents/Resources.

Will we get a constant in Firefox, which we could use to open that file for modifications inside of Firefox like for getting the profile folder? 

> 1. Before we run the .app, we need to ensure that the signature is valid.
> The simplest way to do this is to run spctl -a -v <bundlename>.app.

That is actually something we could add to our update tests. Can I assume that this command will also work for all the existent installers?

> 2. When channel-prefs.js needs to be modified, the app should be run a first
> time after being downloaded. Only then should channel-prefs.js be modified.
> Note that with a modified channel-prefs.js, we can no longer test for valid
> signatures.

Can you please explain why this has to be run once first? It would be good to know, which initialization methods are getting run. In case of Mozmill we modify this file before we start Firefox the first time. So this would have to be changed. We could change it from within Firefox via nsIFile, as long as there is no pref available to overwrite it.

> 3. After update tests that don't modify channel-prefs.js, we will need to
> make sure that the signature on the .app bundle is still correct. Again, the
> easiest way to do this is with the command given in step 1.
> 4. If channel-prefs.js needs to be modified for update tests, it will need
> to be changed back before checking the signature.

So we would simply have to check for 'accepted' here? Or are special comparisons to the signature check from step 1 necessary?

Comment 16

[Stephen A Pohl [:spohl]]

(In reply to Henrik Skupin (:whimboo) from comment #15)
> (In reply to Stephen Pohl [:spohl] from comment #13)
> > MozResources is no longer an option. We will have to move channel-prefs.js
> > to Contents/Resources.
> 
> Will we get a constant in Firefox, which we could use to open that file for
> modifications inside of Firefox like for getting the profile folder?

I'm not sure I understand this. What are you using today to get the channel-prefs.js?

> > 1. Before we run the .app, we need to ensure that the signature is valid.
> > The simplest way to do this is to run spctl -a -v <bundlename>.app.
> 
> That is actually something we could add to our update tests. Can I assume
> that this command will also work for all the existent installers?

I'm assuming that 'installers' here means our .app bundles. If so, then the behavior will vary. Any existing .app bundles will successfully verify via spctl if they're verified on 10.9.4 and below. On 10.9.5 and above, only .app bundles with the new v2 signature will verify successfully. This is expected to land in FF34.

> > 2. When channel-prefs.js needs to be modified, the app should be run a first
> > time after being downloaded. Only then should channel-prefs.js be modified.
> > Note that with a modified channel-prefs.js, we can no longer test for valid
> > signatures.
> 
> Can you please explain why this has to be run once first? It would be good
> to know, which initialization methods are getting run. In case of Mozmill we
> modify this file before we start Firefox the first time. So this would have
> to be changed. We could change it from within Firefox via nsIFile, as long
> as there is no pref available to overwrite it.

On 10.9.5 and above, Firefox needs to be run first to clear the quarantine bit on the .app bundle after it was downloaded. If the Gatekeeper is turned on (default on OSX) and you modify any file within the bundle before the first launch, the signature on the .app bundle will no longer verify and Gatekeeper refuses to start the the app. However, if you launch the app before any modification, Gatekeeper will be able to validate the signature and clear the quarantine bit. You can then modify files and Gatekeeper will no longer reject the app.

There are some other methods to clear the quarantine bit, but I'm not sure any of these are easier to implement. The .app bundle could be downloaded with a protocol that doesn't set the quarantine bit (I believe smb and afp would work). Or the quarantine bit could be cleared via xattr on the Terminal. Simply launching Firefox a first time before modifying files seems to be the easiest solution, but your experience with these tests may tell us otherwise.

> > 3. After update tests that don't modify channel-prefs.js, we will need to
> > make sure that the signature on the .app bundle is still correct. Again, the
> > easiest way to do this is with the command given in step 1.
> > 4. If channel-prefs.js needs to be modified for update tests, it will need
> > to be changed back before checking the signature.
> 
> So we would simply have to check for 'accepted' here? Or are special
> comparisons to the signature check from step 1 necessary?

It should be the same as in step 1. I just verified this and it looks like spctl will quit with an exit value of 0 when a signature validated successfully, non-zero otherwise.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to Stephen Pohl [:spohl] from comment #16)
> Simply launching Firefox a first time before modifying files seems
> to be the easiest solution, but your experience with these tests may tell us
> otherwise.

Whimboo, without wanting to derail this bug, I'd suggest we disable updates before doing this first launch, to avoid an update starting before mozmill is ready. Then turn them back on for the actual testing.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Nick Thomas [:nthomas] from comment #17)
> Whimboo, without wanting to derail this bug, I'd suggest we disable updates
> before doing this first launch, to avoid an update starting before mozmill
> is ready. Then turn them back on for the actual testing.

The work we would have to do here is not that much. It should be done within a day. I'm just querying for feedback to ensure we implement our bits correctly. So what is the ETA of getting this landed? Also do you have test builds available, which we could use? If the latter is not the case, I agree that we should stop running update tests for Nightly, where this most likely land first, right?

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Stephen Pohl [:spohl] from comment #16)
> > > MozResources is no longer an option. We will have to move channel-prefs.js
> > > to Contents/Resources.
> > 
> > Will we get a constant in Firefox, which we could use to open that file for
> > modifications inside of Firefox like for getting the profile folder?
> 
> I'm not sure I understand this. What are you using today to get the
> channel-prefs.js?

At the moment we are doing all that via Python before we start Firefox. When we have to do it inside of Firefox I wonder if we could get a constant like ProfD, but this time for the Resources e.g. ResD.

https://developer.mozilla.org/en-US/docs/Using_nsIDirectoryService#Known_Locations

If it is not worth the time then no worries. We could construct the path to the file by ourselves.

> > That is actually something we could add to our update tests. Can I assume
> > that this command will also work for all the existent installers?
> 
> I'm assuming that 'installers' here means our .app bundles. If so, then the
> behavior will vary. Any existing .app bundles will successfully verify via
> spctl if they're verified on 10.9.4 and below. On 10.9.5 and above, only
> .app bundles with the new v2 signature will verify successfully. This is
> expected to land in FF34.

Sorry, yes I meant app bundles. Thanks for the feedback. I filed https://github.com/mozilla/mozmill-automation/issues/163 so we can get this implemented.

> > Can you please explain why this has to be run once first? It would be good
> > to know, which initialization methods are getting run. In case of Mozmill we
> > modify this file before we start Firefox the first time. So this would have
> > to be changed. We could change it from within Firefox via nsIFile, as long
> > as there is no pref available to overwrite it.
> 
> On 10.9.5 and above, Firefox needs to be run first to clear the quarantine
> bit on the .app bundle after it was downloaded. If the Gatekeeper is turned
> on (default on OSX) and you modify any file within the bundle before the
> first launch, the signature on the .app bundle will no longer verify and
> Gatekeeper refuses to start the the app. However, if you launch the app
> before any modification, Gatekeeper will be able to validate the signature
> and clear the quarantine bit. You can then modify files and Gatekeeper will
> no longer reject the app.
> 
> There are some other methods to clear the quarantine bit, but I'm not sure
> any of these are easier to implement. The .app bundle could be downloaded
> with a protocol that doesn't set the quarantine bit (I believe smb and afp
> would work). Or the quarantine bit could be cleared via xattr on the
> Terminal. Simply launching Firefox a first time before modifying files seems
> to be the easiest solution, but your experience with these tests may tell us
> otherwise.

Thanks a lot. Now it is clear.

> It should be the same as in step 1. I just verified this and it looks like
> spctl will quit with an exit value of 0 when a signature validated
> successfully, non-zero otherwise.

Exit code is even more helpful. Thanks.

I don't see a problem by the above to get implemented on our side. As mentioned in my last comment please let us know about the ETA, and if we can get test builds and update snippets ahead of the landing.

Comment 20

[Stephen A Pohl [:spohl]]

(In reply to Henrik Skupin (:whimboo) from comment #18)
> So what is the ETA of getting this landed?

As soon as possible. :-) We still need to update a few test harnesses and make some updater changes before we can request reviews, but this is definitely top priority.

> Also do you have test builds available, which we could use?

I should be able to spin one up for you, so just let me know when you're ready so I can get all the latest changes in there.

Comment 21

[Stephen A Pohl [:spohl]]

(In reply to Henrik Skupin (:whimboo) from comment #19)
> (In reply to Stephen Pohl [:spohl] from comment #16)
> > What are you using today to get the channel-prefs.js?
> 
> At the moment we are doing all that via Python before we start Firefox. When
> we have to do it inside of Firefox I wonder if we could get a constant like
> ProfD, but this time for the Resources e.g. ResD.
> 
> https://developer.mozilla.org/en-US/docs/
> Using_nsIDirectoryService#Known_Locations

I think you should still be able to do this via Python. Couldn't you launch Firefox to clear the quarantine bit, shut it down, modify the channel-prefs.js file, then launch it again and run all the tests?

If you do have to modify it in Firefox, the GreD should be all you need (it changed to point to Contents/Resources as part of all this v2 signature work).


> I don't see a problem by the above to get implemented on our side. As
> mentioned in my last comment please let us know about the ETA, and if we can
> get test builds and update snippets ahead of the landing.

Sorry, update snippets..? This may be something that rstrong could provide, as I'm not even sure what this means. :-)

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Stephen Pohl [:spohl] from comment #21)
> I think you should still be able to do this via Python. Couldn't you launch
> Firefox to clear the quarantine bit, shut it down, modify the
> channel-prefs.js file, then launch it again and run all the tests?

Nope. Once Firefox is launched the tests are running. We had a feature called Python callbacks, which we were able to call from Javascript, but it was too flaky and we removed it.

> If you do have to modify it in Firefox, the GreD should be all you need (it
> changed to point to Contents/Resources as part of all this v2 signature
> work).

Oh, perfect! That's what I will use then. For Linux and Windows I might have to use "GreD" and concatenate defaults/channel-prefs.js. 

> > I don't see a problem by the above to get implemented on our side. As
> > mentioned in my last comment please let us know about the ETA, and if we can
> > get test builds and update snippets ahead of the landing.
> 
> Sorry, update snippets..? This may be something that rstrong could provide,
> as I'm not even sure what this means. :-)

It would be more part of nthomas or bhearsum. We would have to test that the update tests can do the following successfully:

* update from an older release to the new signed one
* update a new signed build to a newer signed build

(In reply to Stephen Pohl [:spohl] from comment #20)
> As soon as possible. :-) We still need to update a few test harnesses and
> make some updater changes before we can request reviews, but this is
> definitely top priority.

Alright. So I will also have to make it my priority.

> > Also do you have test builds available, which we could use?
> 
> I should be able to spin one up for you, so just let me know when you're
> ready so I can get all the latest changes in there.

Whenever you have something please build it. It would be good to have an already signed build while updating the tests.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Anything related to the update tests modifications is covered on bug 1060599 now.

Comment 24

[Stephen A Pohl [:spohl]]

(In reply to Henrik Skupin (:whimboo) from comment #22)
> (In reply to Stephen Pohl [:spohl] from comment #21)
> > > Also do you have test builds available, which we could use?
> > 
> > I should be able to spin one up for you, so just let me know when you're
> > ready so I can get all the latest changes in there.
> 
> Whenever you have something please build it. It would be good to have an
> already signed build while updating the tests.

Here you go: http://people.mozilla.org/~spohl/FirefoxNightly.app.zip

A couple things you may try:
codesign -vvvv FirefoxNightly.app -> This should tell you that the bundle signature is valid.
codesign -dv FirefoxNightly.app -> This should tell you that the signature version is 2 (second to last line)
spctl -a -v FirefoxNightly.app -> This should tell you that the app is accepted based on the Developer ID.

I may state the obvious, but remember that Gatekeeper will only verify the signature the first time the app is launched. You may want to download it and make a copy before you're testing things. You can then delete the already launched app and replace it with the copy, which will still have the quarantine bit set.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Thanks Stephen. For anything else lets move the discussion over to the newly filed bug.(In reply to

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Stephen, do you have some WIP code, which you could already share with us?

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Oh, and something else regarding the location of the default prefs. Given that we move channel-prefs.js into a new location, the code behind "PrfDef" (http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsAppDirectoryServiceDefs.h#33) might have to be updated?

Comment 28

[Stephen A Pohl [:spohl]]

(In reply to Henrik Skupin (:whimboo) from comment #26)
> Stephen, do you have some WIP code, which you could already share with us?

So, this should already be in the build from comment 24: http://people.mozilla.org/~spohl/FirefoxNightly.app.zip

Or are you referring to something else?

(In reply to Henrik Skupin (:whimboo) from comment #27)
> Oh, and something else regarding the location of the default prefs. Given
> that we move channel-prefs.js into a new location, the code behind "PrfDef"
> (http://mxr.mozilla.org/mozilla-central/source/xpcom/io/
> nsAppDirectoryServiceDefs.h#33) might have to be updated?

This should already work, since it's based on the GreD (which was changed to point to Contents/Resources). Please let me know if it doesn't.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

This actually is an addon-compat bug! I would expect that a couple of add-ons make use of GreD to get specific files. Given that with this patch the location will change to Contents/Resources, breakage can be expected. I have already seen this with Mozmill, which I have to fix today.

Comment 30

[Jorge Villalobos [:jorgev] (he/him)]

I see a few add-ons that use GreD. What kind of breakage should be expected? Is it that some files under GreD will no longer be there?

Comment 31

[Stephen A Pohl [:spohl]]

Any executable code will no longer be there (executables, dylibs, .app bundles). Any other files like application.ini etc. will still be retrievable with a simple GreD lookup.

Comment 32

[Stephen A Pohl [:spohl]]

Flagged bug 1050944 for addon-compat instead of this one, as that's where we're changing the GRE and XRE directory to point to Contents/Resources.

Comment 33

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

Nothing needs to be done here so resolving wfm