search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

Language packs fail to install on -dev #6372

Closed AlexandraMoga closed 5 years ago

AlexandraMoga commented 5 years ago

Possible regression from mozilla/addons#5936 Prerequisites:

STR:

  1. Open the following language pack - https://addons-dev.allizom.org/en-US/firefox/addon/finnish-language/
  2. Try to install it

Actual result: Language pack doesn't install

Expected result: Language pack can be installed

Notes:

Console error: image

EnTeQuAk commented 5 years ago

That indeed shouldn't happen, I'll check with autograph folks to see what's wrong.

Did you try for example with a very minimal webextension (https://staticfil.es/webext-generator/)? Afaik this worked for me some time ago, checking again.

g-k commented 5 years ago

Huh yeah I'm seeing the same console error and "this addon appears to be corrupt" on Nightly 67.0a1 2019-02-04. The COSE signature parses as valid CBOR, but I forget what format it's supposed to be in. I'll dig into this.

EnTeQuAk commented 5 years ago

Thanks a lot, @g-k, let me know if I can be of any help

g-k commented 5 years ago

OK since this was signed with autograph -dev I don't think it'll work with the Firefox dev-root, because the dev-root key is in stage (to be extra confusing). I'll check that:

  1. the autograph web ext dev signer is in the Fx dev-root chain of trust
  2. and if it isn't, try resigning the language pack XPI with the stage autograph config and see if it loads without errors

edit: autograph stage and dev are using the same cert and key so that's not the issue

AlexandraMoga commented 5 years ago

Did you try for example with a very minimal webextension (https://staticfil.es/webext-generator/)? Afaik this worked for me some time ago, checking again.

@EnTeQuAk I've uploaded and installed all types of addons, but I ran into this error only with language packs.

g-k commented 5 years ago

I looked at the signatures in cbor.me and lapo.it/asn1js but nothing stood out.

I deleted 'META-INF/*' from the finnish lang pack to get an unsigned version of the finnish langpack and resigned it with autograph's golang client, which validates the crypto parts of the signatures in the signed file (specifically the PKCS7 sig and chain of trust against the dev root and the COSE signatures).

The web extension signer that AMO uses still produced a corrupt langpack.

Since the CN is langpack-fi@firefox.mozilla.com, I tried resigning with the mozilla extension signer to get a different OU (but langpacks are supposed to be normal web extensions as far as I can tell). That produced a corrupt langpack too. Varying the CN to a non-mozilla.com domain didn't work either.

I ran into this error only with language packs.

This is interesting (a regular XPI worked for me too).

I'd like to confirm that Fx supports langpacks w/ COSE sigs and asked in https://bugzilla.mozilla.org/show_bug.cgi?id=1515173#c10

g-k commented 5 years ago

Yes, signatures on langpacks are validated in the same way as signatures on extensions, themes, etc.

:aswan in https://bugzilla.mozilla.org/show_bug.cgi?id=1515173#c12

so this looks like an autograph issue after all.

g-k commented 5 years ago

ulfr looked at this and pointed out that the test langpack uses firefox.mozilla.org instead of .com: https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/tests/browser_browser_languages_subdialog.js#27

Does using a CN with a .com TLD matter?

EnTeQuAk commented 5 years ago

So, I uploaded another langpack that has the original @firefox.mozilla.org CN, https://addons-dev.allizom.org/en-US/firefox/addon/deutsch-de-lpack-cgrebs/ - and tested that. I didn't use an unbranded build though, only set the relevant variables.

1549519871959   addons.xpi-utils    WARN    Add-on langpack-de-test@firefox.mozilla.org is not correctly signed.
1549519871960   addons.xpi-utils    WARN    Preference xpinstall.signatures.dev-root is set.
1549519871960   addons.xpi-utils    WARN    Add-on langpack-de-test@firefox.mozilla.org is not correctly signed.
1549519871960   addons.xpi-utils    WARN    Preference xpinstall.signatures.dev-root is set.
1549519871962   addons.xpi  WARN    Download of https://addons-dev-cdn.allizom.org/user-media/addons/502957/deutsch_de_language_pack-66.0buildid20190204181317-fx.xpi?filehash=sha256%3A81f208dd68eb49ddacff5e64e20b2d90752a28ce18a7c0fd5e9541dac0ca43a9 failed: signature verification failed

So it's unrelated to the CN? lapo.it looks fine though.

Edit Also tried with firefox devedition (which should work according to https://dxr.mozilla.org/mozilla-central/source/browser/config/mozconfigs/linux64/devedition#6 unless I'm mistaken?) - and got the same errors.

AlexandraMoga commented 5 years ago

For extra thoroughness, I've downloaded an unbranded version of the latest beta - 66.0b5 - and I can confirm that the error is still present when trying to install https://addons-dev.allizom.org/en-US/firefox/addon/deutsch-de-lpack-cgrebs/

g-k commented 5 years ago

Huh, yeah I figure the PKCS7 signatures are fine since we issue those and they install without errors.

NI'd in :keeler in https://bugzilla.mozilla.org/show_bug.cgi?id=1515173#c13 since she might have a better idea what Fx is doing.

g-k commented 5 years ago

OK :keeler let us know we're failing on the line length. Somewhat frustrating since I had a reminder to check that in https://github.com/mozilla-services/autograph/issues/147:

Also, check whether Fx XPIs expect that and line length to be enforced (per #145 (comment)).

Anyway, I have a PR with a fix in https://github.com/mozilla-services/autograph/pull/241

Testing it locally against the stage config (same webexts key as dev). I signed with:

go run client.go -r test.addons.signing.root.ca.crt -f ~/Downloads/finnish_language_pack-64.0buildid20190108160530-fx-unsigned.xpi -u amo -p $AMO_STAGE -cn 'langpack-fi@firefox.mozilla.com' -pk7digest sha1 -k webextensions_rsa -c ES256 -o finnish_lang_pack-fx-pkcs7-sha1-es256.xpi

and the signed finnish langpack installs in a nightly profile from about:addons load from file:

screenshot_2019-02-07_15-24-38

\o/

g-k commented 5 years ago

Deployed the fix (in 2.8.2) to -dev, so this should be ready to test again.

$ curl http://localhost/__version__
{"commit":"567ec4d92dc70d094b0ae0e8cec91959514236e1","version":"2.8.2","source":"https://github.com/mozilla-services/autograph","build":"https://circleci.com/gh/mozilla-services/autograph/1926"}
EnTeQuAk commented 5 years ago

Awesome, thanks a lot @g-k! I'll test this in a bit, I think @AlexandraMoga will do so as well to verify.

AlexandraMoga commented 5 years ago

@EnTeQuAk @g-k I've submitted a few new language packs on -dev and now they are successfully installed in FF65, Beta 66 and Nightly 67.

ex.1 - https://addons-dev.allizom.org/en-US/firefox/addon/ukrainian-ua-language-pack/ ex. 2 - https://addons-dev.allizom.org/en-US/firefox/addon/korean-kr-languagepack/

The examples that were not working before are still failing to install. Is that expected?

EnTeQuAk commented 5 years ago

The examples that were not working before are still failing to install. Is that expected?

We haven't resigned them yet, correct. I think it's only addons-dev.allizom.org/en-US/firefox/addon/deutsch-de-lpack-cgrebs so I'll do that in a bit.

EnTeQuAk commented 5 years ago

I resigned the add-on that I uploaded for completeness.

I'm going to close this as fixed given the recent fixes and verifications.

AlexandraMoga commented 5 years ago

@EnTeQuAk I'm still receiving the signature error with your resigned lang pack Would you mind double checking this on your end? I just want to make sure that what's causing this issue doesn't have an implication for other add-ons that are going to be resigned.

EnTeQuAk commented 5 years ago

@EnTeQuAk I'm still receiving the signature error with your resigned lang pack Would you mind double checking this on your end? I just want to make sure that what's causing this issue doesn't have an implication for other add-ons that are going to be resigned.

Eh, yeah… next time I know that our resign script only signs actual extensions, sigh. I'll submit an update for the resign script and will test it on that add-on in particular.