Closed Bug 1158467 Opened 9 years ago Closed 9 years ago

Signed extensions on AMO have invalid signatures (And cause to many crashes for Fx28 and older versions)

Categories

(addons.mozilla.org :: Security, defect, P1)

Product:
addons.mozilla.org
Component:
Security
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mark, Unassigned)

References

(
URL
)

Details

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.3) Gecko/20150323 Firefox/31.9 PaleMoon/25.3.1
Build ID: 20150323170640

Steps to reproduce:

Signed AMO add-ons as pushed out today cause issues in some versions of Firefox and derivatives (among other things: crashes of the affected browsers).

This seems to be directly caused by not adhering to the JAR signing specification in XPI files.
http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.html#Signature_File

Your supplied .sf files are missing file information as is required by the spec.
They only have a manifest digest in them, and no individual file listings to verify.
As a side note: if I understand the spec correctly, not having any files listed in the .sf files will simply prevent signatures from being checked -- this kind of defeats the point of signing the XPI JARs, to begin with?
Severity: normal → critical
Thanks for reporting, we're going to have a look into that asap.

:rtilder, :mossop and :dveditz, do I need to add more/other peoples to this bug?

:jason should we revert in the meantime?
Flags: needinfo?(rtilder)
Flags: needinfo?(jthomas)
Flags: needinfo?(dveditz)
Flags: needinfo?(dtownsend)
Priority: -- → P1
Same here since some days.
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
A. Heinecke, can you please confirm "since some days"? We've signed the first addons yesterday, and according to some other reporter on irc, the issue is visible on Firefox versions older than 28
Mathieu Agopian: researching the date I've commented this on #rust-offtopic atm
Mathieu Agopian:
2015.04.25 I've experienced this.
Could be -3/4 days since the bug is appearing, because I'm manually checking for updates every week.
[But I know for a certain reason that it should be only 3/4 days max since the last time I've updated a plugin back then.]

Console extract: (of the current 3 updates available, which are all failing)
>1429965505402	addons.xpi	WARN	Download of https://addons.cdn.mozilla.net/user-media/addons/722/noscript_security_suite-2.6.9.22-sm+fn+fx.xpi failed: Downloaded file hash (0c6774cd5479a315253eb403c6a7c074658154630ac9c4bc481aa0aba6eb16a8) did not match provided hash (c12fd43da6db7a976518b3df6cbcf99ff6260d18554a87d0c13332b4c5417b6e)
>1429965775267	addons.xpi	WARN	Download of https://addons.cdn.mozilla.net/user-media/addons/1843/firebug-2.0.9-fx.xpi failed: Downloaded file hash (74d5d66bdaa0b78542d791db2e8480f7f396f85345b85524417991b1d597674a) did not match provided hash (dd3da6e3ca2ea15f062cd69d457bc99071c78ae349406f3cb69555aceb65010d)
>1429965775279	addons.xpi	WARN	Download of https://addons.cdn.mozilla.net/user-media/addons/472577/classic_theme_restorer_customize_ui-1.3.0-fx.xpi failed: Downloaded file hash (9aa2446b3f2db9b93aee2e3cc1a6eb947370717f6abe3e8e071d5e04f812e23f) did not match provided hash (6c3f1212daa9b0cac44d1c225f9f906f1040507b6ec89d72df2736bf1cea86a0)
I'm sorry, didn't check the date.
Asking the others at the moment, since HexChat doesn't log Dates per default.
Problem occurs since the last week ~21/22
Ok A. Heinecke: this is not linked to this bug (and it can't be that old, we started signing yesterday).

Your issue is known already, and should be fixed shortly (it's a CDN cache issue).

From what I can understand from the poster, this issue is an entirely different problem, causing the browser to crash because of invalid jar signing.
Mathieu Agopian that's possible.
(Timestamp was 2015-04-24 18:33:40 UTC-4)
(In reply to Mathieu Agopian [:magopian] from comment #4)
> issue is visible on Firefox versions older than 28

Actually, that *includes* Firefox 28.
The Mozilla-signed extensions seem to hit bug 952110 on FF 28 and below and immediately crashes upon trying to install a Moz-signed extension, and any derivatives that do not have the patch in place for this bug.
Anything later would prevent the crash (but as said still won't be checking anything because the .sf would be empty).
I see this issue on Firefox 28 too. I was able to replicate a crash when installing https://addons.mozilla.org/en-US/firefox/addon/firebug/?src=hp-dl-mostpopular. crash report https://crash-stats.mozilla.com/report/index/9340c559-72af-480f-997c-f0a072150425
Flags: needinfo?(jthomas)
Other reports on 2015-04-24 or 2015-04-25 include:

"What causes extension updating errors?"
SUMO
https://support.mozilla.org/en-US/questions/1059056#answer-721526

"Addon hash mis-match"
http://forums.mozillazine.org/viewtopic.php?f=23&t=2930585

"ADDON Updates Not Downloading Tonight??"
http://forums.mozillazine.org/viewtopic.php?f=7&t=2930627
a more general thread at mozillazine, points to issue starting on 2015-04-24.

I think this might be fall out from 

"Add pref to require installs be signed by a Mozilla-issued add-on signing certificate"
https://bugzilla.mozilla.org/show_bug.cgi?id=1038068

DJ-Leith
Blocks: 1070152
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Signed extensions on AMO have invalid signatures → Signed extensions on AMO have invalid signatures (And cause to many crashes for Fx28 and older versions)
I see this crashes on Fx28 and many users mention this bug.


For example, install the https://addons.mozilla.org/firefox/addon/aboutme/ (about:me 0.7.1-signed) on Firefox 28.0 will cause a crash (when rendering an add-on installation interface).


I seems to found a key node, if I modify the "aboutme-0.7-fx.xpi/META-INF/mozilla.sf" file, add a new line to the content end (no changes for the file name, character encoding, or newline character), this crash no longer occurs.
Here's a PR that reverts the signing: https://github.com/mozilla/olympia/pull/526

Merged in https://github.com/mozilla/olympia/commit/e6253e3caa69acea2d5a83123e517f3950a8673a

We're going to deploy that on production, and make sure it fixes those crashes.
(In reply to Mark Straver from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.3) Gecko/20150323
> Firefox/31.9 PaleMoon/25.3.1
> Build ID: 20150323170640
> 
> Steps to reproduce:
> 
> Signed AMO add-ons as pushed out today cause issues in some versions of
> Firefox and derivatives (among other things: crashes of the affected
> browsers).
> 
> This seems to be directly caused by not adhering to the JAR signing
> specification in XPI files.
> http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.
> html#Signature_File
> 
> Your supplied .sf files are missing file information as is required by the
> spec.
> They only have a manifest digest in them, and no individual file listings to
> verify.

I don't know if Firefox has ever religiously followed the JAR signing spec. All Firefox requires is that every file in the XPI be listed in manifest.mf, the hash for the manifest is listed in the .sf file and the .sf file is correctly signed by the .rsa file. This ensures nothing in the XPI has been modified since signing.
Flags: needinfo?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #15)
> I don't know if Firefox has ever religiously followed the JAR signing spec.

I assumed it did, since there has never been any documentation otherwise -- and initially the extension files have always been supplied as actual JAR files (and your way of signing does follow the spec otherwise).

> All Firefox requires is that every file in the XPI be listed in manifest.mf,
> the hash for the manifest is listed in the .sf file and the .sf file is
> correctly signed by the .rsa file. This ensures nothing in the XPI has been
> modified since signing.

Then this effectively prevents multiple vendors from signing different files in a single XPI -- you probably want to change that. Even if not religiously following the signing specification right now, those spec rules are there for a reason and you should consider following them ;)

The manifest should contain a list of all files in the XPI that are signed by *someone*, and the individual .sf files should list those files signed with the included, equally named, rsa file, one matching sf/rsa pair per vendor.
Additional info from the document listing the spec, if you decide to follow it:

"Not all files in the JAR file need to be listed in the manifest as entries, but all files which are to be signed must be listed."

"Each signer is represented by a signature file with extension .SF. The major part of the file is similar to the manifest file. It consists of a main section which includes information supplied by the signer but not specific to any particular jar file entry. ... The main section is followed by a list of individual entries whose names must also be present in the manifest file. Each individual entry must contain at least the digest of its corresponding entry in the manifest file."

If you decide not to follow the official JAR signing spec, then please provide proper documentation of Mozilla's own signing spec -- although I strongly suggest to simply adhere to a long-standing standard.
The 28.x and older editions' crashes fixed by bug 952110 ( 29+ )
xunxun, so you want to intentionally break older versions?
(In reply to Lena from comment #19)
> xunxun, so you want to intentionally break older versions?

I just want to say why 29.0+ editions don't crash.
(In reply to Mark Straver from comment #16)
> (In reply to Dave Townsend [:mossop] from comment #15)
> > I don't know if Firefox has ever religiously followed the JAR signing spec.
> 
> I assumed it did, since there has never been any documentation otherwise --
> and initially the extension files have always been supplied as actual JAR
> files (and your way of signing does follow the spec otherwise).
> 
> > All Firefox requires is that every file in the XPI be listed in manifest.mf,
> > the hash for the manifest is listed in the .sf file and the .sf file is
> > correctly signed by the .rsa file. This ensures nothing in the XPI has been
> > modified since signing.
> 
> Then this effectively prevents multiple vendors from signing different files
> in a single XPI -- you probably want to change that. Even if not religiously
> following the signing specification right now, those spec rules are there
> for a reason and you should consider following them ;)

Yeah this is a known issue with the way we check signatures right now. There hasn't yet been a compelling reason to do the work to fix it. We don't really expose who signed what out of an XPI and think very few of our users would benefit from that sort of information.
So, this is almost certainly tied to bug 952110.  Bug 952110 comment 8 has a workaround that may apply.  Does someone have one of the signed XPIs so we can verify if that workaround applies?
URL: https://bugzilla.mozilla.org/show_bug...
(In reply to Mike Connor [:mconnor] from comment #22)
> So, this is almost certainly tied to bug 952110.  Bug 952110 comment 8 has a
> workaround that may apply.  Does someone have one of the signed XPIs so we
> can verify if that workaround applies?

The workaround doesn't apply, I'm going to dig in today and see if I can figure out how we can workaround this.
Flags: needinfo?(rtilder)
Flags: needinfo?(dveditz)
(In reply to Dave Townsend [:mossop] from comment #23)
> (In reply to Mike Connor [:mconnor] from comment #22)
> > So, this is almost certainly tied to bug 952110.  Bug 952110 comment 8 has a
> > workaround that may apply.  Does someone have one of the signed XPIs so we
> > can verify if that workaround applies?
> 
> The workaround doesn't apply, I'm going to dig in today and see if I can
> figure out how we can workaround this.

A slight variation of it works. We need to add an additional newline to the end of mozilla.sf. This breaks the signature of course so this needs to happen before signing.
Depends on: 1158938
Mozilla's own individually signed extensions sign according to spec. Why not simply follow and sign all the files needed in the xpi properly?
Of note, other people have signed their extensions as well properly according to jar signing spec, and those have not crashed the browser, either. So just do that, and the problem would be solved? You're signing already anyway, might as well do it the way it's supposed to be done...
You do realize this will have to be fixed according to spec before you lock down the browser in release versions. Because right now someone could take the extension as is and modify it and it would not show with a "This add-on is corrupted" message and just install and run normally.

It defeats the entire reason for doing this at all.
(In reply to Matt A. Tobin [:BinOC] from comment #27)
> You do realize this will have to be fixed according to spec before you lock
> down the browser in release versions. Because right now someone could take
> the extension as is and modify it and it would not show with a "This add-on
> is corrupted" message and just install and run normally.
> 
> It defeats the entire reason for doing this at all.

That isn't the case, see comment 15
See Also: → 1159055
(In reply to Dave Townsend [:mossop] from comment #21)
> Yeah this is a known issue with the way we check signatures right now. There
> hasn't yet been a compelling reason to do the work to fix it. We don't
> really expose who signed what out of an XPI and think very few of our users
> would benefit from that sort of information.

That begs the question then: If you are going to force Mozilla-signed extensions for Firefox, with only one possible vendor in the XPI because of this half-specced signing, how are independently-signed extensions going to be handled then? Are you just going to throw away credentials in already-signed extensions to replace them with what you're doing now?... I don't think people are going to be happy about that.
The existing signature system will be deprecated, yes. For discussion about this plan, please go to the mozilla.addons.user-experience newsgroup.
OK, so if I understand correctly, if Mozilla starts enforcing signatures as planned, then proper jar-style signatures will be overwritten, replaced with Mozilla signatures? How would I verify that what was made available to me was actually created by the author of the extension, and not just uploaded by someone else to be signed on the server? o.O
This discussion is unrelated to this bug. Like I said, please go to the newsgroup with those questions.
Right.

Well at least for this bug, if you're not going to use to-spec jar signing with file signatures actually in the .sf signature files, which what I started this bug for (and not necessarily the other half which are the crashes caused by something else), then can you please get a document up that exactly outlines your signing policy that you are going to use? 

If I want to be able to parse the Mozilla-signed extensions in the future, I need that information, because as they were signed now, the signing done is invalid for parsing it as signed JARs.
(In reply to Mark Straver from comment #16)
> (In reply to Dave Townsend [:mossop] from comment #15)
> > I don't know if Firefox has ever religiously followed the JAR signing spec.
> 
> I assumed it did, since there has never been any documentation otherwise --
> and initially the extension files have always been supplied as actual JAR
> files (and your way of signing does follow the spec otherwise).

No, Sun/Oracle Java ARchives use DSA signatures while Netscape/Mozilla has always used RSA signatures. They've never been compatible.

> Then this effectively prevents multiple vendors from signing different files
> in a single XPI -- you probably want to change that. Even if not religiously
> following the signing specification right now, those spec rules are there
> for a reason and you should consider following them ;)

We have always explicitly required installs to have a single signer. Having multiple signatures is an error. One of the needs for Java is to combine various signed libraries into a single package, because in Java it makes sense to use parts of an archive. Our requirement is that an "install" be signed by a single entity. We could allow for multiple signatures on the complete contents, but since we have no good UI for conveying that fact to users we have simply disallowed it until. We can reconsider if someone presents a compelling use case.

> The manifest should contain a list of all files in the XPI that are signed
> by *someone*, and the individual .sf files should list those files signed
> with the included, equally named, rsa file, one matching sf/rsa pair per
> vendor.

For install packages we require the manifest to list _all_ the files in the archive (minus the signature meta files), and they must all be signed by the same entity. Doing it this way reduces the processing overhead of checking N .sf hashes in addition to the N .mf hashes, and this has been helpful for validating app packages on limited power Firefox OS devices.

We could, in fact, simplify this further and have the .rsa file sign the .mf file directly and get rid of the .sf file middle-man.
(In reply to Daniel Veditz [:dveditz] from comment #34)
> No, Sun/Oracle Java ARchives use DSA signatures while Netscape/Mozilla has
> always used RSA signatures. They've never been compatible.

Sorry Dan, but I'm afraid you're incorrect. See link in comment #0

"Digital Signatures
A digital signature is a signed version of the .SF signature file. These are binary files not intended to be interpreted by humans.

Digital signature files have the same filenames as the .SF files but different extensions. The extension varies depending on the type of digital signature.

    .RSA (PKCS7 signature, SHA-256 + RSA)
    .DSA (PKCS7 signature, DSA)"

> We have always explicitly required installs to have a single signer. Having
> multiple signatures is an error. One of the needs for Java is to combine
> various signed libraries into a single package, because in Java it makes
> sense to use parts of an archive. Our requirement is that an "install" be
> signed by a single entity. We could allow for multiple signatures on the
> complete contents, but since we have no good UI for conveying that fact to
> users we have simply disallowed it until. We can reconsider if someone
> presents a compelling use case.

See my post posted in mozilla.addons.user-experience newsgroup -- since that kind of compelling argument was considered not relevant to this bug by Jorge (see comment 31, comment 32)

> .sf hashes in addition to the N .mf hashes, and this has been helpful for
> validating app packages on limited power Firefox OS devices.

So, sacrificing a good method of verifying signatures for the lowest common denominator... Is mobile/b2g even using this code? If so, wouldn't it be much better to not enforce super low HW spec considerations (at the cost of security) on everyone? 
In fact, can desktop extensions even be installed on Firefox OS to begin with?... Sorry if these are obvious questions but I don't own (or plan to own) a Firefox OS device, so I don't know what exactly is in use in b2g

> We could, in fact, simplify this further and have the .rsa file sign the .mf
> file directly and get rid of the .sf file middle-man.

I'm all for keeping it simple if you're not following jarsigning anyway. Whichever way you go, please at least provide documentation for other software developers to know how things are signed @Mozilla, exactly?
(In reply to Mark Straver from comment #35)
> Digital signature files have the same filenames as the .SF files but
> different extensions. The extension varies depending on the type of digital
> signature.

This isn't really a fruitful topic of discussion, so I'll just say that this has been different in different versions of the JAR spec. The initial version only mentioned DSA but said that other widely-recognized ciphers might be accepted based on their extension. Then RSA was mentioned, as RSA + MD5, now it's RSA + sha256.

> So, sacrificing a good method of verifying signatures for the lowest common
> denominator... Is mobile/b2g even using this code? If so, wouldn't it be
> much better to not enforce super low HW spec considerations (at the cost of
> security) on everyone? 
> In fact, can desktop extensions even be installed on Firefox OS to begin
> with?... Sorry if these are obvious questions but I don't own (or plan to
> own) a Firefox OS device, so I don't know what exactly is in use in b2g

That isn't really the point. Desktop extensions can't currently be installed on b2g devices, no. Many of them can be installed in Firefox for Android, which doesn't currently require add-ons to be signed, but may in the future. Either way, we know that the additional overhead can be problematic for many classes of users, and we'd rather avoid the performance cost.

And in point of fact, we're not sacrificing anything. XPIs have a single signer intentionally. Their signatures do not serve the same purposes as in Java Archives, where they're meant to verify the identity of individual objects for the purpose of code security. While Firefox did at one time support signed JARs in that way for the sake of the `enablePrivilege` API, that's no longer the case (and never applied to XPIs in any significant way). The former signature system was meant solely to identify the origin of the XPIs to the user doing the installation. The new system is meant solely to verify the origin of the entire XPI to the XPInstall system. Neither of these benefit in any way from multiple signatures (and both have always explicitly forbidden them), which means that duplicating the verification work is entirely unnecessary.
Closing now, it was fixed in https://github.com/mozilla/olympia/commit/e6253e3caa69acea2d5a83123e517f3950a8673a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
IMHO the signatures are still invalid since they don't adhere to any known spec, but I guess the crash issue with not signing for now has been solved (which wasn't my initial report here).

I guess for further questions on how this signing and verification is done, I have to reverse-engineer the source code, then? Don't get me wrong, I get it, it's currently "not relevant" for anyone but Mozilla to know the signing and verification procedure used if not to any specific standard, but if we're looking at "may in the future" statements anyway, and Firefox "will in the future" require signed add-ons, then having a dev doc would be useful/needed (third time I ask about this, 2 times ignored so far).

Also, for signing closed source/NDA-burdened extensions that are supposed to be used in a closed environment, apart from needing the as-of-yet unknown way to provide credentials to the verification (alternative to Mozilla built-in cert used for "public" add-ons on AMO and elsewhere), this signing procedure will have to be known as well (since you're not using jar signing the java way...) to satisfy Firefox's XPInstall verification when signing in-house.
(In reply to Mark Straver from comment #35)
> Digital signature files have the same filenames as the .SF files but
> different extensions. The extension varies depending on the type of digital
> signature.
> 
>     .RSA (PKCS7 signature, SHA-256 + RSA)
>     .DSA (PKCS7 signature, DSA)"

I guess you're correct now, but at the time Netscape started using this variation of the format in the 1990s that wasn't true. They started out incompatible and we've never made any attempt to make them compatible since. JAR archives (.jar) are not Xross-Platform Install packages (.xpi), and the signing has different requirements for different purposes. 

> Is mobile/b2g even using this code? [...] In fact, can desktop extensions even be
> installed on Firefox OS to begin with?

Packaged Firefox OS apps are signed using this same scheme
You need to log in before you can comment on or make changes to this bug.