1169532 - extension XPI signing still uses SHA1 for digests; should use SHA2

Status: VERIFIED FIXED

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

Description

[Dave Garrett]

The new AMO addon mass signing frenzy is upon us, but the signatures aren't exactly bleeding edge.

Signed extension manifest.rf files contain MD5 & SHA1 digests. Finding an MD5 collision now takes half a day via cloud computing, at the cost of less than a dollar. SHA1 is considered weak and is being phased out by everyone in favor of SHA2. The combination of the two together, if both checked, can be greater than the sum of their parts, but they're still old deprecated parts.

I did a little digging, and near as I can tell, there is currently no support for better hash algorithms here. I'm also not entirely sure the MD5 hash is checked at all, but there's multiple different manifest parsers in the codebase and I'm not sure which gets used.

Support needs to be added for SHA2 at some point and SHA3 might be desired in the not too distant future as well. SHA1 can be kept for backwards compatibility for now, but MD5 needs to be nuked from orbit as soon as better options are available.

While we're on the subject of obsolete crypto, transitioning from RSA to some form of ECC signature would significantly reduce the key size and possibly improve performance.

See also the application update signing equivalent of this: bug 1105689

Comment 1

[Dave Garrett]

I don't think this can be under AMO, as Firefox doesn't seem to have the ability to parse SHA2 digests yet. That would be needed prior to AMO taking any action.

Comment 2

[Kris Maglione [:kmag]]

If that's the case, we'd need two bugs. I'll leave it up to Dan to decide where this belongs, and if it's something we need to fix. If Firefox currently does not support it, and we do want to change it, that means getting support in before mandatory signing hits beta.

Comment 3

[Daniel Veditz [:dveditz]]

If the signing tools are putting MD5 hashes in these files they can stop wasting the space: only the SHA1 hashes are used, MD5 is ignored.

Assuming this bug is about the AMO signing process adding SHA2 hashes (which it could do prior to Firefox support for it, as long as it didn't remove the SHA1 hashes) then we need a separate bug for Gecko app signature support for SHA2 (if there isn't one already).

Comment 4

[Kumar McMillan [:kumar]]

The MD5 digest is added to signed add-ons here: https://github.com/rtilder/websigning/blob/master/websigning/sign/xpi.py#L89-L90

It's likely just there to support the JAR signing spec (although the spec doesn't actually require an MD5 digest). It can probably be removed. Adding a SHA2 digest seems easy enough :)

Comment 5

[Julien Vehent]

There are two layers:

1. the hashes of all files in the XPI stored in mozilla.mf, itself hashes in mozilla.sf
2. the PKCS7 signature of the XPI stored in mozilla.rsa

Both layers currently use SHA1.

(1) requires modifying the AMO backend code Kumar mentioned in comment 4, and modifying the verifier code in Firefox. Can we add sha256 to the websigning library today without breaking verification in Firefox?

For (2), I'm currently reimplementing the PKCS7 signer and adding support for all SHA2 functions and ECDSA. https://github.com/mozilla-services/autograph/pull/46
Dan: do you know if the pkcs7 verifier in Firefox needs to be changed to support SHA2 and ECDSA?

Comment 6

[Daniel Veditz [:dveditz]]

AppSignatureVerification.cpp uses mozilla::pkix to verify the cert and NSS_CMSSignerInfo_Verify to verify the signature. It should handle whatever NSS handles for TLS.

Bug 1180826 covers making Firefox handle better hashes in mozilla.mf, but the signature-handling part should be OK.

Comment 7

[Jorge Villalobos [:jorgev] (he/him)]

Julien, is there any work that needs to happen on AMO or the signing service, outside of what's covered in https://github.com/mozilla-services/autograph/pull/46 and related issues?

Comment 8

[Julien Vehent]

Four things need to happen:

1. The generation of manifest.mf and mozilla.sf must include a SHA256-Digest. Currently only MD5 and SHA1 are supported. This needs to be done in the python library used to generate those files: https://github.com/mozilla/signing-clients/issues/20 . This library is used by addons-server, as well as various scripts used to sign other types of addons.

2. Firefox must verify SHA256 digests instead of using the SHA1 digest. This is discussed in bug 1180826.

3. The PKCS7 signature of the mozilla.sf file, stored in mozilla.rsa, currently uses SHA1 because that's the only PKCS7 digest Firefox seems to support. We need to add support for SHA256. This is discussed in bug 1357815.

4. When #3 is implemented, we can re-enabled hashing with SHA256 in the PKCS7 library of the autograph backend. I had previously reverted this feature and left a comment in https://github.com/mozilla-services/pkcs7/blob/master/sign.go#L118-L121


I own task #4, but I'm not sure who can, or should be, responsible for the three others.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

ni a couple of people who might help us figure out 1, 2, and 3 from comment #8. (tried to ni pauljt also, but it bounced)

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Jorge Villalobos [:jorgev] from comment #9)
> ni a couple of people who might help us figure out 1, 2, and 3 from comment
> #8. (tried to ni pauljt also, but it bounced)

Im back from PTO now(well, tomorrow technically). For Firefox client crypto, that's someone in JC's team but I see he is away, so NI'ing Wennie, who should be able to help prioritize this work.

Comment 11

[Wennie]

Hi David, can you take a look at this bug. Thanks!

Comment 12

[Stuart Colville [:muffinresearch]]

Part 1 from https://bugzilla.mozilla.org/show_bug.cgi?id=1169532#c8 has been assigned to :cgrebs and the amo team now has access to the signing-clients repo.

Comment 13

[Dana Keeler (she/her) [:keeler]]

I can drive parts 2 and 3 from comment 8 forward.

Comment 15

[Daniel Veditz [:dveditz]]

Firefox doesn't and hasen't used the MD5 digests in years, maybe even a decade. The server-side signing tool continued to write out both into the manifest files for a long time but it was ignored by the client.

Comment 16

[Liz Henry (:lizzard)]

Is this work still ongoing or was it resolved in the other bugs related to bug 1403838?

Comment 17

[Dana Keeler (she/her) [:keeler]]

As of bug 1357815 and bug 1403840, Firefox can verify signatures that use SHA-256. The add-on signing and hosting infrastructure still needs to be updated, however (note that we can't start hosting add-ons signed with the new signatures until usage of pre-58/59 Firefox is low enough).

Comment 18

[Dave Garrett]

As of this month, addons uploaded to AMO are now being signed with SHA2-256. I uploaded a file today and it's now public and installs/updates with a signature using SHA2 (in addition to the old SHA1 & MD5). Closing.

Comment 19

[Vlad Jiman]

Attachment: 2018-09-13_17h22_35.png

Verified using latest Nightly (09/13/2018) running on Windows 10 x64 bit and Ubuntu, extensions are being signed with SHA256, tested using both new extensions as well as extensions that have been uploaded before.