419906 - Upgrade install hashes to use SHA-256 instead of SHA-1

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[Laura Thomson :laura]

Right now the file hashes for addons are SHA-1.  The security review for the Addons Manager suggested we should move to SHA-256 to be consistent with internal standards.

There are two things that need to be done:
1.  Check length of hash column in remora and modify remora.sql if it's not long enough.  SHA-256 hashes are 64 chars long plus any manifests.

2.  There is a script in bin that was previously used to update all the file hashes.  Use this as a starting point, modify it, and run it on the prod db.

Comment 1

[Reed Loden [:reed]]

Attachment: patch - v1

remora.sql has:

`hash` varchar(255) default NULL,

Looks large enough to me.

I took this opportunity to modify update-hashes.php to use database.class.php, too...

Comment 2

[Laura Thomson :laura]

Comment on attachment 306086 [details] [diff] [review]
patch - v1

Before I grant review: is 'sha256' the string the client is looking for?

Comment 3

[Justin Scott [:fligtar]]

3. Update Developer Tools additem script to correctly calculate future hashes

4. Update Admin Tools hash recalculator to correctly calculate hashes

Comment 4

[Reed Loden [:reed]]

Attachment: patch - v2

Go ahead and change everything.

Note that site/app/webroot/services/install.php will need to be updated when the hashes change. Should also probably modify the test data to use sha256 hashes, too.

Comment 5

[Michael Morgan [:morgamic]]

Seems like a better approach would be to store the hashtype and hashvalue in the db instead of hard-coding it everywhere.

Comment 6

[Reed Loden [:reed]]

(In reply to comment #2)
> (From update of attachment 306086 [details] [diff] [review])
> Before I grant review: is 'sha256' the string the client is looking for?

<reed> Mossop: https://bugzilla.mozilla.org/show_bug.cgi?id=419906#c2 <-- can you answer that? from looking at the code, I'm pretty sure that's right, but want to double check
<Mossop> reed: Yes, looks right

Comment 7

[Michael Morgan [:morgamic]]

Comment on attachment 306090 [details] [diff] [review]
patch - v2

see comment #5

Comment 8

[Michael Morgan [:morgamic]]

alternatively, we could use the length of the value as well and add something to constants.php to map those to the hashFunction.

Comment 9

[Carsten Book [:Tomcat]]

-> Cleaning up this Component because we Retire the "add-ons" Component in
Bugzilla 
--> Over to Administration

Comment 10

[Reed Loden [:reed]]

(In reply to comment #5)
> Seems like a better approach would be to store the hashtype and hashvalue in
> the db instead of hard-coding it everywhere.

We _do_ store the hashtype and hashvalue of each file in the database... It's done under the form "hashtype:hashvalue". My patch changes the default hash type from SHA-1 to SHA-256 and then updates update_hashes.php so that it can be run against the current database to update all the current hashes.

What exactly are you wanting? Some way to change the hash used on the fly via some admin panel? That seems unnecessary and pretty useless.

Comment 11

[Michael Morgan [:morgamic]]

Comment on attachment 306090 [details] [diff] [review]
patch - v2

Wil and Laura - could you take a look at this?  Think it's ready for primetime?

Comment 12

[Wil Clouser [:clouserw]]

Comment on attachment 306090 [details] [diff] [review]
patch - v2

worked well for me

Comment 13

[Reed Loden [:reed]]

Sending        bin/update-hashes.php
Sending        site/app/controllers/admin_controller.php
Sending        site/app/controllers/components/developers.php
Transmitting file data ...
Committed r21480.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

I'm waiting for bug 472784 to be fixed before I can test uploading an add-on on preview.

Comment 15

[Daniel Veditz [:dveditz]]

So does this only affect new and updated addons? Or is it just a tool and
someone needs to run it? When I check current versions I'm still seeing sha1
hashes from versioncheck.

e.g.:
https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=1&id=[e-mail]&version=2.0.1&maxAppVersion=3.2a1pre&status=userEnabled&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=3.0.6pre&appOS=Darwin&appABI=x86-gcc3&locale=en-US


https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=1&id=[e-mail]&version=0.5.2&maxAppVersion=3.2a1pre&status=userEnabled&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=3.0.6pre&appOS=Darwin&appABI=x86-gcc3&locale=en-US

Comment 16

[Reed Loden [:reed]]

(In reply to comment #15)
> So does this only affect new and updated addons? Or is it just a tool and
> someone needs to run it? When I check current versions I'm still seeing sha1
> hashes from versioncheck.

Note the "push-needed" keyword. This isn't live yet, and I don't have any ETA on when that might be. You can test using preview.addons.mozilla.org for now.

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED for the following couple cases:

[1] NEW add-on: uploaded a test add-on (https://preview.addons.mozilla.org/en-US/firefox/addon/9600), which had:

addonHash="sha256:59cf3deb3d1ce9b721b8d5a2434a0edc8c18c4ac6871ba6476c8df0f0ee587ce"

[2] EXISTING add-on: page source of https://preview.addons.mozilla.org/en-US/firefox/addon/421, as an example is using sha256 as well.