The tests here: http://www.mozilla.org/projects/xpinstall/signed/testcases/index.html fail. Meaning that an original signed extension appears signed, and so does the altered version. Another test - download a signed extension from here: http://www.j-maxx.net/abtrans/abextension.php, modify a file inside the extension keeping the cert. (metadata) intact. The extension still appears signed. IMHO this is a serious security bug - a user can be installing a malware believing it is signed by an entity he/she trusts.
I tested this a bit further, and it seems that this is an extension manager bug with the latest FF nightly. If you press "Install" it actually says "Install success". After you restart FF, the extension appears in EM, but without its full name - only the filename that was downloaded. The extension seems -not- to be installed - it just sits there. So, it now looks more like an EM bug than a security one.
firefox 1.0 does the right thing (almost). A dialog box appears saying it couldn't _download_ the file (wrong, it downloaded) because "Signing not verified" (correct). While the alert is showing the extension appears in the EM dialog behind as "Install Success" (wrong), but disappears from the EM dialog when you click OK on the error alert (correct in the end). Over to the Firefox product, it's the front end that's wrong.
Clearing the security flag so more people who could fix the UI might see it.
13 years ago
When installing the extension from http://www.j-maxx.net/abtrans/abextension.php it displayed as signed and installed successfully when unmodified. I then downloaded it, modified the install.rdf inside the xpi, and tried to install it. The ui showed as signed and the extension failed to install with an alert stating: Deer Park could not download the file at <path to file>/albhed-0.1.2.xpi because: Signing could not be verified. This was reported to the js console: Signature Verification Error: the signature on install.rdf is invalid because the MANIFEST.MF file does not contain a valid hash of the file being verified. The extension was also removed from the ui after it failed to install. This is WFM but I am going to hold off on resolving since this may not be entirely the desired behavior though I believe it is close enough. Also, I suspect that the patch from bug 286034 may have fixed this bug.
This is basically correct behavior. The only remaining problem is unrelated to signing: when there's an installation error the message starts out "Firefox could not download the file at" when it's quite often not a download error. Would be better to say "Firefox could not install the file", and for the few times it is a download problem finish with "because: download error". (Right now it will say it could not download the file because of download error, which is annoying.) The message issue is separate and probably already filed, although I couldn't find one. The string is http://lxr.mozilla.org/mozilla/source/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties#64 The code is http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/content/extensions.js#361 The code doesn't handle status 999 correctly -- that's not an install failure, that's a success with restart required to finish replacing in-use files.
Thanks Daniel - I have a couple of other patches in progress including one that handles the status 999 and will fix this after the freeze is lifted.
Created attachment 184718 [details] [diff] [review] patch I split this out from another patch. Per bsmedberg I changed errorInstallMessage to errorInstallMsg to make it easier for translators to see there is a change. I test for 999 instead of just going with all errors < 0 since there may be other errors above 0 that should be displayed though I suspect this is unlikely.
I doubt that a safer patch could be found to fix the remaining issues so nominating for 1.8b2
Comment on attachment 184718 [details] [diff] [review] patch Benjamin - this is a very simple / safe patch that fixes the remaining issues as pointed out by dveditz and with blockers that were just added for 1.8b2 I think that it may be appropriate to land this.
Comment on attachment 184718 [details] [diff] [review] patch a=chofmann, lets get this in quick
Fixed on trunk for 1.1a1.
13 years ago
Comment on attachment 184718 [details] [diff] [review] patch I wish we had constants for the '999' that meant something, or perhaps even a comment, but a=shaver nonetheless.
*** Bug 296954 has been marked as a duplicate of this bug. ***