-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Proposal: Consider removing https -> https/http rewrite rules? #1327
Comments
FWIW, here's a rough list of files with downgrade/https matching rules. Aeriagames.xml |
The proposal is to disable all rulesets that require downgrades, treating the sites as too broken for us to fix currently? I am in favor. @2d1, opinions? Blockers:
I think other than that none of these rules are vital enough that we can't remove them. |
Hm, @2d1 I think better user experience could lead to wider adoption, which is of value too. Perhaps I'll implement a "lean" (opt-in) mode in Chrome, which drops the HTTPS listeners. Here's a wild idea: What if this significantly increased the extension's security by allowing us to ultimately remove the This would require careful thought (we couldn't regain the ability to read |
I agree that it would be nice to make a security guarantee that our extension can never rewrite content that's already on HTTPS. Note that the downgrade rules may not capture all such examples: It's possible for a rule to rewrite from HTTPS to a different HTTPS URL. @semenko could you do a quick estimate of how many such rules there are? @2d1: I hear you about coverage vs speed. But the list of downgrade rules is quite small, relatively speaking. And I think a number of them may no longer be necessary or may be able to be modified. Also keep in mind that since Chrome and Firefox implemented MCB, we can't actually downgrade JS or CSS, which are the most likely page-breaking subresources. So we're mostly interested in rewriting top-level navigation events where there are incorrect links to an https version that doesn't exist. @semenko, perhaps we could modify your proposal to only allow downgrades of top-level navigation? Or would that still require blocking all requests? @diracdeltas: I'm looking at the downgrade in the Apple rule,
Do you remember writing this one? Can you tell me more about it? |
Could it possible be a crossdomain.xml problem? https://trailers.apple.com http://trailers.apple.com They look the same. If it is a crossdomain.xml problem see the exclusion rule here https://github.com/EFForg/https-everywhere/blob/691be3b4bcb6540258ba895a037866ab48a87cca/src/chrome/content/rules/Cloudfront.xml |
@StevenRoddis I don't think it's a crossdomain problem. The HTTPS version of trailers.apple.com simply doesn't work. The question is: Would it be sufficient to simply have no rule for that subdomain? Presumably not, or the ruleset would be written that way. But I'm not sure who would have linked to the HTTPS version. |
FYI, if you include sideways HTTPS rewrites, i.e. rewriting from one HTTPS URL to another, we have 140 relevant rulesets:
|
I think the next step here should be to see if we can quantify the performance impact of checking HTTPS URLs. We could also look at short-circuiting HTTPS URLs that couldn't possibly match. To do that, at ruleset loading time we would check whether a ruleset includes downgrade or sideways URLs, and make that an attribute of the in-memory RuleSet object. When an HTTPS URL matches a ruleset without the downgrade-or-sideways attribute, we can bail on processing without evaluating any regexes. |
Regardless of performance thoughts, the downgrading of sites from https->http strikes me as really strange (and not something our users are expecting). @jsha unfortunately, earlier/short circuited rules won't help a ton. We spend much of our time now in context-switching overhead (from webRequest C -> handlers for blocking javascript -> C). You can get details via:
The resulting profile is heavy in internal bindings (extensions::) instead of stuff we control. For a head-to-head comparison, get a good testing machine (I use a new-ish Chromebook, which is still pretty underpowered) and try:
To revert:
|
Perhaps when we do addListener, instead of listening on all HTTPS URLs, we could listen only on the ~140 hosts for which we have rewrite rules that can apply to HTTPS? |
Ah -- good call! I tried this once with just HTTP, but it has some undocumented limit (IIRC ~1000?). I'll give it a whirl. |
Ok, I gave this a try with only listening to the target hosts of the https rules. Unfortunately, some wildcard usage at weird places makes this hard. e.g. for Aeriagames.xml:
Here, we'd listen on https://www.aeriagames.com, https://aeriagames.com, https://*.aeriastatic.com/ -- works great! However, for a few sites, we're using * in weird places. Good examples: The VA hospitals (where we have a * in the middle of a url, like subdomain..example.com), and for Google, where use use a * as a TLD (which is really weird) like some.site.google.. I'll look into this a bit more. |
I'm in favor of removing middle-wildcarding for target hosts, and fixing up the relevant rules. I'm guessing for most of them we can enumerate the labels that go in the middle? I assume Chrome also disallows right-hand wildcarding? Those may be more of a hassle to get rid of, since we have more rules depending on them, and the number of hostnames they expand to is larger. However, at least the set of possible hostnames they can expand to is better-defined. One non-intuitive thing about wildcarding: Left-hand wildcards can be arbitrarily deep, but right (and I think middle) wildcards can only go one label deep. |
I think this has gotten better with the awesome new ruleset updates. For example, the GoogleMaps.xml ruleset (which has some rough
Yep. Per the bottom of https://developer.chrome.com/extensions/match_patterns:
|
Have you taken another look at this recently? I see the downgrade whitelist has gotten a lot smaller. |
Briefly -- it's definitely worth doing. The right-sided wildcards are still an issue -- and I think we should aim to remove them. With new TLDs like |
Yeah, I think it would be useful to run a script that does lookups for all the ICANN public suffixes to expand the right-side wildcards to the things they actually resolve to. However, if that affects hostnames that have downgrades, we'll wind up with an explosion of hostnames to the point that it might break this proposal. We should check to see if e.g. the Google Maps downgrade can be removed now. |
@jsha there does not appear to be a downgrade https://github.com/EFForg/https-everywhere/blob/master/src/chrome/content/rules/GoogleMaps.xml |
@jsha @fuzzyroddis (me) Relevant Comment: <!-- covered in exclusion rule, there should be no need to downgrade -->
<!--rule from="^https://maps\.googleapis\.com/map(?=files/lib/map_\d+_\d+\.swf|sapi/publicapi\?file=flashapi)"
to="http://maps.googleapis.com/map" downgrade="1" /--> |
I might be a little tired this morning but I can't understand the logic in this downgrade https://github.com/EFForg/https-everywhere/blob/master/src/chrome/content/rules/Stack-Exchange.xml#L126
Examples of subdomains that match:
I'm trying to find an example of a resource that is included by a https page that causes MCB |
https://github.com/EFForg/https-everywhere/blob/master/src/chrome/content/rules/Yandex.net.xml cs-ellpic.yandex.net https appears to work now |
Problem is on this line: https://github.com/samuelclay/NewsBlur/blob/b8a82544bb81f8d1b54576cadcbcb4d8e8ac4b1f/templates/social/social_page.xhtml#L124 |
…) (#6242) * new readthedocs.io domain * Remove HTTP/HTTPS redirects in line with #1327 * Fix for regex in readthedocs.io * [Read the Docs] Add tests, other minor updates. * Add tests * Expand <securecookie> coverage * Fix missing `$` in a <securecookie> * Move some tests around * Update comments * [Read the Docs] Fix the comment right this time
I am closing this as the support of downgrade rules has been removed in #10396. |
For reference, the work to remove |
We have a small subset of rules that rewrite HTTPS sites to other HTTPS/HTTP sites. A rough grepping shows ~63 rules like this.
I propose removing these rules, and the code needed to handle them.
Benefits:
Downsides:
Thoughts?
The text was updated successfully, but these errors were encountered: