Opened 2 years ago

Last modified 10 months ago

#22971 new defect

The XPI signing mechanism needs to use different hash functions.

Reported by: yawning Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-security, ff60-esr
Cc: intrigeri Actual Points:
Parent ID: #26553 Points:
Reviewer: Sponsor:

Description

https://wiki.mozilla.org/Add-ons/Extension_Signing

Signing 2 hashes of a manifest file containing 2 hashes each of every file in an archive, especially when "2 hashes" is MD5 and SHA1 is cryptographically unsound.

See Joux, A., "Multicollisions in Iterated Hash Functions. Application to Cascaded Constructions".

Child Tickets

Change History (10)

comment:2 Changed 2 years ago by yawning

Keywords: tbb-security added
Priority: MediumHigh
Severity: NormalMajor

This is probably more an upstream issue since the practical result is "Extension Signing is worthless vs adversaries that can produce SHA1 collisions".

comment:3 in reply to:  2 Changed 2 years ago by isis

Replying to yawning:

This is probably more an upstream issue since the practical result is "Extension Signing is worthless vs adversaries that can produce SHA1 collisions".


Ugh. And yeah, this seems to be an upstream issue, we should see if they've already got a fix they're working on.

comment:4 Changed 2 years ago by yawning

Upstream bug has been around for years apparently: https://bugzilla.mozilla.org/show_bug.cgi?id=1169532

Fun facts:

  • The MD5 digest is ignored (sigh).
  • The PKCS7 RSA signature *also* uses SHA1 (I should have checked this).
  • Their plan apparently is to move to *also* include SHA256 digests and transition to ECDSA.

I'm uncertain if we should treat this more severely. I'm not exactly thrilled about "keeping the same old busted manifest format, adding yet another M-D construct hash, and doing absolutely shit fuckall to mitigate length extension attacks" as the upstream response.

At a minimum, I think we can do better by patching the XPI verification code at least for our addons (like we do for the MAR signatures), but what do I know.

comment:5 Changed 2 years ago by yawning

If I were to try to mitigate this without breaking things for lots of users, I would replace the SHA1 implementation used for XPI verification with the hardened variant that came out of the shattered.io research.

https://github.com/cr-marcstevens/sha1collisiondetection

comment:6 in reply to:  5 Changed 2 years ago by gk

Replying to yawning:

If I were to try to mitigate this without breaking things for lots of users, I would replace the SHA1 implementation used for XPI verification with the hardened variant that came out of the shattered.io research.

https://github.com/cr-marcstevens/sha1collisiondetection

Sounds like a useful thing to look at, thanks.

comment:7 Changed 2 years ago by gk

Keywords: ff59-esr added

We need to sign our extensions ourselves anyway when switching to ESR59. If we get to it earlier then it can't hurt, though.

comment:8 Changed 23 months ago by gk

Keywords: ff60-esr added; ff59-esr removed

Firefox 60 is the new ESR.

comment:9 Changed 18 months ago by gk

Parent ID: #26553

comment:10 Changed 10 months ago by intrigeri

Cc: intrigeri added
Note: See TracTickets for help on using tickets.