Closed
Bug 1164168
Opened 10 years ago
Closed 10 years ago
RFE: mozconfig option to disable unsigned addon warning at build time for non-Firefox applications
Categories
(Toolkit :: Add-ons Manager, defect, P1)
Tracking
()
People
(Reporter: kevink9876543, Assigned: mossop)
References
Details
(Whiteboard: [hijacking][fxsearch])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
gps
:
review+
dveditz
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
I've noticed that SeaMonkey nightlies addons manager is warning about unsigned addons: "[addon] could not be verified for use in SeaMonkey. Proceed with caution."
While this scary warning is very appropriate for unbranded Firefox builds, it makes no sense in an application that has no intention to enforce addon signing - it just needlessly spooks users into thinking most of our addons are suddenly dangerous or we'll be stuck to lose them.
Please make this warning, as well as all aspects of the signing enforcement, disable-able at build time with a mozconfig option, so that users of SeaMonkey and other non-Firefox Toolkit applications can rest assured that we can keep our custom addons.
Thanks.
Some comments from RattyAway from irc:
RattyAway The changes look pretty confined https://hg.mozilla.org/mozilla-central/rev/65afb97fc399
RattyAway should be eazy to add an extra condition to
RattyAway } else if (this._addon.signedState <= AddonManager.SIGNEDSTATE_MISSING) {
Some discussion at mozillaZine: http://forums.mozillazine.org/viewtopic.php?p=14151755#p14151755
Ratty also suggest in bug 1149702 comment 20 that a #ifdef for MOZ_REQUIRE_SIGNING could be a possible solution.
Reporter | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
I think the right solution here is an ifdef that makes verifyDirSignedState and verifyZipSignedState return undefined. That should flag everything as not needing signing and any messaging anywhere in the product should go away.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dtownsend
Flags: firefox-backlog+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [fxsearch][hijacking]
Reporter | ||
Updated•10 years ago
|
Assignee | ||
Comment 2•10 years ago
|
||
I think this is what we want, two build flags to control whether signatures are checked and required, set at the browser level only. Only trouble is it looks like MOZ_OFFICIAL_BRANDING isn't available in browser/confvars.sh so this doesn't work.
Should I move things in configure.in so MOZ_OFFICIAL_BRANDING is set sooner or should I do something else here?
Attachment #8604882 -
Flags: feedback?(gps)
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8604882 [details] [diff] [review]
patch
So with this patch, is there a way to *explicitly* disable building addon signing in .mozconfig for a non-Firefox build?
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to kevink9876543 from comment #3)
> Comment on attachment 8604882 [details] [diff] [review]
> patch
>
> So with this patch, is there a way to *explicitly* disable building addon
> signing in .mozconfig for a non-Firefox build?
No, with this patch add-on signing is already disabled for non-Firefox, there is no way to enable it
Reporter | ||
Comment 5•10 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #4)
> No, with this patch add-on signing is already disabled for non-Firefox,
> there is no way to enable it
Sounds good, thanks for the clarification.
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8604882 [details] [diff] [review]
patch
Ted, maybe you can look at this sooner than gps?
Attachment #8604882 -
Flags: feedback?(ted)
Updated•10 years ago
|
Rank: 10
Priority: -- → P1
Whiteboard: [fxsearch][hijacking] → [hijacking][fxsearch]
Assignee | ||
Comment 7•10 years ago
|
||
[Tracking Requested - why for this release]:
This causes problems for other applications
status-firefox40:
--- → affected
tracking-firefox40:
--- → ?
Based on the vote count (10 so far) and the impact (comment #1) adding a tracking flag for firefox40.
Comment 9•10 years ago
|
||
Comment on attachment 8604882 [details] [diff] [review]
patch
Review of attachment 8604882 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM.
::: browser/confvars.sh
@@ +72,5 @@
> +MOZ_ADDON_SIGNING=1
> +if test "$MOZ_OFFICIAL_BRANDING"; then
> + if test "$MOZ_UPDATE_CHANNEL" = "beta" -o \
> + "$MOZ_UPDATE_CHANNEL" = "release" -o \
> + "$MOZ_UPDATE_CHANNEL" = "esr"; then
I'm not a huge fan of this chained channel name check. IMO this should be encapsulated in its own variable defined in configure.in. We already have EARLY_BETA_OR_EARLIER. We may want to create BETA_OR_LATER. I /thought/ we already had such a mechanism. But on a quick glance, I can't find it, so maybe it doesn't exist.
Given the urgency of this request, I think a proper variable can be deferred to a follow-up bug.
Attachment #8604882 -
Flags: feedback?(gps) → feedback+
Updated•10 years ago
|
Status: UNCONFIRMED → ASSIGNED
Iteration: --- → 41.1 - May 25
Ever confirmed: true
Flags: qe-verify?
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #2)
> Created attachment 8604882 [details] [diff] [review]
> patch
>
> I think this is what we want, two build flags to control whether signatures
> are checked and required, set at the browser level only. Only trouble is it
> looks like MOZ_OFFICIAL_BRANDING isn't available in browser/confvars.sh so
> this doesn't work.
>
> Should I move things in configure.in so MOZ_OFFICIAL_BRANDING is set sooner
> or should I do something else here?
I think you missed the question I asked here
Flags: needinfo?(gps)
Comment 11•10 years ago
|
||
I don't see any uses of MOZ_OFFICIAL_BRANDING in the app-specific confvars.sh or included files. It should be safe to move MOZ_OFFICIAL_BRANDING to before confvars.sh is called. However, MOZ_BRANDING_DIRECTORY is set by a few confvars.sh files. Furthermore, processing of --enable-official-branding validates that MOZ_BRANDING_DIRECTORY is set. So this isn't simply going to be copy and paste.
Flags: needinfo?(gps)
Assignee | ||
Comment 12•10 years ago
|
||
This seems to work. GPS to review the build config changes, Dan to review the XPIProvider.jsm changes.
Attachment #8604882 -
Attachment is obsolete: true
Attachment #8604882 -
Flags: feedback?(ted)
Attachment #8608358 -
Flags: review?(gps)
Attachment #8608358 -
Flags: review?(dveditz)
Comment 13•10 years ago
|
||
Comment on attachment 8608358 [details] [diff] [review]
patch
Review of attachment 8608358 [details] [diff] [review]:
-----------------------------------------------------------------
r=dveditz for the XPIProvider.jsm bits.
Attachment #8608358 -
Flags: review?(dveditz) → review+
Comment 14•10 years ago
|
||
Comment on attachment 8608358 [details] [diff] [review]
patch
Review of attachment 8608358 [details] [diff] [review]:
-----------------------------------------------------------------
This is good enough to land.
If I had one nit, it would be to add configure flags for MOZ_ADDON_SIGNING and MOZ_REQUIRE_SIGNING. MOZ_REQUIRE_SIGNING is the more important one, as without it, developers will need to enable --enable-official-branding and pretend they are building on a release channel to activate this feature. That seems like a bit much. But presumably the audience who needs to do that is small. And you are part of that pool. So I'll leave this up to you to file a follow-up if necessary.
Attachment #8608358 -
Flags: review?(gps) → review+
Assignee | ||
Comment 16•10 years ago
|
||
Note that it will take up to a day after a user updates to a version with this change in it before the signing warnings will disappear in other apps. I still need to do up an aurora version of this patch.
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 17•10 years ago
|
||
Comment 18•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8609541 [details] [diff] [review]
aurora patch
This is essentially the same code uplifted to Aurora but without the bits to force signing on in beta/release since we don't want that in 40.
Approval Request Comment
[Feature/regressing bug #]: Various from add-on signing bugs
[User impact if declined]: Users of Thunderbird, Seamonkey etc. will see warnings about unsigned add-ons needlessly.
[Describe test coverage new/current, TreeHerder]: Running on m-c
[Risks and why]: This should have no impact on Firefox, only other applications are affected
[String/UUID change made/needed]: None
Flags: needinfo?(dtownsend)
Attachment #8609541 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8609541 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 20•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•