Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that when the extension is updated, older rulesets are cleared… #17551

Merged
merged 1 commit into from Mar 7, 2019

Conversation

Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@Hainish
Copy link
Member

@Hainish Hainish commented Mar 6, 2019

In https://trac.torproject.org/projects/tor/ticket/29454 there's a discussion about how Tor Browser users are often getting older versions of rulesets.

Currently, if any rulesets have been successfully downloaded and verified from the update channels (https://www.https-rulesets.org/ for instance) , this will overwrite the extension built-in rulesets.

Polling of the update channels occurs every 24 hours, or on browser startup if it's been longer than 24 hours since the last poll.

In Tor Browser this is a problem, since the tor circuit hasn't been established at browser startup, when the poll request is made. It fails, and the rulesets aren't updated.

For some Tor Browser users, they've successfully downloaded rulesets once, but subsequently they've gotten fails for the above reason. This means they're stuck on an old version of the rulesets, even after the extension itself is updated and they have newer rules available within the extension.

This PR does a few things:

  • When an update to the extension is made, we clear the https://www.https-rulesets.org/ rulesets from the browser. We'll then be using the extension-bundled rulesets.
  • We only update the extension to the latest rulesets from https://www.https-rulesets.org/ if the timestamp for those rulesets is newer than the extension itself. This will prevent the case where HTTPS Everywhere downloads the newest ruleset, then an update to the extension is made which clears the https-rulesets.org rulesets, and then you need to download the rulesets from https-rulesets.org again.
@Hainish Hainish requested review from cschanaj and zoracon Mar 6, 2019
Copy link
Collaborator

@pipboy96 pipboy96 left a comment

In general:

  • There should be a space between a keyword and a parenthesis, such as:
if (true) { }
for (;;) { }

rulesets_obj.addFromJson(channel_result.json.rulesets, channel_result.scope);
let replaces = false;
for(let channel_result of channel_results) {
if(channel_result.replaces == true) {
Copy link
Collaborator

@pipboy96 pipboy96 Mar 6, 2019

Use ===, else it will match any trueish value, e.g. [] or {}.

for(let channel_result of channel_results) {
rulesets_obj.addFromJson(channel_result.json.rulesets, channel_result.scope);
let replaces = false;
for(let channel_result of channel_results) {
Copy link
Collaborator

@pipboy96 pipboy96 Mar 6, 2019

const?

async function initialize(store_param, cb) {
store = store_param;
background_callback = cb;

await loadUpdateChannelsKeys();

if (await store.local.get_promise('extensionTimestamp', 0) != extension_timestamp) {
Copy link
Collaborator

@pipboy96 pipboy96 Mar 6, 2019

!== should be used to avoid edge cases with values that are loosely equal (e.g. 0 and empty string or false).

@@ -271,12 +284,33 @@ function destroyTimer() {
}
}

function clear_replacement_update_channels() {
let keys = [];
for (let update_channel of combined_update_channels) {
Copy link
Collaborator

@pipboy96 pipboy96 Mar 6, 2019

const?

rulesets_obj.addFromJson(channel_result.json.rulesets, channel_result.scope);
let replaces = false;
for(let channel_result of channel_results) {
if(channel_result.replaces == true) {
Copy link
Collaborator

@cschanaj cschanaj Mar 6, 2019

Is it safe to assume channel_result.replaces == true implies the content of channel_result.json.rulesets originates from EFF? My understanding is that if an evil update_channel crafted the response such that channel_result.replaces is true, the bundled rules from EFF might never apply.

Copy link
Member Author

@Hainish Hainish Mar 6, 2019

That is the assumption - there is no way in the update channels UX to add an update channel that has the replacement flag set to true, and I don't plan on exposing that to the UX either. This means that adding an update channel will not result in your default rulesets being overwritten.

Copy link
Collaborator

@cschanaj cschanaj Mar 7, 2019

I see your point and I believe this is a reasonable assumption. (though I guess this should be explicitly documented somewhere, possibly in this or later PR).

@Hainish Hainish force-pushed the ensure-latest-rulesets branch from 43efb74 to 91d2aae Mar 6, 2019
… from storage and not re-downloaded on next check.
@Hainish Hainish force-pushed the ensure-latest-rulesets branch from 91d2aae to ffa5501 Mar 6, 2019
zoracon
zoracon approved these changes Mar 7, 2019
Copy link
Contributor

@zoracon zoracon left a comment

👍

@zoracon zoracon merged commit de97a00 into EFForg:master Mar 7, 2019
1 check passed
@Hainish Hainish deleted the ensure-latest-rulesets branch Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment