Digital signature causes extension styles to lose privileges

RESOLVED FIXED

Status

()

defect
--
major
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: gaubugzilla, Unassigned)

Tracking

Trunk
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

This issue seems to affect bootstrapped extensions only, at least I never noticed it before turning Adblock Plus into a bootstrapped extension. The minimal extension attached will simply open a XUL window (chrome://testtest/content/test.xul) that uses an external CSS file (chrome://testtest/skin/test.css). The CSS file defines the list-style-image property for an image in that window. The image displays fine if the extension is unsigned, the signed version of the same extension produces a warning in the Error Console however:

> Security Error: Content at file:///.../extensions/testtest@adblockplus.org.xpi may not load or link to chrome://testtest/skin/smiley.png.

It seems that the principal of the stylesheet is being determined by the digital signature rather than its protocol. I reproduced it in Firefox 12 and Firefox 16.0a1 (2012-06-14 nightly) on Windows 7 x64, other people reported the same issue for other Firefox versions as well.

Filing under Add-ons Manager because I'm unsure about the actual component causing this. Bug 726125 might be related.
Steps to reproduce:

1. Install either the signed or unsigned extension variant (attached to this bug).
2. The extension will install without a browser restart and immediately open a window.

Expected results:
A smiley image is visible in the window below the "An image should be visible below" text.

Actual results:
The smiley image is only visible when running the unsigned extension variant. The image doesn't show up with the signed variant and the Error Console displays the security error quoted in comment 0.
See Also: → 726125
Hmm, no idea where this should go either. CC'ing bz, who seems to have worked on signing issues in the past.

As a workaround, I bet it will work fine if the extension is unpacked.
(In reply to Wladimir Palant from comment #0)
> This issue seems to affect bootstrapped extensions only, at least I never
> noticed it before turning Adblock Plus into a bootstrapped extension.

Sorry, this was wrong - I think that the important change was getting rid of the inner JAR, not making the extension bootstrapped. As a result, chrome://adblockplus/ is now mapped to a jar: URL pointing to a signed archive (the XPI file). Only chrome://adblockplus/skin/ is affected, not CSS files served from chrome://adblockplus/content/.

(In reply to Blair McBride (:Unfocused) from comment #4)
> As a workaround, I bet it will work fine if the extension is unpacked.

I am sure that it will - but it will also degrade performance. So far the better workaround is to publish the extension unsigned. The signature provides close to no benefits, only causes issues.
The way signed jars work is they set the owner on the channel.

The way chrome:// URIs are given the system principal (or not) is via the chrome protocol handler setting an owner on the channel (or not).

If the chrome protocol handler ends up with a jar: channel for a signed jar, then it will set the owner, and then the jar code will go ahead and stomp on that.

One option is to not set the owner in signed jar code if there's already an owner on the channel.  But then it's not clear what the point of the signing is, at runtime... is there a point?
(In reply to Boris Zbarsky (:bz) from comment #6)
> One option is to not set the owner in signed jar code if there's already an
> owner on the channel.  But then it's not clear what the point of the signing
> is, at runtime... is there a point?

Originally, you were supposed to upload your web app as a signed JAR file - this would allow you to request privileges like UniversalBrowserRead. But from what I remember this is no longer supported. Not that anybody seriously used that, with Gecko being the only engine to support JAR urls...
(In reply to Boris Zbarsky (:bz) from comment #6)
> But then it's not clear what the point of the signing
> is, at runtime... is there a point?

At runtime, there isn't much of a point, IMO. And it causes issues like this and bug 726125.

Most of the benefit comes from when the xpi is downloaded. The way we handle that sucks at the moment, but going forward it'll become better.
> Originally, you were supposed to upload your web app as a signed JAR file

Sure.  I'm talking in the context of signed jars in extensions.

> At runtime, there isn't much of a point, IMO. And it causes issues like this and bug
> 726125.

Alright, then.  ;)

But now I'm confused.  nsJARChannel::GetOwner does in fact check for a non-null mOwner already...

Is this only happening for chrome:// channels that are NOT supposed to get a system principal?
> Only chrome://adblockplus/skin/ is affected

Ah, yes.  Only things without the system principal.  So comment 6 was off: the problem is not both places writing mOwner but just the fact that the URI passed to GetCertificatePrincipal is mJarBaseURI, but CheckLoadURI checks use the principal's scheme.

I wonder whether it would make sense, in nsJARChannel::GetOwner, to pass mOriginalURI to GetCertificatePrincipal when NS_GetFinalChannelURI(this) == mOriginalURI.  That would correspond to the "final jar location not important; what matters is the original URI you were created with" case, which is what we're looking at here.

Another option, of course, would be for the addon manager to repack into an unsigned jar at install time or something...  That would also fix bug 726125, which the proposal above does not fix.
Repacking extensions would also fix some more issues (like bad performance of well-compressed XPI files, bug 648213) but IMHO isn't likely to happen any time soon.
What Wladimir said. And at any rate, even with bug 648213 fixed, we might want/have to have an option to not do that.
Duplicate of this bug: 670572
This should be fixed as a byproduct of the fix for bug 726125. I cannot reproduce the issue with the latest nightly any longer...
Wladimir, can you verify this please?
Wladimir: ping. Is this fixed (by bug 726125) ?
Yes, seems fixed in 21.0a1 nightly (2013-01-23) on OpenSUSE.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.