Closed Bug 1108056 Opened 5 years ago Closed 5 years ago

AutoConfig doesn't work on Firefox 34

Categories

(Core :: Preferences: Backend, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox34 --- wontfix
firefox35 + fixed
firefox36 + fixed
firefox37 + fixed
firefox-esr31 --- unaffected

People

(Reporter: mkaply, Assigned: spohl)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

It looks like with the changes for bug 1046906, the AutoConfig code was never updated to look for AutoConfig files in the new Resources directory.

See:

http://mxr.mozilla.org/mozilla-central/source/extensions/pref/autoconfig/src/nsReadConfig.cpp#242
I'll take this.
Assignee: nobody → mozilla
Blocks: 1046906
rstrong:

I'm trying to test my fix, but when I build on Mac, I'm not getting the new Mac layout for the .app file?

Does the new layout not happen until the signing step?

How can I emulate a Firefox app bundle?
Flags: needinfo?(robert.strong.bugs)
If you're building the current mozilla-central, you will get the new layout. A simple build will leave symlinks to some test files behind, which shouldn't matter. If you want to get rid of these symlinks too, you can do the following:
1. cd to mozilla-central source code directory
2. execute './mach build' and wait for build to complete
3. cd to obj-dir
4. execute 'make package'

You should now see a .dmg file in your obj-dir. You can extract the .app bundle from this .dmg and it will no longer have the symlinks to test files etc.
Flags: needinfo?(robert.strong.bugs)
Attached patch Add #ifdefs for new Mac location (obsolete) — Splinter Review
All we can do here is add #ifdefs for Mac.
Attachment #8534333 - Flags: review?(robert.strong.bugs)
Comment on attachment 8534333 [details] [diff] [review]
Add #ifdefs for new Mac location

Review of attachment 8534333 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like you should be able to make this change unconditionally for all platforms. Have you tried that?
See https://bugzilla.mozilla.org/show_bug.cgi?id=841011

Technically the location of the config file is the same as the location of the executable file (except on Mac now).

So NS_GRE_DIR is not correct for other platforms.

In particular, according to http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsDirectoryServiceDefs.h#51

> If no GRE is used, this propery will behave like
> NS_XPCOM_CURRENT_PROCESS_DIR.

On Windows, the current process directory is the browser subdirectory, not the EXE directory.
(In reply to Mike Kaply (:mkaply) from comment #6)
> See https://bugzilla.mozilla.org/show_bug.cgi?id=841011
> 
> Technically the location of the config file is the same as the location of
> the executable file (except on Mac now).

Right, and from what I can tell, NS_GRE_DIR will be set to the location of the executable file except for Mac now. It would be great if you could make this change and send to try to see where we stand.

> So NS_GRE_DIR is not correct for other platforms.

It would be great to have a try run to confirm this statement.

> In particular, according to
> http://mxr.mozilla.org/mozilla-central/source/xpcom/io/
> nsDirectoryServiceDefs.h#51
> 
> > If no GRE is used, this propery will behave like
> > NS_XPCOM_CURRENT_PROCESS_DIR.
> 
> On Windows, the current process directory is the browser subdirectory, not
> the EXE directory.

As the comment states, this is only applicable if no GRE directory is used.
(In reply to Stephen Pohl [:spohl] from comment #7)
> (In reply to Mike Kaply (:mkaply) from comment #6)
> > So NS_GRE_DIR is not correct for other platforms.
> 
> It would be great to have a try run to confirm this statement.

Hmm, arguably the reason why we didn't find this bug before shipping with the new layout is because there is no test for this on try. If you don't have a Windows machine, could you explain step by step how to reproduce this issue? I don't mind testing this myself if you don't have the necessary system. Thanks!
> Hmm, arguably the reason why we didn't find this bug before shipping with the new layout is because there is no test for this on try. 

Unfortunately in talking to the test team, no one could figure out how to make a test for this.

See:

https://bugzilla.mozilla.org/show_bug.cgi?id=576430
https://bugzilla.mozilla.org/show_bug.cgi?id=841011

for other breakages.

To test for this, add new file autoconfig.js in the defaults/pref directory that contains:

pref("general.config.obscure_value", 0);
pref("general.config.filename", "firefox.cfg");

Then in the directory where the EXE is located (or Resource dir on Mac), create a file firefox.cfg that looks like this:

// First line is a comment (mandatory)
lockPref("a.b.c.d", "Locked Pref");

If it works, you won't get an error on startup and when you go to about:config, you'll see a.b.c.d as the first pref and it will be in italics (locked).

I can test this on Windows if we do a try build, but I can't build on Windows.
Attached patch Use NS_GRE_DIR (obsolete) — Splinter Review
Will send this to try once it reopens (currently closed due to bug 1109752).
Comment on attachment 8534333 [details] [diff] [review]
Add #ifdefs for new Mac location

Let's go with spohl's approach if it works... which I think it should
Attachment #8534333 - Flags: review?(robert.strong.bugs)
(In reply to Stephen Pohl [:spohl] from comment #10)
> Created attachment 8534499 [details] [diff] [review]
> Use NS_GRE_DIR
> 
> Will send this to try once it reopens (currently closed due to bug 1109752).

Sent to try:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c60acd2c4406
How do I get the build to download from the try server?
Click on the green 'B', then select 'Go to build directory' in the lower left corner. Here are the direct links for Windows and Mac:
Windows: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/spohl@mozilla.com-c60acd2c4406/try-win32/
Mac: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/spohl@mozilla.com-c60acd2c4406/try-macosx64/
Comment on attachment 8534499 [details] [diff] [review]
Use NS_GRE_DIR

Review of attachment 8534499 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, you're right. We can go back to the old way. It works on Windows.
Attachment #8534499 - Flags: review+
Attached patch PatchSplinter Review
Thanks, :mkaply! Updated patch with proper commit message.
Assignee: mozilla → spohl.mozilla.bugs
Attachment #8534333 - Attachment is obsolete: true
Attachment #8534499 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8535702 - Flags: review?(benjamin)
I'd really like to get this in Firefox 35. Is there anyone else that can review?
Comment on attachment 8535702 [details] [diff] [review]
Patch

This will still break app signing because we're stuffing files into the app bundle, right? So this is not really a long-term solution.
Attachment #8535702 - Flags: review?(benjamin) → review+
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #19)
> This will still break app signing because we're stuffing files into the app
> bundle, right? So this is not really a long-term solution.

That's right. I want to clarify though (for everyone who may come here to read this) that the 'we' here isn't the Mozilla build system (i.e. how we build Firefox by default), but rather someone who deliberately puts an autoconfig.js file in the defaults/pref directory after install. Currently, OSX only verifies our signature at first run after an install. So, as long as the file is added after this first run, things will work fine. In the future, we may start using APIs that prompt the OS to check our signature more frequently, such as the OSX Keychain API for example. Hopefully, we'll have a better, long-term solution in place by then.
[Tracking Requested - why for this release]:

This broke due to the v2 signature changes on OSX. Not sure what release we want to track this though. I'll request approval for uplift on the patch after it had time to bake on m-c. Risk is minimal.
> In the future, we may start using APIs that prompt the OS to check our signature more frequently, such as the OSX Keychain API for example.

What would happen in those cases if the signature was completely removed? Isn't that a way to work around this?

> Hopefully, we'll have a better, long-term solution in place by then.

There's really only one good solution besides Mozilla signing custom enterprise builds - the autoconfig file needs to be specified via MCX on Mac and Group Policy on Windows and all browser modifications need to be moved completely out of the Firefox app bundle or directory.

That's the only way to make this work.
And please track this into Firefox 35. This is a show stopper for people keeping up with rapid release in enterprise.
(In reply to Mike Kaply [:mkaply] from comment #23)
> > In the future, we may start using APIs that prompt the OS to check our signature more frequently, such as the OSX Keychain API for example.
> 
> What would happen in those cases if the signature was completely removed?
> Isn't that a way to work around this?

It may or may not depend on your system configuration, i.e. whether or not you permit apps from unknown developers to run etc. Personally, I doubt that removing signatures will be a good workaround.
https://hg.mozilla.org/mozilla-central/rev/6b0d0598973e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Stephen, could you fill an uplift request to aurora & beta? Thanks
Flags: needinfo?(spohl.mozilla.bugs)
Keywords: regression
Comment on attachment 8535702 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Apple v2 signing (bug 1047584)
[User impact if declined]: AutoConfig does not work on OSX
[Describe test coverage new/current, TBPL]: Has been on m-c for a day and :mkaply confirmed that this patch fixes the issue he reported.
[Risks and why]: Very minimal, limited to AutoConfig alone.
[String/UUID change made/needed]: n/a
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8535702 - Flags: approval-mozilla-beta?
Attachment #8535702 - Flags: approval-mozilla-aurora?
Now that this is working, I'm looking into the signing stuff and I'm not very optimistic. I don't see a way to remove the signing and just convert Firefox to "unsigned".

I found this:

https://developer.apple.com/library/mac/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-TNTAG402

Changes That Don't Invalidate a Code Signature

There are a few changes you can make to a signed bundle that won't invalidate its signature.

If you have optional or replaceable content you wish to change without invalidating the code signature, nested code can be replaced with equivalent (conforms to the designated requirement) nested code without disturbing the outer signature. This is the design mechanism for indirection in code bundles. It is acceptable to code-sign a bundle containing no main executable, and then treat it as nested code (typically in Contents).

Removing files from .lproj directories inside Contents/Resources will not invalidate the code signature, but adding or changing files will.

Removing the Mach-O slice for a particular architecture from a universal binary will also not invalidate the code signature


Can this be used for some files I wonder?
Attachment #8535702 - Flags: approval-mozilla-beta?
Attachment #8535702 - Flags: approval-mozilla-beta+
Attachment #8535702 - Flags: approval-mozilla-aurora?
Attachment #8535702 - Flags: approval-mozilla-aurora+
See Also: → 1185048
You need to log in before you can comment on or make changes to this bug.