1646989 - Make `--disable-verify-mar` imply `DISABLE_UPDATER_AUTHENTICODE_CHECK`

Status: RESOLVED FIXED

(Firefox :: Installer, enhancement, P5)

Description

[Nick Alexander :nalexander [he/him]]

In the Setting up an update server MDN docs, there's a note that when debugging unsigned updates one should set --disable-verify-mar and then manually bump some lines referencing DISABLE_UPDATER_AUTHENTICODE_CHECK. Let's make the former imply the latter so that working in this area is just a different mozconfig away.

Comment 1

[Nick Alexander :nalexander [he/him]]

Even better would be to get rid of the separate flag/define, which doesn't seem to be particularly valuable. mhowell: any reason to not just use the existing MOZ_VERIFY_MAR_SIGNATURE (implied by --disable-verify-mar)?

Comment 2

[Molly Howell (she/her) (no longer active)]

I think this is a good idea. Having DISABLE_UPDATER_AUTHENTICODE_CHECK be controlled by some configure flag is just clearly a great idea, we should have done that ages ago. I can't see any compelling reason to keep them as separate flags either; there aren't really any good reasons to make builds with one but not the other.

If we are going to have that be the same flag that controls MOZ_VERIFY_MAR_SIGNATURE though, then we should rename it (and we should rename the preprocessor symbol as well if we use the same one for both) just to keep it from getting confusing reading that code.

There's only one tiny issue I can see, and it's not important. If anyone is shipping something with --disable-verify-mar set, then it would be surprising to them for that flag to begin also implying the authenticode change. But then again, they would only be losing security that they already did not have, because turning off MAR signing effectively breaks all of it. And if we're renaming the flag, that would mitigate this problem by forcing the surprising new thing to their attention. So I think we're fine not worrying about that.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1646989 - Replace --disable-verify-mar with --enable-unverified-updates, make it imply DISABLE_UPDATER_AUTHENTICODE_CHECK r=#firefox-build-system-reviewers!,nalexander!

This generalizes the existing --disable-verify-mar flag to be more
indicative of what it does, and makes it imply a further option that
was essentially required to use the flag for debugging.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:nalexander, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 5

[Nick Alexander :nalexander [he/him]]

Clearing NI since things have moved forward and I think we'll want to do some things slightly different in this area when (if) I revisit it.

Comment 6

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1646989 - Update docs to account for --enable-unverified-updates flag r=nalexander!

Also adds docs for how to run the Maintenance Service tests, which is another thing that the --enable-unverified-updates will be useful for.

Depends on D81279

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd42fdc548ac
Replace --disable-verify-mar with --enable-unverified-updates, make it imply DISABLE_UPDATER_AUTHENTICODE_CHECK r=firefox-build-system-reviewers,nalexander
https://hg.mozilla.org/integration/autoland/rev/3aa5fe508801
Update docs to account for --enable-unverified-updates flag r=nalexander,application-update-reviewers

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/bd42fdc548ac
https://hg.mozilla.org/mozilla-central/rev/3aa5fe508801