Closed
Bug 1186522
Opened 9 years ago
Closed 8 years ago
force per checkin and release desktop firefox builds to require signed add-ons on beta 48
Categories
(Release Engineering :: General, defect)
Release Engineering
General
Tracking
(firefox41 affected, firefox42 fixed, firefox43 fixed, firefox47 wontfix, firefox48+ fixed)
People
(Reporter: jlund, Assigned: kmoir)
References
Details
(Whiteboard: [releng])
Attachments
(10 files, 17 obsolete files)
1.42 KB,
patch
|
rail
:
review+
|
Details | Diff | Splinter Review |
1.17 KB,
patch
|
mossop
:
review+
ritu
:
approval-mozilla-aurora+
jlund
:
checked-in+
|
Details | Diff | Splinter Review |
644 bytes,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
8.18 KB,
patch
|
Details | Diff | Splinter Review | |
6.62 KB,
patch
|
Details | Diff | Splinter Review | |
3.99 KB,
patch
|
mshal
:
review+
kmoir
:
checked-in+
|
Details | Diff | Splinter Review |
5.00 KB,
patch
|
mshal
:
review+
ritu
:
approval-mozilla-beta-
kmoir
:
checked-in-
|
Details | Diff | Splinter Review |
4.97 KB,
patch
|
mshal
:
review+
ritu
:
approval-mozilla-beta+
kmoir
:
checked-in+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
rail
:
review+
kmoir
:
checked-in+
|
Details | Diff | Splinter Review |
3.87 KB,
patch
|
Sylvestre
:
approval-mozilla-beta+
kmoir
:
checked-in+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•9 years ago
|
||
Jordan, is this is the right direction? I didn't enable it for release builds until we got the checkin builds working
Attachment #8639437 -
Flags: feedback?(jlund)
Reporter | ||
Comment 2•9 years ago
|
||
Reporter | ||
Comment 3•9 years ago
|
||
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #1) > Created attachment 8639437 [details] [diff] [review] > bug1186522.patch > > Jordan, is this is the right direction? I didn't enable it for release > builds until we got the checkin builds working looks pretty much like mine. I'd imagine we will end up taking a combination of our patches. I think we should enlist the 'pros' at this stage and see if we are on the right path. I noticed that you have things like: MOZ_REQUIRE_ADDON_SIGNING= iiuc - the above, will ignore anything set from the mozconfigs and thus never get pref'd on our off. Or am I reading that wrong? What we may need is something like: MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-0} # where we pass the env var and default to 0 if not defined. in the patch I was working on I scrapped trying to find all the mozconfigs leaf nodes and instead I'm using the common mozconfig that is used by all mozconfigs (iiuc). I'm not using the common-override one though as I want to be able to flip MOZ_REQUIRE_ADDON_SIGNING off for things like Seamonkey or our newly upcoming disable-require-signing builds, etc: https://bugzilla.mozilla.org/attachment.cgi?id=8639624 finally, I also deleted the branch specific logic currently in confvars.sh. I think what we want to do is not rely on that but instead be explicit in the beta, release, and esr trees themselves. So I was thinking of was: 1) landing something like https://bugzilla.mozilla.org/attachment.cgi?id=8639625 on m-c/m-a and then uplift it to esr. 2) then, landing something like the following on beta not on the next uplift but the one after that (Sept): https://bugzilla.mozilla.org/attachment.cgi?id=8639624 With the goal that (1) will be a no-op and we can land anywhere/anytime. and when (2) lands, will force all builds, unless specified in a leaf mozconfig, to require addon signing (MOZ_REQUIRE_ADDON_SIGNING) on m-b only. We will need to find out what seamonkey mozconfigs we need to patch to pref this off like we are doing in mozconfigs/linux64/nightly-no-add-on-sign mossop: could you just let me know if we are still on the same page. I'd like to save you from having to do this work yourself but at least get your nod that this goal is correct mshal: could you quickly poke the patches here and see if we are on the right track to my goal defined above. rail: this will take some coordination with merges and also define some migration/release logic so that https://bugzilla.mozilla.org/attachment.cgi?id=8639624 stays on m-b after every migration no matter what m-a says and also gets uplifted to m-r/esr on every cycle. Will that require much work? I guess this is decided by 'whitelisting' and migration scripts?
Flags: needinfo?(rail)
Flags: needinfo?(mshal)
Flags: needinfo?(dtownsend)
Reporter | ||
Comment 5•9 years ago
|
||
Comment on attachment 8639437 [details] [diff] [review] bug1186522.patch Review of attachment 8639437 [details] [diff] [review]: ----------------------------------------------------------------- commented in the bug
Attachment #8639437 -
Flags: feedback?(jlund) → feedback+
Comment 6•9 years ago
|
||
That reminds me that the merge scripts live in mozharness. :) Depending on what we want to do during merges, we'll need to change the configs, see http://hg.mozilla.org/build/mozharness/file/2982687f5fa8/configs/merge_day/aurora_to_beta.py#l4 as an example. something like config = { ... "replacements": [ ... ("build/mozconfig.common", "export MOZ_REQUIRE_ADDON_SIGNING=1", "export MOZ_REQUIRE_ADDON_SIGNING=0"), ("build/mozconfig.common", "# force add-ons to be signed", "# disable add-ons signing"), ... ] ... } You may want to apply this to multiple files in http://hg.mozilla.org/build/mozharness/file/2982687f5fa8/configs/merge_day
Flags: needinfo?(rail)
Comment 8•9 years ago
|
||
Comment on attachment 8639625 [details] [diff] [review] 150727_1186522_force_addon_signing_per_checkin-disables-by-branch-check.patch >diff --git a/configure.in b/configure.in >--- a/configure.in >+++ b/configure.in >@@ -3913,19 +3913,19 @@ ACCESSIBILITY=1 > MOZ_TIME_MANAGER= > MOZ_SIMPLEPUSH= > MOZ_PAY= > MOZ_AUDIO_CHANNEL_MANAGER= > MOZ_CONTENT_SANDBOX= > MOZ_GMP_SANDBOX= > MOZ_SANDBOX=1 > MOZ_BINARY_EXTENSIONS= > MOZ_ADDON_SIGNING= >-MOZ_REQUIRE_SIGNING= >+MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-0} I don't think you need the ${MOZ_REQUIRE_ADDON_SIGNING-0} bit here. Since MOZ_REQUIRE_ADDON_SIGNING is already in the mozconfig, I think you can just AC_SUBST it (would need to test though). You could use a construct like 'MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1}' in build/mozconfig.common if you wanted to have all builds default to MOZ_REQUIRE_ADDON_SIGNING=1, unless it was already explicitly set to 0 in something like the nightly-no-add-on-sign file. That's only necessary if nightly-no-add-on-sign includes mozconfig.common after MOZ_REQUIRE_ADDON_SIGNING is set, though - it may not be necessary here.
Flags: needinfo?(mshal)
Attachment #8639625 -
Flags: feedback+
Comment 9•9 years ago
|
||
Comment on attachment 8639624 [details] [diff] [review] 150727_1186522_force_addon_signing_per_checkin-enables-on-all-mozconfigs.patch >+export MOZ_REQUIRE_ADDON_SIGNING=0 >+export MOZ_REQUIRE_ADDON_SIGNING=1 Looks good, though I don't think you need the exports here.
Attachment #8639624 -
Flags: feedback+
Reporter | ||
Comment 10•9 years ago
|
||
thanks mshal! > I don't think you need the ${MOZ_REQUIRE_ADDON_SIGNING-0} bit here. Since > MOZ_REQUIRE_ADDON_SIGNING is already in the mozconfig, I think you can just > AC_SUBST it (would need to test though). Ah so I was wrong about that and we should be doing what Kim has. > You could use a construct like > 'MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1}' in > build/mozconfig.common if you wanted to have all builds default to > MOZ_REQUIRE_ADDON_SIGNING=1, unless it was already explicitly set to 0 in > something like the nightly-no-add-on-sign file. That's only necessary if > nightly-no-add-on-sign includes mozconfig.common after > MOZ_REQUIRE_ADDON_SIGNING is set, though - it may not be necessary here. I had a look and it seemed like mozconfig.common was sourced early in all the leaf mozconfigs so it may not be necessary but to be sure to be sure, it is probably better to use the construct you suggest. Kim, do you think it makes sense to use mozconfig.common and have it default REQUIRE_ADDON_SIGNING pref'd on. Then, pref'ing it off on the specific mozconfigs we choose? I argue this since it seems like we want this pref'd on for pretty much all ff/fennec builds on beta and there are a *lot* of leaf nodes to touch: http://gittup.org/blog/2015/01/12-combining-nodes-in-graphviz/mozconfigs.png
Assignee | ||
Comment 11•9 years ago
|
||
Jordan, yes that sounds like a good idea, I'll rework the patches OMG that dependency graph!!!
Reporter | ||
Comment 12•9 years ago
|
||
this should be a no-op for existing ff and fennec builds. It also adds a linux64 mozconfig that will no matter what mozconfig.common says will override and disable requiring signed addons. If r+, I would land this on m-c and uplift across trees. then, once beta hits 42, I can land on beta the following to actually flip on requiring all beta builds to have signed add-ons: diff --git a/build/mozconfig.common b/build/mozconfig.common --- a/build/mozconfig.common +++ b/build/mozconfig.common @@ -12,13 +12,13 @@ mk_add_options AUTOCLOBBER=1 ac_add_options --enable-crashreporter ac_add_options --enable-release # Enable checking that add-ons are signed by the trusted root MOZ_ADDON_SIGNING=1 -# Disable enforcing that add-ons are signed by the trusted root -MOZ_REQUIRE_ADDON_SIGNING=0 +# Enable enforcing that add-ons are signed by the trusted root +MOZ_REQUIRE_ADDON_SIGNING=1 . "$topsrcdir/build/mozconfig.automation" my staging master has this attachment and above paste on beta and behaves as I expect: normal build: [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com ~]# cd /builds/slave/m-beta-l64-0000000000000000000 [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-0000000000000000000]# grep MOZ_REQUIRE_ADDON_SIGNING build/obj-firefox/config.status (''' MOZ_REQUIRE_ADDON_SIGNING ''', ' 1 '), [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-0000000000000000000]# grep MOZ_ADDON_SIGNING build/obj-firefox/config.status (''' MOZ_ADDON_SIGNING ''', ' 1 '), [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-0000000000000000000]# no-addon-sign build: [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com ~]# cd /builds/slave/m-beta-l64-no-add-on-sign-0000 [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-no-add-on-sign-0000]# grep MOZ_ADDON_SIGNING build/obj-firefox/config.status (''' MOZ_ADDON_SIGNING ''', ' 1 '), [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-no-add-on-sign-0000]# grep MOZ_REQUIRE_ADDON_SIGNING build/obj-firefox/config.status [root@dev-linux64-ec2-012.dev.releng.use1.mozilla.com m-beta-l64-no-add-on-sign-0000]# before this lands, we will also need to address seamonkey/tb and any build outside of ff that uses mozconfig.common. philip, kevin: would you be able to help me out with this. we are changing the logic from https://bugzil.la/1164168 and I believe you will need to either 1) add the following to all sm leaf mozconfigs like I've done in this patch's nightly-no-add-on-sign: +MOZ_REQUIRE_ADDON_SIGNING=0 +MOZ_ADDON_SIGNING=0 2) override MOZ_REQUIRE_ADDON_SIGNING and MOZ_ADDON_SIGNING set in mozconfig.common in some common tb/sm mozconfig
Flags: needinfo?(philip.chee)
Flags: needinfo?(kevink9876543)
Attachment #8644137 -
Flags: review?(mshal)
Reporter | ||
Comment 13•9 years ago
|
||
if r+, this won't be landed until beta hits 42 mshal: it looks like gps reviewed mossop's similar patches in https://bugzil.la/1163046 and https://bugzil.la/1164168 so feel free to bounce these if you like
Attachment #8644138 -
Flags: review?(mshal)
Reporter | ||
Comment 14•9 years ago
|
||
rail: I believe this will be needed once my two previous patches land.
Attachment #8644139 -
Flags: review?(rail)
Comment 15•9 years ago
|
||
@jlund: Thanks for adding me to the CC list, but I'm not one to ask about the official SeaMonkey builds because I know nothing about that system, sorry (and I mostly make my own builds anyway, so for me I'd just add those two lines to my own mozconfig and be done).
Flags: needinfo?(kevink9876543)
Updated•9 years ago
|
Attachment #8644139 -
Flags: review?(rail) → review+
Comment 16•9 years ago
|
||
>+MOZ_REQUIRE_ADDON_SIGNING=0
>+MOZ_ADDON_SIGNING=0
Correct me if I am wrong but as long as we don't define either of these in our mozconfigs they are equivalent to:
MOZ_REQUIRE_ADDON_SIGNING=0
MOZ_ADDON_SIGNING=0
Flags: needinfo?(jlund)
Updated•9 years ago
|
Flags: needinfo?(philip.chee)
Comment 17•9 years ago
|
||
Is this work going to land on aurora before the merge? Otherwise we need to remove the code at https://dxr.mozilla.org/mozilla-central/source/browser/confvars.sh#69 or beta 41 will start failing
Comment 18•9 years ago
|
||
(In reply to Jordan Lund (:jlund) from comment #12) > before this lands, we will also need to address seamonkey/tb and any build > outside of ff that uses mozconfig.common. philip, kevin: would you be able > to help me out with this. we are changing the logic from > https://bugzil.la/1164168 and I believe you will need to either > > 1) add the following to all sm leaf mozconfigs like I've done in this > patch's nightly-no-add-on-sign: > +MOZ_REQUIRE_ADDON_SIGNING=0 > +MOZ_ADDON_SIGNING=0 > > 2) override MOZ_REQUIRE_ADDON_SIGNING and MOZ_ADDON_SIGNING set in > mozconfig.common in some common tb/sm mozconfig FYI/HEADS-UP to SM and TB people too.
Flags: needinfo?(rkent)
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(bugspam.Callek)
Flags: needinfo?(Pidgeot18)
Comment 19•9 years ago
|
||
Comment on attachment 8644137 [details] [diff] [review] 150805_bug_1186522_use_mozconfigs_instead_of_confvars_and_branches.patch Looks mostly good, though I think you've been bitrotted by a few patches to the mozconfigs since you started on this. >diff --git a/browser/config/mozconfigs/linux64/nightly-no-add-on-sign b/browser/config/mozconfigs/linux64/nightly-no-add-on-sign >+ac_add_options --enable-signmar See bug 1158870 - I think this goes away and gets replaced by --enable-verify-mar >+ >+ nit: Double newline >+# Nightlies only since this has a cost in performance >+ac_add_options --enable-js-diagnostics See bug 1171904 - I think this line goes away. >+# Use ccache >+. "$topsrcdir/build/mozconfig.cache" >+ >+. "$topsrcdir/build/mozconfig.common.override" See bug 1181040 - mozconfig.cache needs to come after mozconfig.common.override Basically, this mozconfig should be the same as 'nightly' but with those two variables set to 0, right? Up to you, but you could look into just including the nightly config and having build/mozconfig.common only setting the ADDON variables if they aren't already set. Eg: nightly-no-addon-sign: # addon signing is not required MOZ_REQUIRE_ADDON_SIGNING=0 MOZ_ADDON_SIGNING=0 . $topsrcdir/browser/config/mozconfigs/linux64/nightly build/mozconfig.common: ... # Enable checking that add-ons are signed by the trusted root MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} # Enable enforcing that add-ons are signed by the trusted root MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1} >diff --git a/toolkit/mozapps/extensions/internal/moz.build b/toolkit/mozapps/extensions/internal/moz.build > if CONFIG['MOZ_ADDON_SIGNING']: > DEFINES['MOZ_ADDON_SIGNING'] = 1 > >-if CONFIG['MOZ_REQUIRE_SIGNING']: >- DEFINES['MOZ_REQUIRE_SIGNING'] = 1 >+if CONFIG['MOZ_REQUIRE_ADDON_SIGNING']: >+ DEFINES['MOZ_REQUIRE_ADDON_SIGNING'] = 1 You should be able to delete all of these lines (both the MOZ_ADDON_SIGNING and MOZ_REQUIRE_ADDON_SIGNING), since you are using AC_DEFINE. Is it not working if you remove these?
Attachment #8644137 -
Flags: review?(mshal) → feedback+
Updated•9 years ago
|
Attachment #8644138 -
Flags: review?(mshal) → review+
Comment 20•9 years ago
|
||
Fallen, we'll need to modify the build setup for this at some point.
Flags: needinfo?(rkent)
Reporter | ||
Comment 21•9 years ago
|
||
(In reply to Philip Chee from comment #16) > >+MOZ_REQUIRE_ADDON_SIGNING=0 > >+MOZ_ADDON_SIGNING=0 > Correct me if I am wrong but as long as we don't define either of these in > our mozconfigs they are equivalent to: > > MOZ_REQUIRE_ADDON_SIGNING=0 > MOZ_ADDON_SIGNING=0 depends if you source build/mozconfig.common because I am doing the following: diff --git a/build/mozconfig.common b/build/mozconfig.common --- a/build/mozconfig.common +++ b/build/mozconfig.common @@ -10,10 +10,15 @@ # another mozconfig file, you'll need to use mozconfig.common.override instead # of this file. mk_add_options AUTOCLOBBER=1 ac_add_options --enable-crashreporter ac_add_options --enable-release +# Enable checking that add-ons are signed by the trusted root +MOZ_ADDON_SIGNING=1 +# Disable enforcing that add-ons are signed by the trusted root +MOZ_REQUIRE_ADDON_SIGNING=0 + and on beta, I'll soon also be: +MOZ_REQUIRE_ADDON_SIGNING=1 so you would need to override those vars (if they == 1) like I am in nightly-no-add-on-sign
Flags: needinfo?(jlund)
Reporter | ||
Comment 22•9 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #17) > Is this work going to land on aurora before the merge? Otherwise we need to > remove the code at > https://dxr.mozilla.org/mozilla-central/source/browser/confvars.sh#69 or > beta 41 will start failing yes that is the goal. If it doesn't, I'll back out the android and desktop confvars branch lines on Aug 9th
Blocks: 1178507
Comment 23•9 years ago
|
||
(In reply to Kent James (:rkent) from comment #20) > Fallen, we'll need to modify the build setup for this at some point. I've filed bug 1191941 for the Thunderbird part.
Reporter | ||
Comment 24•9 years ago
|
||
Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: [Describe test coverage new/current, TreeHerder]: [Risks and why]: [String/UUID change made/needed]: because tb and sm haven't had a chance to change their logic yet and the fact that we uplift + release beta1 on monday, I'm going to simply remove the branch logic that would break beta releases then follow up with the remaining bits next week. this patch needs to land and get uplifted to m-a before monday's merge.
Attachment #8645217 -
Flags: review?(dtownsend)
Attachment #8645217 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 25•9 years ago
|
||
addresses previous comments. tested in staging. interdiff: http://people.mozilla.org/~jlund/addon_signing_review_feedback-interdiff.patch
Attachment #8639437 -
Attachment is obsolete: true
Attachment #8639624 -
Attachment is obsolete: true
Attachment #8639625 -
Attachment is obsolete: true
Attachment #8644137 -
Attachment is obsolete: true
Attachment #8645219 -
Flags: review?(mshal)
Updated•9 years ago
|
Attachment #8645217 -
Flags: review?(dtownsend) → review+
Reporter | ||
Comment 27•9 years ago
|
||
Comment on attachment 8645217 [details] [diff] [review] 150807_bug_1186522_remove_beta_release_branch_logic_for_require_addon_signing.patch thanks, https://hg.mozilla.org/integration/mozilla-inbound/rev/a6f105aba8d2
Attachment #8645217 -
Flags: checked-in+
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
Comment on attachment 8645217 [details] [diff] [review] 150807_bug_1186522_remove_beta_release_branch_logic_for_require_addon_signing.patch Approved.
Attachment #8645217 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 30•9 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #29) > Comment on attachment 8645217 [details] [diff] [review] > 150807_bug_1186522_remove_beta_release_branch_logic_for_require_addon_signing > .patch > > Approved. thanks https://hg.mozilla.org/releases/mozilla-aurora/rev/7a647fc4d0ba
status-firefox42:
--- → fixed
status-firefox43:
--- → fixed
Comment 31•9 years ago
|
||
Comment on attachment 8645219 [details] [diff] [review] 150807_bug_1186522_use_mozconfigs_instead_of_confvars_and_branches.patch >diff --git a/browser/config/mozconfigs/linux64/nightly-no-add-on-sign b/browser/config/mozconfigs/linux64/nightly-no-add-on-sign >new file mode 100644 >--- /dev/null >+++ b/browser/config/mozconfigs/linux64/nightly-no-add-on-sign >@@ -0,0 +1,4 @@ >+# addon signing is checked but not required >+MOZ_REQUIRE_ADDON_SIGNING=0 >+MOZ_ADDON_SIGNING=1 These were both 0 in https://bug1186522.bmoattachments.org/attachment.cgi?id=8644137 - did you intend to change MOZ_ADDON_SIGNING to 1 here? >diff --git a/build/mozconfig.common b/build/mozconfig.common >+# Enable checking that add-ons are signed by the trusted root >+MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} >+# Disable enforcing that add-ons are signed by the trusted root >+MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1} Similarly, in https://bug1186522.bmoattachments.org/attachment.cgi?id=8644137 you had MOZ_REQUIRE_ADDON_SIGNING default to 0 in mozconfig.common. I think you wanted to have it 0 here and then changed to 1 in https://bug1186522.bmoattachments.org/attachment.cgi?id=8645219 right? So you'd have: MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-0} Everything else looks fine though, just wanted to call those out in case they are not intentional :)
Attachment #8645219 -
Flags: review?(mshal) → review+
Reporter | ||
Comment 32•9 years ago
|
||
> >--- /dev/null > >+++ b/browser/config/mozconfigs/linux64/nightly-no-add-on-sign > >@@ -0,0 +1,4 @@ > >+# addon signing is checked but not required > >+MOZ_REQUIRE_ADDON_SIGNING=0 > >+MOZ_ADDON_SIGNING=1 > > These were both 0 in > https://bug1186522.bmoattachments.org/attachment.cgi?id=8644137 - did you > intend to change MOZ_ADDON_SIGNING to 1 here? I intended this. I realized that checking if addon's were signed (MOZ_ADDON_SIGNING) is not a big deal. as it stands, that should be 1 across all our platforms already. MOZ_REQUIRE_ADDON_SIGNING is the main one we care about and I'd like to explicitly set that to 0 for this mozconfig. > > >diff --git a/build/mozconfig.common b/build/mozconfig.common > >+# Enable checking that add-ons are signed by the trusted root > >+MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} > >+# Disable enforcing that add-ons are signed by the trusted root > >+MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1} > > Similarly, in > https://bug1186522.bmoattachments.org/attachment.cgi?id=8644137 you had > MOZ_REQUIRE_ADDON_SIGNING default to 0 in mozconfig.common. oops, I got confused about this one. for now, this patch should be: +# Enable checking that add-ons are signed by the trusted root +MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} +# Disable enforcing that add-ons are signed by the trusted root +MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-0} then override that on beta with https://bug1186522.bmoattachments.org/attachment.cgi?id=8644138 <- and this patch needs updating too to reflect ${MOZ_REQUIRE_ADDON_SIGNING-1} style
Reporter | ||
Comment 33•9 years ago
|
||
carrying r+ interdiff: diff --git a/build/mozconfig.common b/build/mozconfig.common --- a/build/mozconfig.common +++ b/build/mozconfig.common @@ -13,12 +13,12 @@ mk_add_options AUTOCLOBBER=1 ac_add_options --enable-crashreporter ac_add_options --enable-release # Enable checking that add-ons are signed by the trusted root MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} # Disable enforcing that add-ons are signed by the trusted root -MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-1} +MOZ_REQUIRE_ADDON_SIGNING=${MOZ_REQUIRE_ADDON_SIGNING-0} . "$topsrcdir/build/mozconfig.automation"
Attachment #8645219 -
Attachment is obsolete: true
Attachment #8646044 -
Flags: review+
Reporter | ||
Comment 34•9 years ago
|
||
fighting bitrot
Attachment #8644138 -
Attachment is obsolete: true
Attachment #8646045 -
Flags: review?(mshal)
Updated•9 years ago
|
Attachment #8646045 -
Flags: review?(mshal) → review+
Reporter | ||
Comment 35•9 years ago
|
||
fyi tb and sm folks - I'll be landing https://bugzilla.mozilla.org/attachment.cgi?id=8646044 tomorrow and looking to uplift that to beta by eow seems like there is a patch in place or at least attached in https://bugzil.la/1191941 for tb case.
Reporter | ||
Comment 37•9 years ago
|
||
Comment on attachment 8646044 [details] [diff] [review] 150810_bug_1186522_use_mozconfigs_instead_of_confvars_and_branches.patch Approval Request Comment [Feature/regressing bug #]: this one: bug 1186522 [User impact if declined]: this lays the pipework for enforcing add-on signing in the future. this patch is required for when we actually change current pref values. [Describe test coverage new/current, TreeHerder]: none [Risks and why]: this patch should be a no-op. it simply rearranges where we set the enforcement and check for add-on signing. it does not change any mozconfig prefs [String/UUID change made/needed]: https://hg.mozilla.org/integration/mozilla-inbound/rev/292d13beeb7b
Attachment #8646044 -
Flags: checked-in+
Attachment #8646044 -
Flags: approval-mozilla-beta?
Attachment #8646044 -
Flags: approval-mozilla-aurora?
Comment 38•9 years ago
|
||
Backed out for breaking Gaia unit tests. https://treeherder.mozilla.org/logviewer.html#?job_id=13433430&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/f93f561fb199
Reporter | ||
Comment 39•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #38) > Backed out for breaking Gaia unit tests. > https://treeherder.mozilla.org/logviewer.html#?job_id=13433430&repo=mozilla- > inbound > > https://hg.mozilla.org/integration/mozilla-inbound/rev/f93f561fb199 hrm, I'll look into this today.
Comment on attachment 8646044 [details] [diff] [review] 150810_bug_1186522_use_mozconfigs_instead_of_confvars_and_branches.patch Aurora-, Beta- due to the back outs. Please renominate once all is well.
Attachment #8646044 -
Flags: approval-mozilla-beta?
Attachment #8646044 -
Flags: approval-mozilla-beta-
Attachment #8646044 -
Flags: approval-mozilla-aurora?
Attachment #8646044 -
Flags: approval-mozilla-aurora-
status-firefox41:
--- → affected
Reporter | ||
Comment 41•9 years ago
|
||
2 parts in this: 1) fights some bitrot where we added/changed MOZ_REQUIRE_SIGNING/MOZ_ADDON_SIGNING bits. * the part I'm worried about here is changing browser/components/moz.build. But I think removing the code in there is safe like we did previously for toolkit/mozapps/extensions/internal/moz.build * interdiff: //people.mozilla.org/~jlund/disable_moz_addon_signing_for_all_b2g.patch 2) disable MOZ_ADDON_SIGNING for all of b2g/* as I guess the b2g desktop builds picked up the common mozconfig change I did and we don't want to enforce/check addon signing for those afaik * interdiff: http://people.mozilla.org/~jlund/disable_moz_addon_signing_for_all_b2g.patch treeherder: before: https://treeherder.mozilla.org/#/jobs?repo=try&revision=23304a6590ff after: https://treeherder.mozilla.org/#/jobs?repo=try&revision=05dcebc63e3a
Attachment #8646044 -
Attachment is obsolete: true
Attachment #8656163 -
Flags: review?
Comment 42•9 years ago
|
||
Unfortunately we've decided to push forced signing out to 44 at the earliest. What needs to change here?
Flags: needinfo?(jlund)
Updated•9 years ago
|
Flags: needinfo?(Pidgeot18)
Reporter | ||
Comment 43•9 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #42) > Unfortunately we've decided to push forced signing out to 44 at the > earliest. What needs to change here? afaik, nothing. we can proceed landing https://bugzilla.mozilla.org/attachment.cgi?id=8656163 which has been on hold due to unknown future. we just basically won't be landing https://bugzilla.mozilla.org/attachment.cgi?id=8646045 on beta this merge but instead wait for 44
Flags: needinfo?(jlund)
Reporter | ||
Comment 44•9 years ago
|
||
> afaik, nothing. we can proceed landing
> https://bugzilla.mozilla.org/attachment.cgi?id=8656163 which has been on
> hold due to unknown future.
actually, we might not even need this at all if we end up going with a developer mode pref and a single builder/job for enforced-signing and not-forced in continuous integration.
I'll wait to see how the add-on mtg goes tomorrow.
Updated•9 years ago
|
Summary: force per checkin desktop and mobile firefox builds to require signed add-ons on beta 42 → force per checkin desktop and mobile firefox builds to require signed add-ons on beta 44
Reporter | ||
Comment 45•9 years ago
|
||
we don't need to land the patch yet so I am going to leave any cosmetic mozconfig changes till after merge/release week. we already took out the enforcement back in https://hg.mozilla.org/releases/mozilla-beta/rev/7a647fc4d0ba
Comment 46•8 years ago
|
||
Signing being enabled, but allowed to be turned off has landed in 43. The plan was to force signing in 44, is there an update on this?
Comment 47•8 years ago
|
||
(In reply to Andy McKay [:andym] from comment #46) > Signing being enabled, but allowed to be turned off has landed in 43. The > plan was to force signing in 44, is there an update on this?
Flags: needinfo?(jlund)
Reporter | ||
Comment 48•8 years ago
|
||
keeping needinfo open on this bug. discussing this in a mtg today. will report back by EOD
Reporter | ||
Comment 49•8 years ago
|
||
after meeting today, we are going to be enforcing addon signing the per-checkin and release builds for beta 45. Getting back up to speed, this bug requires: * a mozconfig change that flips MOZ_REQUIRE_SIGNING to 1 for desktop ff only on m-b. ** we should also change this to MOZ_REQUIRE_ADDON_SIGNING to not confuse with the actual ff signing itself * a migration/uplift patch similar to: https://bug1186522.bmoattachments.org/attachment.cgi?id=8644139
Flags: needinfo?(jlund)
Summary: force per checkin desktop and mobile firefox builds to require signed add-ons on beta 44 → force per checkin and release desktop firefox builds to require signed add-ons on beta 45
Updated•8 years ago
|
Flags: needinfo?(bugspam.Callek)
Comment 50•8 years ago
|
||
For some reason Bugzilla made an unwarrented removal for me. Justin, see comment #18
Flags: needinfo?(bugspam.Callek)
Comment 51•8 years ago
|
||
(In reply to Philip Chee from comment #18) > (In reply to Jordan Lund (:jlund) from comment #12) > > > before this lands, we will also need to address seamonkey/tb and any build > > outside of ff that uses mozconfig.common. philip, kevin: would you be able > > to help me out with this. we are changing the logic from > > https://bugzil.la/1164168 and I believe you will need to either > > > > 1) add the following to all sm leaf mozconfigs like I've done in this > > patch's nightly-no-add-on-sign: > > +MOZ_REQUIRE_ADDON_SIGNING=0 > > +MOZ_ADDON_SIGNING=0 > > > > 2) override MOZ_REQUIRE_ADDON_SIGNING and MOZ_ADDON_SIGNING set in > > mozconfig.common in some common tb/sm mozconfig > > FYI/HEADS-UP to SM and TB people too. I'm aware, thanks.
Flags: needinfo?(bugspam.Callek)
Comment 52•8 years ago
|
||
Seems like there's confusion in the community, and even among Firefox developers, what the actual plan is: https://groups.google.com/d/msg/firefox-dev/wR5MvjIIPV0/yEP7MGT9DwAJ Should we be removing the bits about "special unbranded builds" on beta/release from https://wiki.mozilla.org/Addons/Extension_Signing#FAQ ? I thought that was off the table now.
Comment 53•8 years ago
|
||
(In reply to Chris Cooper [:coop] from comment #52) > Seems like there's confusion in the community, and even among Firefox > developers, what the actual plan is: > > https://groups.google.com/d/msg/firefox-dev/wR5MvjIIPV0/yEP7MGT9DwAJ > > Should we be removing the bits about "special unbranded builds" on > beta/release from https://wiki.mozilla.org/Addons/Extension_Signing#FAQ ? I > thought that was off the table now. The unbranded builds are still planned. Aiming for a clarifying blog post soon.
Assignee | ||
Comment 54•8 years ago
|
||
andym: Could you provide insight on the scope of the unbranded builds? I seem to recall in a recent meeting that that the intent would be they would be on a periodic basis, but not on every commit. I looked for a blog post but didn't see one and catlee asked me about this today:-)
Flags: needinfo?(amckay)
Comment 55•8 years ago
|
||
301 over to kev who has been chatting to people about this
Flags: needinfo?(kev)
Comment 56•8 years ago
|
||
Scope of the unbranded builds would be installers based off what we test and release, but with official branding turned off, signing preference enabled, and a channel identified. No manual QA, and supplied on an as-is basis. No MAR generation for partial updates, and I don't know if anything's been discussed around whether we'd offer updates per release or expect users to update manually (this would be limited to update server config, we'd want to ensure the product checked for updates).
Flags: needinfo?(kev)
Assignee | ||
Comment 57•8 years ago
|
||
kev, thanks for the clarification. We read your comments in the addons meeting and have some further questions. So we would envision these as similar to partner builds they are a one-off are generated each point release on stable, with the two signing preference flipped to unsigned addons could be installed? And just for Windows, Windows 64-bit, Mac OS X, Linux and Linux 64-bit?
Flags: needinfo?(kev)
Assignee | ||
Comment 58•8 years ago
|
||
Some more questions after discussions with coop Should the build be unbranded? i.e. is a build without MOZ_OFFICIAL_BRANDING sufficient What parts of the release process they need to be generated for (aurora/beta/release)
Updated•8 years ago
|
Whiteboard: [releng]
Comment 59•8 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #58) > Some more questions after discussions with coop > > Should the build be unbranded? i.e. is a build without MOZ_OFFICIAL_BRANDING > sufficient The build should be unbranded, per the config you outline. > What parts of the release process they need to be generated for > (aurora/beta/release) They'd need to be available for beta and release. Nightly and Aurora maintain the pref already, but when we switch to mandatory the xpinstall.signatures.required will be ignored (this is, I believe, a mozconfig pref). As a result, we wouldn't be creating a partner build, we'd use the build flags to remove official branding and to keep the signing pref something that a developer could still flip to test against release-based code.
Flags: needinfo?(kev)
Assignee | ||
Comment 60•8 years ago
|
||
Talked to coop about this last week, he suggested that we should just land the configs for this on a try run in tree and get addons team to validate that this is what they are looking for before adding all the automation to generate builds that won't be widely used.
Updated•8 years ago
|
Flags: needinfo?(amckay)
Assignee | ||
Comment 61•8 years ago
|
||
email from coop on this dburns is interested in this build for other use cases "As a starting point, we'll need to set the branding flag, something like: ac_add_options --with-branding=browser/branding/unofficial You can check one of the existing build logs on buildbot/treeherder for a proper example. Whether that actually sets the proper addon flag, I don't know. That's what we need to figure out, and possibly change."
Assignee | ||
Comment 62•8 years ago
|
||
updated patch to require addon signing on beta
Assignee | ||
Comment 63•8 years ago
|
||
unbitrotten patch
Assignee | ||
Comment 64•8 years ago
|
||
Attachment #8726314 -
Attachment is obsolete: true
Assignee | ||
Comment 65•8 years ago
|
||
Trying another try build here https://treeherder.mozilla.org/#/jobs?repo=try&revision=58e909082aa5 from beta
Assignee | ||
Comment 66•8 years ago
|
||
Attachment #8726264 -
Attachment is obsolete: true
Assignee | ||
Comment 67•8 years ago
|
||
Trying another builder to fix branding and disable signing preference more thoroughly https://treeherder.mozilla.org/#/jobs?repo=try&revision=3dc48cc0686e
Assignee | ||
Comment 68•8 years ago
|
||
Attachment #8726317 -
Attachment is obsolete: true
Assignee | ||
Comment 69•8 years ago
|
||
So this looks better, preference is disabled but branding is still nightly. Is this expected? Lots of test failures, some related to addons. Not sure if this is because I used mozilla-beta and pushed to try.
Assignee | ||
Comment 70•8 years ago
|
||
Here is the build from yesterday that I created on try from the latest mozilla-beta code http://archive.mozilla.org/pub/firefox/try-builds/kmoir@mozilla.com-3dc48cc0686ee5f65836bb4aa6ed8e5d4145955b/try-macosx64/firefox-46.0.en-US.mac.dmg If someone could validate that this reflects the content that we want to run on CI on a periodic basis, that would be helpful. Then I'll add it to our configs.
Assignee | ||
Comment 71•8 years ago
|
||
As a note, you'll have to go into settings on your mac to be able to install it since it's not signed. System preferences -> Security & Privacy -> General temporarily set the preference to be Allow apps to be downloaded from Anywhere to install the dmg otherwise you'll get a message that the dmg is corrupt
Assignee | ||
Comment 72•8 years ago
|
||
Kev, I was talking to coop yesterday and he said you would have insight about the unbranded build should look like. What is the expectation from a legal perspective? Right now, I just have a build built from beta code that that has nightly branding but with the addon signing preferences disabled
Flags: needinfo?(kev)
Comment 73•8 years ago
|
||
That sounds perfect. Main thing is that we don't want to use official branding, lest there be confusion with release. (In reply to Kim Moir [:kmoir] from comment #72) > Kev, I was talking to coop yesterday and he said you would have insight > about the unbranded build should look like. What is the expectation from a > legal perspective? Right now, I just have a build built from beta code that > that has nightly branding but with the addon signing preferences disabled
Flags: needinfo?(kev)
Comment 74•8 years ago
|
||
So does this mean I can't use unsigned extensions anymore in Nightly?
Comment 75•8 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #74) > So does this mean I can't use unsigned extensions anymore in Nightly? No, this only applies to beta and release builds.
Comment 76•8 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #75) > (In reply to Martijn Wargers [:mwargers] (QA) from comment #74) > > So does this mean I can't use unsigned extensions anymore in Nightly? > > No, this only applies to beta and release builds. I'm trying to use the Special Powers extension in Nightly, which used to work for years, but recently stopped working without any error messages in the console. You're saying that this isn't happening because of disabling unsigned extensions?
Flags: needinfo?(dtownsend)
Comment 77•8 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #76) > (In reply to Dave Townsend [:mossop] from comment #75) > > (In reply to Martijn Wargers [:mwargers] (QA) from comment #74) > > > So does this mean I can't use unsigned extensions anymore in Nightly? > > > > No, this only applies to beta and release builds. > > I'm trying to use the Special Powers extension in Nightly, which used to > work for years, but recently stopped working without any error messages in > the console. > You're saying that this isn't happening because of disabling unsigned > extensions? I'm saying you can still turn off signing requirements in nightly by setting xpinstall.signatures.required to false same as you have been able to since we turned this feature on. I don't know what the Special Powers extensions is so I can't really say more than that.
Flags: needinfo?(dtownsend)
Comment 78•8 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #77) > I'm saying you can still turn off signing requirements in nightly by setting > xpinstall.signatures.required to false same as you have been able to since > we turned this feature on. I don't know what the Special Powers extensions > is so I can't really say more than that. It's used for mochitests and more, it works there by doing a temporary install from the Addon Manager or something. Setting the xpinstall.signatures.required pref to false is the first thing I did, which doesn't help. Bug 1186522 seems to indicate it should work, but it doesn't for me.
Comment 79•8 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #78) > (In reply to Dave Townsend [:mossop] from comment #77) > > I'm saying you can still turn off signing requirements in nightly by setting > > xpinstall.signatures.required to false same as you have been able to since > > we turned this feature on. I don't know what the Special Powers extensions > > is so I can't really say more than that. > > It's used for mochitests and more, it works there by doing a temporary > install from the Addon Manager or something. > Setting the xpinstall.signatures.required pref to false is the first thing I > did, which doesn't help. > Bug 1186522 seems to indicate it should work, but it doesn't for me. Please either file another bug or bring this up in a newgroup where we can figure this out without cluttering up this bug.
Assignee | ||
Comment 80•8 years ago
|
||
patch for m-b to enable addon signing Will have to update merge scripts so this is not overwritten upon uplift
Attachment #8737333 -
Flags: review?(dtownsend)
Assignee | ||
Updated•8 years ago
|
Attachment #8726376 -
Attachment is obsolete: true
Comment 81•8 years ago
|
||
Comment on attachment 8737333 [details] [diff] [review] bug1186522m-b.patch Review of attachment 8737333 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure that this is right since MOZ_REQUIRE_ADDON_SIGNING doesn't appear anywhere in the tree. Should it be MOZ_REQUIRE_SIGNING or is one of the other patches in this bug that changes to MOZ_REQUIRE_ADDON_SIGNING landing too? Either way I'd prefer a build config peer to sign off on this as I'm not totally sure on the syntax.
Attachment #8737333 -
Flags: review?(dtownsend)
Assignee | ||
Comment 82•8 years ago
|
||
Attachment #8737333 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Assignee: jlund → kmoir
Assignee | ||
Comment 83•8 years ago
|
||
Patch for mozilla-inbound to redefine MOZ_REQUIRE_SIGNING in a common mozconfig file on mozilla-inbound. Once this is uplifted, we can land the next patch on m-b to enable the requirement for signed addons on mozilla-beta desktop builds, but disabled for android. There is also a patch I need to write for the merge scripts so the settings on beta aren't overwritten.
Attachment #8737356 -
Attachment is obsolete: true
Attachment #8738001 -
Flags: feedback?(mshal)
Assignee | ||
Comment 84•8 years ago
|
||
patch for m-b as an aside, this is just to enable forcing addons to be signed The unbranded builds with signing disabled will be in later patches :mshal: jlund suggested you would be a good resource for feedback
Attachment #8738002 -
Flags: feedback?(mshal)
Comment 85•8 years ago
|
||
Comment on attachment 8738001 [details] [diff] [review] bug1186522m-i.patch Overall it looks good to me. >+if test "$MOZ_ADDON_SIGNING" = 1; then >+ AC_DEFINE(MOZ_ADDON_SIGNING) >+fi >+if test "$MOZ_REQUIRE_SIGNING" = 1; then >+ AC_DEFINE(MOZ_REQUIRE_SIGNING) >+fi Nowadays we would want this logic to go into moz.configure as a python script. However, since you need to uplift to beta, putting this in old-configure.in for now is fine. (For uplifting, you'll have to re-apply the change to configure.in). Just a heads up depending on when this lands. Also, you can get rid of this moz.build line: https://dxr.mozilla.org/mozilla-central/rev/d9f50aa0a1aaf90499b85c31e0f329b762e80fdd/toolkit/modules/moz.build#127 Since that is just defining MOZ_REQUIRE_SIGNING, but you're using AC_DEFINE which already does that. (The other three variables in the loop need to stay I'd guess).
Attachment #8738001 -
Flags: feedback?(mshal) → feedback+
Comment 86•8 years ago
|
||
Comment on attachment 8738002 [details] [diff] [review] bug1186522m-b.patch Seems straightforward.
Attachment #8738002 -
Flags: feedback?(mshal) → feedback+
Assignee | ||
Comment 87•8 years ago
|
||
Attachment #8738001 -
Attachment is obsolete: true
Attachment #8738250 -
Flags: review?(mshal)
Assignee | ||
Updated•8 years ago
|
Attachment #8738002 -
Flags: review?(mshal)
Updated•8 years ago
|
Attachment #8738250 -
Flags: review?(mshal) → review+
Updated•8 years ago
|
Attachment #8738002 -
Flags: review?(mshal) → review+
Comment 88•8 years ago
|
||
This should land at leastfew hours before we intend to start the beta 10 build on Monday.
Assignee | ||
Updated•8 years ago
|
Attachment #8738250 -
Flags: checked-in+
Assignee | ||
Comment 91•8 years ago
|
||
Comment on attachment 8738250 [details] [diff] [review] bug1186522m-i-2.patch backed out because xpcshell started failing https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=abe062737a19c27f6c392a4b9ced5c027266fc7d
Attachment #8738250 -
Flags: checked-in+ → checked-in-
Backed it out harder in https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb7a77cd764 because the original backout somehow managed to leave the signing requirement enabled, causing Mn tests to fail like https://treeherder.mozilla.org/logviewer.html#?job_id=25532084&repo=mozilla-inbound
(In reply to Wes Kocher (:KWierso) from comment #92) > Backed it out harder in > https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb7a77cd764 because > the original backout somehow managed to leave the signing requirement > enabled, causing Mn tests to fail like > https://treeherder.mozilla.org/logviewer.html#?job_id=25532084&repo=mozilla- > inbound Er, the original backout ended up removing the signing requirement, breaking a test that expected unsigned addons to fail, I guess.
Comment 94•8 years ago
|
||
(In reply to Kev Needham [:kev] from comment #73) > That sounds perfect. Main thing is that we don't want to use official > branding, lest there be confusion with release. > > (In reply to Kim Moir [:kmoir] from comment #72) > > Kev, I was talking to coop yesterday and he said you would have insight > > about the unbranded build should look like. What is the expectation from a > > legal perspective? Right now, I just have a build built from beta code that > > that has nightly branding but with the addon signing preferences disabled Does that mean that all the unbranded builds will be branded "Nightly"?
Comment 95•8 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #79) > (In reply to Martijn Wargers [:mwargers] (QA) from comment #78) > > (In reply to Dave Townsend [:mossop] from comment #77) > > > I'm saying you can still turn off signing requirements in nightly by setting > > > xpinstall.signatures.required to false same as you have been able to since > > > we turned this feature on. I don't know what the Special Powers extensions > > > is so I can't really say more than that. > > > > It's used for mochitests and more, it works there by doing a temporary > > install from the Addon Manager or something. > > Setting the xpinstall.signatures.required pref to false is the first thing I > > did, which doesn't help. > > Bug 1186522 seems to indicate it should work, but it doesn't for me. > > Please either file another bug or bring this up in a newgroup where we can > figure this out without cluttering up this bug. Filed bug 1263931.
Comment 96•8 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #94) > (In reply to Kev Needham [:kev] from comment #73) > > That sounds perfect. Main thing is that we don't want to use official > > branding, lest there be confusion with release. > > > > (In reply to Kim Moir [:kmoir] from comment #72) > > > Kev, I was talking to coop yesterday and he said you would have insight > > > about the unbranded build should look like. What is the expectation from a > > > legal perspective? Right now, I just have a build built from beta code that > > > that has nightly branding but with the addon signing preferences disabled > > Does that mean that all the unbranded builds will be branded "Nightly"? Mike raises a good point, and I made an assumption Kim was referring to the logo and visuals, which are the same as Nightly. For the avoidance of doubt, I want to make sure we're using the "unofficial" branding flag, which will incorporate logos that are the same as nightly, but use the "Mozilla Developer Preview" and associated branding in assets/info from http://mxr.mozilla.org/mozilla-central/source/browser/branding/unofficial/
Comment 97•8 years ago
|
||
Also in the interests of clarity, the plan of record is to allow a full beta cycle of testing before this hits release. Modifying the subject to make that clearer; signing enforcement will hit release in 47, so should be enabled for beta 47 at the latest, with unofficial builds available for testing.
Summary: force per checkin and release desktop firefox builds to require signed add-ons on beta 45 → force per checkin and release desktop firefox builds to require signed add-ons on beta 47
Assignee | ||
Comment 98•8 years ago
|
||
I tried these patches on try and the xpcshell tests failed, among others. Ahal do you have any insight into why this might be happening https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bd9b610af16202886ba9c8d73c18763db0ef798&selectedJob=19493604
Flags: needinfo?(ahalberstadt)
Comment 99•8 years ago
|
||
Sure, it's been awhile since the tests were greened up, and new bustages have likely slipped in. I'd say disable with extreme prejudice on beta-only and file a bug to fix it on all branches. I'm going to be on extended PTO starting this afternoon, so won't be able to help unfortunately. I'll be back in May. Bug 1259528 is filed to stop setting the pref to disable signing on central so new dependencies on unsigned addons stop slipping in. But even that has proven tricky and is temporarily stalled out (I'll pick it back up when I get back).
Flags: needinfo?(ahalberstadt)
Comment 100•8 years ago
|
||
Any updates? Will we be landing this for 47 beta?
Comment 101•8 years ago
|
||
> Any updates? Will we be landing this for 47 beta?
After the mess around expired add-ons, I'm wondering if it should be delayed again...
Assignee | ||
Comment 102•8 years ago
|
||
:Kev I'm trying to get the tests to pass on trunk on beta/m-i with the preference enabled. Right now, there are either problems with the mozconfigs and possibly all the test fixes did not land on beta. So I can't land this until the failing tests are green again. See bug 1264994 for details mshal: Do you have any insight on to why the patches aren't enabling signing. See https://bugzilla.mozilla.org/show_bug.cgi?id=1264994#c13 for details. I'm really not that familiar with the build side of things and I don't know why the preference is not being enabled as expected.
Flags: needinfo?(mshal)
Assignee | ||
Comment 103•8 years ago
|
||
From mshal in #releng 3:24 PM <mshal> kmoir: looks like we need both the AC_SUBST and AC_DEFINE, so something like this: https://pastebin.mozilla.org/8868922 3:24 PM <mshal> I didn't realize mozinfo.py was using the substs, I thought it was just used for preprocessor defines 3:25 PM <kmoir> mshal: thanks - will try that 3:25 PM <mshal> okie, let me know how it goes! diff --git a/old-configure.in b/old-configure.in index ab08949..a279b4a 100644 --- a/old-configure.in +++ b/old-configure.in @@ -6762,9 +6762,11 @@ if test -n "$MOZ_BINARY_EXTENSIONS"; then AC_DEFINE(MOZ_BINARY_EXTENSIONS) fi +AC_SUBST(MOZ_ADDON_SIGNING) if test "$MOZ_ADDON_SIGNING" = 1; then AC_DEFINE(MOZ_ADDON_SIGNING) fi +AC_SUBST(MOZ_REQUIRE_SIGNING) if test "$MOZ_REQUIRE_SIGNING" = 1; then AC_DEFINE(MOZ_REQUIRE_SIGNING) fi
Flags: needinfo?(mshal)
Assignee | ||
Comment 104•8 years ago
|
||
Attachment #8738250 -
Attachment is obsolete: true
Assignee | ||
Comment 105•8 years ago
|
||
new try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=02cc8a304f13
Assignee | ||
Comment 106•8 years ago
|
||
new patch for m-b now that try run on m-c looks good
Attachment #8738002 -
Attachment is obsolete: true
Assignee | ||
Comment 107•8 years ago
|
||
new try run on m-b https://treeherder.mozilla.org/#/jobs?repo=try&revision=543211c03155
Assignee | ||
Comment 108•8 years ago
|
||
Comment on attachment 8745521 [details] [diff] [review] bug1186522m-i-3.patch Thanks Mike, your suggestion yesterday fixed the problem!
Attachment #8745521 -
Flags: review?(mshal)
Assignee | ||
Comment 109•8 years ago
|
||
Running another try run because w-e10s 4 failed in previous try run on m-i https://treeherder.mozilla.org/#/jobs?repo=try&revision=25b192dd5a5f650cc29d7a4b0b4c3f5d4ce7e3f2
Updated•8 years ago
|
Attachment #8745521 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 110•8 years ago
|
||
So the try run I ran last night on beta had several test failures which I will investigate and see if this is related to test fixes that were not uplifted. Another issue is for this build is that I can still toggle xpinstall.signatures.required in about:config in this build. I thought this would be disabled so users can't change the preference https://treeherder.mozilla.org/#/jobs?repo=try&revision=25b192dd5a5f650cc29d7a4b0b4c3f5d4ce7e3f2 This morning I ran another try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=e175d0b7abab2464b11bdbf7791c880b59fac1ec this patch https://bugzilla.mozilla.org/show_bug.cgi?id=1264994#c20 to ensure the preferences are set correctly From this log http://archive.mozilla.org/pub/firefox/try-builds/kmoir@mozilla.com-e175d0b7abab2464b11bdbf7791c880b59fac1ec/try-macosx64/try-macosx64-bm87-try1-build16315.txt.gz it appears that Exception: {'require_signing': '1', 'addon_signing': '1'} are both set. Is there something that needs to be done besides this in to disable the preference? diff --git a/build/mozconfig.common b/build/mozconfig.common --- a/build/mozconfig.common +++ b/build/mozconfig.common @@ -11,9 +11,14 @@ # of this file. mk_add_options AUTOCLOBBER=1 ac_add_options --enable-crashreporter ac_add_options --enable-release +# Enable checking that add-ons are signed by the trusted root +MOZ_ADDON_SIGNING=${MOZ_ADDON_SIGNING-1} +# Enable enforcing that add-ons are signed by the trusted root +MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-1}
Flags: needinfo?(dtownsend)
Comment 111•8 years ago
|
||
The preference can be toggled if it's defined, but the change in the build should cause Firefox to ignore it (e.g. changing the pref will have no effect).
Comment 112•8 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #110) > So the try run I ran last night on beta had several test failures which I > will investigate and see if this is related to test fixes that were not > uplifted. > > Another issue is for this build is that I can still toggle > xpinstall.signatures.required in about:config in this build. I thought this > would be disabled so users can't change the preference What Kev said. We can't stop users toggling preferences. With MOZ_REQUIRE_SIGNING set we should just ignore the pref entirely.
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 114•8 years ago
|
||
Comment on attachment 8745521 [details] [diff] [review] bug1186522m-i-3.patch Try run showed w4 e10s tests passed https://treeherder.mozilla.org/#/jobs?repo=try&revision=25b192dd5a5f650cc29d7a4b0b4c3f5d4ce7e3f2 landed this patch on m-i
Attachment #8745521 -
Flags: checked-in+
Assignee | ||
Comment 115•8 years ago
|
||
Thanks Kev and mossop, for the clarification. I'll look at the test failures on beta and see what we can do to resolve them. I hope that most of them are just from test fixes from ahal that haven't been uplifted yet.
Comment 116•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3f0264791127
Assignee | ||
Updated•8 years ago
|
Attachment #8745809 -
Flags: review?(mshal)
Assignee | ||
Comment 117•8 years ago
|
||
So my try run from yesterday on m-b was okay except for failures on M other https://treeherder.mozilla.org/#/jobs?repo=try&author=kmoir@mozilla.com&selectedJob=20056985 opened bug 1268545 - for those issues
Depends on: 1268545
Updated•8 years ago
|
Attachment #8745809 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 118•8 years ago
|
||
rerunning a try run with the patches in bug 1256399 to ensure that the tests pass so once it lands we can ask for approval for the patches in this bug to enable the disabling of the pref https://treeherder.mozilla.org/#/jobs?repo=try&revision=a41428236474d805ebd7cd7ace5b4e8e587bfd21
Assignee | ||
Comment 119•8 years ago
|
||
Comment on attachment 8745809 [details] [diff] [review] bug1186522m-b-2.patch Approval Request Comment [Feature/regressing bug #]:1186522 [User impact if declined]: preference to install unsign addons can continue to be toggled by users [Describe test coverage new/current, TreeHerder]: N/A, this is already covered by existing tests [Risks and why]: enabling addon signing by default in all automated testing is wanted for 47 [String/UUID change made/needed]: none
Attachment #8745809 -
Flags: approval-mozilla-beta?
status-firefox47:
--- → affected
Comment 120•8 years ago
|
||
Comment on attachment 8745809 [details] [diff] [review] bug1186522m-b-2.patch Related to add-on signing required by default, Beta47+
Attachment #8745809 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Updated•8 years ago
|
Attachment #8745809 -
Flags: checked-in+
In addition to bug 1269498's failures, this also appears to have broken all android mochitests and reftests in similar ways. Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/6f82d30fe05e
Triggered android tests on your latest try run to see if they fail, too, but I'm pretty sure they will.
Flags: needinfo?(kmoir)
And indeed those manually triggered android tests are failing just like they were on beta.
Assignee | ||
Comment 125•8 years ago
|
||
I'm sorry, will run some more try runs to see the root cause of the problem. Thank you for backing it out.
Flags: needinfo?(kmoir)
Assignee | ||
Updated•8 years ago
|
Attachment #8745809 -
Flags: checked-in+ → checked-in-
Assignee | ||
Comment 126•8 years ago
|
||
I ran another try run this morning to try to address the failing android tests. And it still is all orange. https://treeherder.mozilla.org/#/jobs?repo=try&revision=17225f8079db I'm really stuck on this issue. I think the root cause is probably that the android builds have that MOZ_REQUIRE_SIGNING preference on which is causing the tests to fail. Since my understanding is that the work was not done to fix the android tests to work with signed addons. Mike do you see anything that is missing in the mozconfigs patches that will allow the preference to continue to be set looking at this mobile/android/config/mozconfigs/common includes . "$topsrcdir/build/mozconfig.common" which sets the value for MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-1} however, we have defined the following in mobile/android/config/mozconfigs/common.override # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. # This file is included at the bottom of all native android mozconfigs # # # Disable enforcing that add-ons are signed by the trusted root MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-0} . "$topsrcdir/build/mozconfig.common.override" . "$topsrcdir/build/mozconfig.cache" which to my understanding would disable the preference for Android
Flags: needinfo?(mshal)
Comment 127•8 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #126) > looking at this > mobile/android/config/mozconfigs/common includes > . "$topsrcdir/build/mozconfig.common" which sets the value for > MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-1} > however, we have defined the following in > mobile/android/config/mozconfigs/common.override > > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > # This file is included at the bottom of all native android mozconfigs > # > # > # Disable enforcing that add-ons are signed by the trusted root > MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-0} This syntax only sets the variable if it isn't already set. From the web of includes, it looks like build/mozconfig.common gets parsed first, which sets MOZ_REQUIRE_SIGNING to 1. So the variable is already set by the time it gets to here, and it isn't subsequently disabled. You could either change mobile/android/config/mozconfigs/common.override to: MOZ_REQUIRE_SIGNING=0 Or just move the MOZ_REQUIRE_SIGNING=${MOZ_REQUIRE_SIGNING-0} construct into mobile/android/config/mozconfigs/common before the '. "$topsrcdir/build/mozconfig.common"' line. This seems to fix it locally, but I'm just running configure and looking at the value of MOZ_REQUIRE_SIGNING in config.status. Let me know if that doesn't fix the actual problem with the tests and I can look further.
Flags: needinfo?(mshal)
Assignee | ||
Comment 128•8 years ago
|
||
Thanks Mike! I'm running a try run now with one of your suggestions.
Assignee | ||
Comment 129•8 years ago
|
||
This try run looks good so far. The mda tests that are orange are not enabled on beta, just on try https://treeherder.mozilla.org/#/jobs?repo=try&revision=2318cf7a8b7d&selectedJob=20312004
Attachment #8748367 -
Flags: review?(mshal)
Updated•8 years ago
|
Attachment #8748367 -
Flags: review?(mshal) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8748367 -
Flags: checked-in+
Assignee | ||
Comment 130•8 years ago
|
||
patch for merge day script looking at the script I think we just have to worry about the values that are different, the values that are added such as MOZ_REQUIRE_SIGNING=0 in mobile/android/config/mozconfigs/common.override on m-b should be fine
Attachment #8749192 -
Flags: review?(rail)
Updated•8 years ago
|
Attachment #8749192 -
Flags: review?(rail) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8749192 -
Flags: checked-in+
Assignee | ||
Comment 132•8 years ago
|
||
closing there is still the work to generate unbranded builds with the pref turned off in bug 1135781, I'm having problems with the branding on that, still working on it.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 133•8 years ago
|
||
Comment on attachment 8745809 [details] [diff] [review] bug1186522m-b-2.patch Reverting the approval as this was backed out. Please re-nom if it needs to land in m-b.
Attachment #8745809 -
Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Assignee | ||
Comment 134•8 years ago
|
||
ritu: actually the patch in comment 129 was landed on mozilla beta (the original patch was backed out). I verified that the preference cannot be toggled on a build subsequent to that. Do I need to request mozilla-beta approval again for that patch?
Flags: needinfo?(rkothari)
Comment 135•8 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #134) > ritu: actually the patch in comment 129 was landed on mozilla beta (the > original patch was backed out). I verified that the preference cannot be > toggled on a build subsequent to that. Do I need to request mozilla-beta > approval again for that patch? No you don't need to. I think I was confused because the patch was A+'d but status-firefox47 flag was still showing affected. I would like update patch (8745809) as A+ and status 47 to fixed if this has landed on m-b. Is that the case? Please let me know.
Flags: needinfo?(rkothari)
Comment 136•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8f425a67e923
Assignee | ||
Comment 137•8 years ago
|
||
Ritu: patch 8748367 landed on beta, not 8745809
Assignee | ||
Comment 138•8 years ago
|
||
Comment on attachment 8748367 [details] [diff] [review] bug1186522m-b-3.patch Approval Request Comment [Feature/regressing bug #]:1186522 [User impact if declined]: preference to install unsign addons can continue to be toggled by users [Describe test coverage new/current, TreeHerder]: N/A, this is already covered by existing tests [Risks and why]: enabling addon signing by default in all automated testing is wanted for 47 [String/UUID change made/needed]: none
Attachment #8748367 -
Flags: approval-mozilla-beta?
Comment 139•8 years ago
|
||
Comment on attachment 8748367 [details] [diff] [review] bug1186522m-b-3.patch Rubber stamping, this already landed on m-b.
Attachment #8748367 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 140•8 years ago
|
||
(In reply to Kim Moir [:kmoir] from comment #137) > Ritu: patch 8748367 landed on beta, not 8745809 Got it. Thanks! Now the fields are set correctly, we are good!
Comment 141•8 years ago
|
||
https://hg.mozilla.org/releases/comm-beta/rev/922d7a7db070e99930b99b03043e4d3ca655f0aa Keep build/ in sync (Bug 1186522). rs,a=bustage-fix
Comment 142•8 years ago
|
||
Hi Kim, Kev (Product manager) & team have decided to disable add-on signing enforcement for Fx47. Can we backout this change so that pref for add-on signing enforcement is off by default? Thanks!
Flags: needinfo?(kmoir)
Comment 143•8 years ago
|
||
To be VERY clear, Kev has decided to back out the mandatory enforcement of signing (e.g. removing the pref that allows people to disable signing enforcement). Because we don't have unbranded builds, we need to back out the change that made signing mandatory, with no option for disabling. The result of the change should be that Fx 47 will have singing enabled by default, and the xpinstall.signatures.required preference honored.
Comment 144•8 years ago
|
||
(In reply to Kev Needham [:kev] from comment #143) > To be VERY clear, Kev has decided to back out the mandatory enforcement of > signing (e.g. removing the pref that allows people to disable signing > enforcement). > > Because we don't have unbranded builds, we need to back out the change that > made signing mandatory, with no option for disabling. The result of the > change should be that Fx 47 will have singing enabled by default, and the > xpinstall.signatures.required preference honored. Ah! This is great. Thanks for the clarification.
Backed out of beta 47 in https://hg.mozilla.org/releases/mozilla-beta/rev/de10e990d861 Heads up to aleth, in case comm-beta needs a backout, too.
Flags: needinfo?(aleth)
Comment 146•8 years ago
|
||
This is now planned for Fx48.
status-firefox48:
--- → affected
tracking-firefox48:
--- → +
Comment 147•8 years ago
|
||
https://hg.mozilla.org/releases/comm-beta/rev/3451e9ca573fe9b52caa7cfe9f7ae127bd8c170a Backed out changeset 922d7a7db070 (bug 1186522). rs,a=aleth
Comment 148•8 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #145) > Heads up to aleth, in case comm-beta needs a backout, too. Thanks!
Flags: needinfo?(aleth)
Assignee | ||
Comment 149•8 years ago
|
||
thanks for backing those out, yesterday was a holiday in Canada
Status: RESOLVED → REOPENED
Flags: needinfo?(kmoir)
Resolution: FIXED → ---
Assignee | ||
Comment 150•8 years ago
|
||
when patches were backed out last month, this part was removed accidentally
Comment 151•8 years ago
|
||
Pushed by kmoir@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dbadac2844a9 force per checkin and release desktop firefox builds to require signed add-ons on beta 47 r=mshal DONTBUILD
Assignee | ||
Updated•8 years ago
|
Attachment #8760861 -
Flags: checked-in+
Comment 152•8 years ago
|
||
kmoir, those settings were deliberately re-added in bug 1277965, we want ADDON_SIGNING to be enabled by default in local builds. I'm not seeing the connection between removing those flags for local builds and the topic of this bug?
Flags: needinfo?(kmoir)
Comment 153•8 years ago
|
||
Pushed by kmoir@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/528945d5a054 revert Bug 1186522 - force per checkin and release desktop firefox builds to require signed add-ons on beta 47 r=mshal DONTBUILD
Assignee | ||
Comment 154•8 years ago
|
||
sorry, I didn't realize this, I have removed my patch. I didn't realize it was related to another bug.
Flags: needinfo?(kmoir)
Assignee | ||
Updated•8 years ago
|
Attachment #8760861 -
Attachment is obsolete: true
Attachment #8760861 -
Flags: checked-in+
Assignee | ||
Comment 155•8 years ago
|
||
Assignee | ||
Comment 156•8 years ago
|
||
Comment on attachment 8762240 [details] [diff] [review] bug1186522m-b-4.patch (unbitrotten) Approval Request Comment [Feature/regressing bug #]:1186522 [User impact if declined]: preference to install unsign addons can continue to be toggled by users [Describe test coverage new/current, TreeHerder]: N/A, this is already covered by existing tests [Risks and why]: enabling addon signing by default in all automated testing is desired on beta [String/UUID change made/needed]: none
Attachment #8762240 -
Flags: approval-mozilla-beta?
Comment 157•8 years ago
|
||
Kev, can you share your plan for this bug in 48? What changed since last month and 47? Thanks
Flags: needinfo?(kev)
Assignee | ||
Comment 158•8 years ago
|
||
I'm not kev but what has changed since last time is that we have builds for addon developers in bug 1135781 that allow addon developers to test their unsigned addons. This is what was missing when we enabled the preference last time, and thus why it was backed out at Kev's direction.
Comment 159•8 years ago
|
||
Kev is on PTO this week, if I can help, please let me know. Everything Kim said is spot on though.
Flags: needinfo?(kev)
Comment 160•8 years ago
|
||
I would like to know if you are planning to ship addon signing in 48. We had too many last minute changes and I would like to avoid that.
Comment 161•8 years ago
|
||
unbranded builds are the only known blocker
Updated•8 years ago
|
Attachment #8762240 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 162•8 years ago
|
||
Comment on attachment 8762240 [details] [diff] [review] bug1186522m-b-4.patch (unbitrotten) https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=35e0d7dd7cd7bff301b75756361b36b275102404 will verify preference once the build are available
Attachment #8762240 -
Flags: checked-in+
Assignee | ||
Comment 163•8 years ago
|
||
Comment on attachment 8762240 [details] [diff] [review] bug1186522m-b-4.patch (unbitrotten) I backed out enabling the preferences on beta until the tests are fixed (bug 1282568)
Attachment #8762240 -
Flags: checked-in+
Assignee | ||
Comment 164•8 years ago
|
||
I verified that the builds don't allow you to install unsigned addons in the revision we backed out so we should be good to go once the tests are fixed. https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=35e0d7dd7cd7bff301b75756361b36b275102404
Assignee | ||
Comment 165•8 years ago
|
||
aswan is running a try run to test fixes for the test failures for bug 1282568 and bug 1282868 https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0f5c530b95b
Assignee | ||
Comment 166•8 years ago
|
||
New try one since last one had issues with uploaded addons https://treeherder.mozilla.org/#/jobs?repo=try&revision=11d1f8e6095c
Assignee | ||
Updated•8 years ago
|
Attachment #8762240 -
Flags: checked-in+
Assignee | ||
Comment 167•8 years ago
|
||
The preference is now enabled on beta and the tests results seem to be good. Thanks andym and aswan for fixing the tests.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Summary: force per checkin and release desktop firefox builds to require signed add-ons on beta 47 → force per checkin and release desktop firefox builds to require signed add-ons on beta 48
Comment 168•8 years ago
|
||
https://hg.mozilla.org/releases/comm-beta/rev/10cbb73a354b0b7ae31afb341e02ae14125db0ff Keep build/ in sync (port bug 1186522). rs,a=bustage-fix
Comment 169•8 years ago
|
||
On a side note, with [xpinstall.signatures.required=false], in about:addons there's still yellow-text warning msgs instead of red ones for unsigned addons. Shouldn't they be red always to show that the preference is completely ignored already? Firefox 48.0b6, Build ID: 20160706215822 Regards!
Assignee | ||
Comment 170•8 years ago
|
||
andym: Do you have any comments about the previous question? I'm not sure if the preference colour should change in this case?
Flags: needinfo?(amckay)
Comment 171•8 years ago
|
||
I don't follow comment 169, but if there is a problem there Javier, please file a new bug rather than add on to this one, its long enough already. I installed an unsigned add-on in Firefox 47 and it shows up as yellow: https://www.dropbox.com/s/mbwtpt0ji3tgtcs/Screenshot%202016-07-11%2013.10.01.png?dl=0 I then ran Firefox 48 beta 6 with the same profile and it shows up as red: https://www.dropbox.com/s/i63a54lgbq8u9h1/Screenshot%202016-07-11%2013.07.41.png?dl=0
Flags: needinfo?(amckay) → needinfo?(redlibrepy)
Comment 172•8 years ago
|
||
(In reply to Andy McKay [:andym] from comment #171) > I installed an unsigned add-on in Firefox 47 and it shows up as yellow: > https://www.dropbox.com/s/mbwtpt0ji3tgtcs/Screenshot%202016-07-11%2013.10.01.png?dl=0 > > I then ran Firefox 48 beta 6 with the same profile and it shows up as red: > https://www.dropbox.com/s/i63a54lgbq8u9h1/Screenshot%202016-07-11%2013.07.41.png?dl=0 If you set [xpinstall.signatures.required] to false, the yellow text ("... Proceed with caution.") appears. Fx 48.0b7, Build ID: 20160711002726 (Windows_NT 6.1 x86) I mentioned this just bc I think that colour/message can be misleading, now that [xpinstall.signatures.required] is already ignored. Sorry to bother.
Comment 173•8 years ago
|
||
Marking fixed for 48 based on comment 167.
Updated•8 years ago
|
Flags: needinfo?(redlibrepy)
Assignee | ||
Comment 174•8 years ago
|
||
Now that the m-b -> m-r merge is complete, I've verified that this preference is set correctly on m-r builds and that unsigned addons cannot be installed.
Blocks: 1290993
Updated•7 years ago
|
Flags: needinfo?(ewong)
Comment 175•6 years ago
|
||
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Updated•6 years ago
|
Component: General Automation → General
Reporter | ||
Comment 176•4 years ago
|
||
Comment on attachment 8656163 [details] [diff] [review] 150902_addon_signing_disable_all_b2g.patch I guess if no one will give me a review after 4 years, we don’t need this patch /s This no longer applies.
Attachment #8656163 -
Flags: review?
You need to log in
before you can comment on or make changes to this bug.
Description
•