1891013 - improve update verify to stop ignoring ChannelPrefs and UpdateSettings frameworks

Status: NEW

(Release Engineering :: Release Automation, enhancement)

Description

[[e-mail] (:bhearsum)]

When we switched out channel-prefs.js and update-settings.ini on macOS in favour of new binary frameworks it became very difficult to evaluate their state as part of update verify. In the end, we ended up ignoring them when diffing, but a recently discovered bug has emphasized the need to do better here.

I do not think it is possible to simply diff these frameworks and have that work due to their code signing, but if we can find a way to reliably include the channel (for ChannelPrefs.framework) and mar_channel_ids/accepted_mar_channel_ids (for UpdateSettings.framework) as plain text, we could diff those files. Ideally, I think we'd put this information somewhere in Info.plist, but failing that we could probably include a simple plain text file in Resources with the necessary information. A downside to this would be potential confusion ("why doesn't my channel change when I change this plaintext value labeled "channel"?) - but I think it's worth the benefit.

I'm open to other options as well - as long as we have a file with the key information that isn't changed by signing this should be easy to do.

Robin, Stephen - I'd appreciate your input and insight here.

Comment 1

[Stephen A Pohl [:spohl]]

We can add any number of custom keys in the Info.plist as long as they don't conflict with Apple's keys. I believe this would be the ideal place to store this information and we have existing examples where we populate Info.plist files at build time (see Info.plist.in).

Comment 2

[Robin Steuber (she/her) [:bytesized]]

I'm sorry, I'm really struggling to understand here.

(In reply to [e-mail] (:bhearsum) from comment #0)

When we switched out channel-prefs.js and update-settings.ini on macOS in favour of new binary frameworks it became very difficult to evaluate their state as part of update verify. In the end, we ended up ignoring them when diffing, but a recently discovered bug has emphasized the need to do better here.

I don't know think I know what "update verify" is.

I do not think it is possible to simply diff these frameworks and have that work due to their code signing, but if we can find a way to reliably include the channel (for ChannelPrefs.framework) and mar_channel_ids/accepted_mar_channel_ids (for UpdateSettings.framework) as plain text, we could diff those files. Ideally, I think we'd put this information somewhere in Info.plist, but failing that we could probably include a simple plain text file in Resources with the necessary information.

I'm now very confused. From the context of the Bug 1799332 link, I assume that we are trying to catch problems with the frameworks having unexpected content after an update. But we only had a problem updating the frameworks, not Info.plist. I don't really understand why it would be helpful to duplicate this information elsewhere and then test for it there.

But ignoring all of that for a moment, if we put the channel in Info.plist, what happens for updates to RC builds? Do they have the channel set to "Release"? That seems incorrect. Or do they have their channel set to "Beta"? That sounds like it would break the signature.

A downside to this would be potential confusion ("why doesn't my channel change when I change this plaintext value labeled "channel"?) - but I think it's worth the benefit.

I definitely don't understand how this happens.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Robin Steuber (they/them) [:bytesized] from comment #2)

I'm sorry, I'm really struggling to understand here.

(In reply to [e-mail] (:bhearsum) from comment #0)

When we switched out channel-prefs.js and update-settings.ini on macOS in favour of new binary frameworks it became very difficult to evaluate their state as part of update verify. In the end, we ended up ignoring them when diffing, but a recently discovered bug has emphasized the need to do better here.

I don't know think I know what "update verify" is.

tl;dr - update verify tests to see if a MAR gets a user to the same place as an installer. On macOS, this currently means that we:

• Download and unpack a target dmg (eg: en-US 125.0)
• Download and unpack a source dmg (eg: en-US 124.0)
• Download a MAR (eg: en-US 124.0 -> 125.0)
• Apply the MAR to the unpacked source dmg
• Diff the result with the unpacked target dmg

Obviously there are some cases (particularly with RCs) where we expect differences, but these tests do a good job of exercising the manifest in the MARs to ensure the correct instructions are present. (To handle expected differences, like channels, we apply some transformations depending on the specific test being done.)

I do not think it is possible to simply diff these frameworks and have that work due to their code signing, but if we can find a way to reliably include the channel (for ChannelPrefs.framework) and mar_channel_ids/accepted_mar_channel_ids (for UpdateSettings.framework) as plain text, we could diff those files. Ideally, I think we'd put this information somewhere in Info.plist, but failing that we could probably include a simple plain text file in Resources with the necessary information.

I'm now very confused. From the context of the Bug 1799332 link, I assume that we are trying to catch problems with the frameworks having unexpected content after an update. But we only had a problem updating the frameworks, not Info.plist. I don't really understand why it would be helpful to duplicate this information elsewhere and then test for it there.

What would be helpful is to have a reliable way to know which settings a framework has. Putting this information in Info.plist is one way, and as long as we never update it independently of the binary, we should be able to rely on it, I think?

But ignoring all of that for a moment, if we put the channel in Info.plist, what happens for updates to RC builds? Do they have the channel set to "Release"? That seems incorrect. Or do they have their channel set to "Beta"? That sounds like it would break the signature.

No - we wouldn't change anything about what content people get. Adding this extra metadata would allow us to easily see any changes to the channel or update settings as a result of a MAR. This would've prevented https://bugzilla.mozilla.org/show_bug.cgi?id=1890528.

Comment 4

[Robin Steuber (she/her) [:bytesized]]

When we added these frameworks, I was concerned about how opaque they are. ChannelPrefs can be read in about:support, but there was basically no good way to examine UpdateSettings without resorting to a hex editor. So I added the --channels-allowed switch. Passing only this argument to the updater causes it to print out the channels on the "allowed channels" list.

I wonder if this could be a reasonable way of checking this? I guess we would probably need something similar for ChannelPrefs, but that seems like it's probably reasonable.

Comment 5

[Julien Cristau [:jcristau] (back August 18)]

(In reply to Robin Steuber (they/them) [:bytesized] from comment #4)

When we added these frameworks, I was concerned about how opaque they are. ChannelPrefs can be read in about:support, but there was basically no good way to examine UpdateSettings without resorting to a hex editor. So I added the --channels-allowed switch. Passing only this argument to the updater causes it to print out the channels on the "allowed channels" list.

I wonder if this could be a reasonable way of checking this? I guess we would probably need something similar for ChannelPrefs, but that seems like it's probably reasonable.

The tests run on linux, they can't run the macos updater.

Comment 6

[[e-mail] (:bhearsum)]

In general, I think it's also best to rely on a bit for bit comparison as much as possible (with our known differences applied). Even if we ran mac update verify on macOS, a bug in --channels-allowed could result in a false pass of a test.