1126898 - Add support for separate "preliminary" signing endpoint URL

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect)

Description

[Ryan Tilder [:rtilder]]

Per our discussion on IRC, something akin to Zamboni's handling of reviewer signing for FirefoxOS privileged apps found here: https://github.com/mozilla/zamboni/blob/master/lib/crypto/packaged.py#L101-104

Comment 1

[Mathieu Agopian [:magopian]]

Jason, is it clear for you what is needed? From what I understand, it needs another instance of trunion running with different settings. Once it's in place, could you please update this bug with the endpoint to use?

I believe all the necessary information are in bug 1123915

Thanks!

Comment 2

[Mathieu Agopian [:magopian]]

Actually, in bug 1126894

Comment 3

[Jason Thomas [:jason]]

I've added PRELIMINARY_SIGNING_SERVER to olympia's private settings file. https://github.com/mozilla-services/svcops-puppet/commit/12a6f90029ecc8778b4e0c875d9b8018f3a9a39c

Comment 4

[Mathieu Agopian [:magopian]]

PR: https://github.com/mozilla/olympia/pull/438

Ryan, is there a way, given a signed addon, to see if it's been fully or preliminary signed? What are the steps to make sure the correct endpoint (with the correct settings) has been used?

Comment 5

[Mathieu Agopian [:magopian]]

Fixed in https://github.com/mozilla/olympia/commit/da3f26487557af5719a5c3916939a820ee867d32

Comment 6

[Madalin Cotetiu]

Please add STR here or mark it with [qa-] if no QA is needed.

Comment 7

[Mathieu Agopian [:magopian]]

:rtilder, :dveditz, :jason, is there a way, given a signed addon, to manually check if it's been signed with the correct endpoint?

Comment 8

[Jason Thomas [:jason]]

I usually test by extracting the addon xpi and running the following openssl command:

openssl pkcs7 -inform der -in META-INF/zigbert.rsa -print_certs -text -noout

OU should be equal to 'Preliminary'.

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            01:4b:c7:e3:db:4a
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C=US, ST=CA, L=Mountain View, O=Addons Test Signing, CN=test.addons.signing.root.ca/emailAddress=[e-mail]
        Validity
            Not Before: Feb 26 21:56:13 2015 GMT
            Not After : Feb 23 21:56:13 2025 GMT
        Subject: OU=Preliminary, C=US, L=Mountain View, O=Addons Testing, ST=CA, CN=someaddonuid
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption

Comment 9

[Mathieu Agopian [:magopian]]

Ah, excellent, thanks Jason!

So the STR are:
1/ submit an addon and choose the prelim review
2/ download the (signed) addon from the listing page
3/ run the above command, and make sure the OU says "Preliminary"
4/ submit another addon and chose the full review
5/ download the (signed) addon from the listing page
6/ run the above command, and make sure the OU says... "Full"? Not sure about the text here, but it shouldn't be "Preliminary"

Comment 10

[Madalin Cotetiu]

Attachment: fullReview.txt

I have followed the steps above and for both full review and preliminary review the OU is "Preliminary" 
Attaching the logs files for both full and preliminary reviews.

Comment 11

[Madalin Cotetiu]

Attachment: preliminaryReview.txt

Reopening the bug.

Comment 12

[Mathieu Agopian [:magopian]]

Thanks Madalin. Jason? Is there a way we can double check that? How can I help?

Comment 13

[Jason Thomas [:jason]]

Logs shows addon in comment 10 (id=490498) was sent to preliminary server. Both servers are configured correctly in the settings.

ar  5 14:24:46 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.devhub:INFO FileUpload created: 33a08667fc5c4757beaf317e32206a9e :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/devhub/views.py:607
Mar  5 14:28:28 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:DEBUG clean_name called without an instance: testPass3.5.2015 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/forms.py:42
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:INFO Version changed from backup: None to None, current: None to None, latest: None to .1 for addon 490498: testPass3.5.2015 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/models.py:716
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.amo:INFO Cache increment failed for key: ns:d2c-versions:490498. Resetting. :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/amo/utils.py:673
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:INFO Incrementing d2c-versions namespace for add-on [490498]: 1425565710:ns:d2c-versions:490498 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/models.py:854
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.versions:INFO New version: <Version: .1> (1526568) from <FileUpload: 33a08667fc5c4757beaf317e32206a9e> :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/versions/models.py:128
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:INFO Version changed from backup: None to None, current: None to .1, latest: .1 to .1 for addon 490498: testPass3.5.2015 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/models.py:716
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.files:INFO Hash changed for file: 246970, addon: 490498, from:  to: sha256:99f1ff8652fb1b7b115a94c75f0fa0d7abc3c0e0e8e888e7fdd09c044ec15418 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/files/models.py:469
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.files:DEBUG New file: <File: 246970> from <FileUpload: 33a08667fc5c4757beaf317e32206a9e> :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/files/models.py:172
Mar  5 14:28:30 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:DEBUG New addon <Addon: 490498: testPass3.5.2015> from <FileUpload: 33a08667fc5c4757beaf317e32206a9e> :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/models.py:500
Mar  5 14:29:44 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.files:INFO Moving file to mirror: /mnt/netapp_amo_dev/addons-dev.allizom.org/files/490498/testpass352015-.1-fx.xpi => /mnt/netapp_amo_dev/addons-dev.allizom.org/files/490498/testpass352015-.1-fx.xpi :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/files/models.py:338
Mar  5 14:29:45 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.files:INFO Moving file to mirror: /mnt/netapp_amo_dev/addons-dev.allizom.org/files/490498/testpass352015-.1-fx.xpi => /mnt/netapp_amo_dev/addons-dev.allizom.org/files/490498/testpass352015-.1-fx.xpi :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/files/models.py:338
Mar  5 14:29:45 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.addons:INFO Incrementing d2c-versions namespace for add-on [490498]: 1425565711:ns:d2c-versions:490498 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/addons/models.py:854
Mar  5 14:29:45 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.crypto:INFO Signing version: 1526568 :./lib/crypto/packaged.py:120
Mar  5 14:29:45 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.crypto:INFO File signature contents: Signature-Version: 1.0#012MD5-Digest-Manifest: BlommQe74LyMBZrPXgUyaA==#012SHA1-Digest-Manifest: IQuXOG8WeBEurfcRZjTFlVR4F4Q=#012 :./lib/crypto/packaged.py:58
Mar  5 14:29:45 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.crypto:INFO Calling signing service: http://prelim-signer.addons.allizom.org/1.0/sign_addon :./lib/crypto/packaged.py:62
Mar  5 14:29:46 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.crypto:INFO Signing complete for file 246970. :./lib/crypto/packaged.py:110
Mar  5 14:29:46 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.mailer:INFO Making 490498: testPass3.5.2015 public :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/editors/helpers.py:667
Mar  5 14:29:46 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.mailer:INFO Sending email for 490498: testPass3.5.2015 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/editors/helpers.py:668
Mar  5 14:29:46 dev1.addons.phx1.mozilla.com: [madalinc][62.231.92.162] z.users:INFO Awarding 120 points to user 10620563: madalinc for "Full Add-on Review" for addon 490498 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305074456-1dce168012/olympia/apps/editors/models.py:405

