1304117 - Unlisted add-ons susceptible to MITM attack over HTTPS when updateKey not used

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Joshua Yabut]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36

Steps to reproduce:

Hello. I am the author of the AMO extension auto-update attack (https://hackernoon.com/tor-browser-exposed-anti-privacy-implantation-at-mass-scale-bd68e9eb1e95#.fxng326l9). The attack is also possible on extensions that do not use the updateKey field. 

We'll analyze the Passifox install.rdf add-on for attack feasibility. https://github.com/pfn/passifox/blob/master/passifox/install.rdf

1. Develop a malicious extension (id verifications are not effective as implemented see #1303418)
2. Sign the extension
3. Generate forged certificate for passifox.appspot.com
4. MITM traffic to passifox.appspot.com and pass the malicious extension


Actual results:

Since <updateKey> is not a requirement for HTTPS <updateURL> only the transport is encrypted. Since HTTPS pinning is not used the transport layer is vulnerable to a forged certificate attack similar to #1303418.


Expected results:

<updateKey> should be required on both HTTP and HTTPS <updateURL> fields.

Comment 1

[Joshua Yabut]

Since this relies on the invalid-bug #1304112 to be successful this is not a significant issue. Although it would be good practice to implement <updateKey> on both HTTP and HTTPS links.

Comment 2

[Daniel Veditz [:dveditz]]

(In reply to Joshua Yabut from comment #0)
> Since HTTPS pinning is not used the transport layer
> is vulnerable to a forged certificate attack similar to #1303418.

Pinning isn't required but the cert might be pinned if the site uses HPKP (unlikely, I know).

> <updateKey> should be required on both HTTP and HTTPS <updateURL> fields.

Or require it if the site isn't pinned? Downsides:
 * might be hard to check pinning status from the update code
 * updates work for a while, then the pin expires and updates break because no updateKey

This is definitely a problem until the fix for bug 1303418 ships. Because there are proactive steps add-on hosters can take to protect their users (opt-in to updateKey and/or HPKP) we might as well unhide and publicize this aspect of the dangers of the known attack.

Making this a requirement will impact add-on developers, I think the add-on team has to make a call on the product impact here.

Options:
 1) educate hosters on taking these extra steps (but otherwise no change)
 2) use nsISiteSecurityService::IsSecureHost(HEADER_HPKP,...) to see if the site
    is pinned and require updateKey if not
 3) always require updateKey if not hosted on AMO

Comment 3

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

Generating an updateKey is barely supported, using an ancient tool, so I don't think it's a good idea to make this a requirement now. I'd go with (1), and look into other options once/if we have a reasonable replacement for McCoy.

Comment 4

[Andy McKay]

We shouldn't forget similar update mechanisms for WebExtensions as well: https://developer.mozilla.org/en-US/Add-ons/Updates and that would be more of a priority for me since that's the way forward.

I agree with jorgev that (1) is probably our best bet. 

Am I correct in reading the MDN docs that updateURL can be HTTP (with an updateKey) or HTTPS? It feels like just forcing people over to HTTPS would be a good thing.

Comment 5

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

I think you need to use HTTPS unless you have an updateKey.

Comment 6

[Andrew Swan [:aswan]]

Consensus seems to be the simple option #1 from comment 2.

Comment 7

[Daniel Veditz [:dveditz]]

We're awarding the bounty to the parent issue in bug 1303418, which Julien filed based on investigation of your original post (and follow-on reporting such as Ryan Duff's). This is essentially a duplicate of that bug plus some questions about whether we wanted to _require_ stronger security from non-hosted add-ons (which we've decided not to do at this time).