The addon.status = 4 for the addon so it should have been sent to the 'final' server.

I manually signed the addon with sign_addon management command and it sent it to the correct server:

Mar  5 18:50:34 addonsadm.private.phx1.mozilla.com: [<anon>][None] z.crypto:INFO File signature contents: Signature-Version: 1.0#012MD5-Digest-Manifest: BlommQe74LyMBZrPXgUyaA==#012SHA1-Digest-Manifest: IQuXOG8WeBEurfcRZjTFlVR4F4Q=#012 :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305174131-8d37083345/olympia/lib/crypto/packaged.py:58
Mar  5 18:50:34 addonsadm.private.phx1.mozilla.com: [<anon>][None] z.crypto:INFO Calling signing service: http://signer.addons.allizom.org/1.0/sign_addon :/data/addons-dev/www/addons-dev.allizom.org/deploy-olympia-dev-20150305174131-8d37083345/olympia/lib/crypto/packaged.py:62

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 1425581439580 (0x14beb467a5c)
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: C=US, ST=CA, L=Mountain View, O=Addons Test Signing, CN=test.addons.signing.root.ca/emailAddress=[e-mail]
        Validity
            Not Before: Mar  5 18:50:39 2015 GMT
            Not After : Mar  2 18:50:39 2025 GMT
        Subject: OU=Testing, C=US, L=Mountain View, O=Addons Testing, ST=CA, CN=gr14hyte-ZmmhUZdl7odf9zGfgG6AG0lsTvg@jetpack
        Subject Public Key Info:

Comment 14

[Jason Thomas [:jason]]

Possibly related to mysql replication lag? Although lag should be very minimal especially on the -dev server.

Comment 15

[Mathieu Agopian [:magopian]]

Should be fixed by https://github.com/mozilla/olympia/pull/503/files#diff-1ff978159be740b1b8edc4fcc67c4faaR27, needs to be tested on -dev or stage

Comment 16

[Jason Thomas [:jason]]

Do we need to QA this again?

Comment 17

[Madalin Cotetiu]

Tested this again on stage

For preliminary review:
 Validity
        	Not Before: Apr 22 14:00:08 2015 GMT
        	Not After : Apr 19 14:00:08 2025 GMT
    	Subject: OU=Preliminary, C=US, L=Mountain View, O=Addons Testing, ST=CA, CN=gr14hyte-ZmmhUZdl7odd9gGdgT8AG1lsTvg@jetpack


For full review:
 Validity
        	Not Before: Apr 22 13:51:40 2015 GMT
        	Not After : Apr 19 13:51:40 2025 GMT
    	Subject: OU=Testing, C=US, L=Mountain View, O=Addons Testing, ST=CA, CN=gr14hyte-ZmmhUZdl7odf9gGdgT8AG8lsTvg@jetpack

I do not think this is expected. :magopian?

Comment 18

[Mathieu Agopian [:magopian]]

From what I can tell, this is exactly what's expected:
- the preliminary reviewed addon has "preliminary" in the OU
- the fully reviewed addon doesn't have "preliminary" in the OU

Comment 19

[Madalin Cotetiu]

Ok so for fully reviewed add-ons the text should be testing. Thanks for response.
Closing bug.

Comment 20

[Mathieu Agopian [:magopian]]

I think the content is not important, it's just the presence (or absence) of "preliminary" that is.

Maybe :dveditz can confirm?

Comment 21

[Daniel Veditz [:dveditz]]

We have two separate roots, a testing root and a prod root. I would hope that the prod root doesn't use OU=Testing for the non-preliminary one (and not O=Addons Testing). The text doesn't really matter though as long as it's not a case-insensitive match for "Preliminary", it would just look strange/bad.

Comment 22

[Jason Thomas [:jason]]

(In reply to Daniel Veditz [:dveditz] from comment #21)
> I would hope that the prod root doesn't use OU=Testing for the non-preliminary one (and not O=Addons Testing).

It doesn't, Bug 1130020 comment 13 has an example.