Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Find and prevent system proxy leaks #2103

Closed
diracdeltas opened this issue Jun 6, 2016 · 6 comments
Closed

Find and prevent system proxy leaks #2103

diracdeltas opened this issue Jun 6, 2016 · 6 comments
Assignees
@diracdeltas
Labels
open-in-brave-core post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). priority/P4 Minor loss of function. Workaround usually present. privacy wontfix

Comments

@diracdeltas
Copy link
Member

Node does not generally use system proxy settings, so there are some proxy leaks in Brave. ex: #770 (comment)
@diracdeltas
Copy link
Member Author

diracdeltas commented Jun 6, 2016 •
edited
Loading

known leaks:

  • app/updater.js in getting brave updates
  • app/datafile.js for downloading updates to ABP, tracking protection, https everywhere, etc. files
  • electron's autoUpdater on Windows

possible leaks inherited from chrome:

@diracdeltas diracdeltas self-assigned this Jun 6, 2016
@diracdeltas diracdeltas added this to the 0.11.0dev milestone Jun 6, 2016
diracdeltas added a commit that referenced this issue Jun 7, 2016
@diracdeltas
Use XHR instead of Node request for getting updates
bd2bc7d 
so that our update checks use the system proxy settings. Addresses #2103.
However, I haven't verified that electron autoUpdater itself uses system proxy.

Auditors: @aekeus, @bbondy
diracdeltas added a commit that referenced this issue Jun 8, 2016
@diracdeltas
Make datafile requests use system proxy settings
7c8e812 
Addresses #2103.

Auditors: @bbondy
@diracdeltas
Copy link
Member Author

@aekeus I'm thinking that on windows, if system proxy settings are detected, we should just download the update directly instead of going through the auto-updater. thoughts?

@aekeus
Copy link
Member

aekeus commented Jun 8, 2016

On Windows it is a three step process.

  • Check with our metadata server for pending upgrade
  • Download RELEASES file that points to update binary
  • Download and apply update binary

Squirrel handles step 2 and 3 directly, we control only step 1

@diracdeltas
Copy link
Member Author

Step 1 should be properly proxied now. I meant that if proxy settings are detected, we should do step 2 and 3 ourselves instead of going through Squirrel

@aekeus
Copy link
Member

aekeus commented Jun 8, 2016

Right, that makes sense. I am not sure of the effort required to implement 2 and 3 ourselves.

@diracdeltas diracdeltas modified the milestones: 1.0.0, 0.11.1dev Jul 6, 2016
@diracdeltas diracdeltas mentioned this issue Oct 3, 2017
Closed
@alexwykoff alexwykoff modified the milestones: 1.0.0, Backlog Nov 1, 2017
@tildelowengrimm tildelowengrimm added priority/P4 Minor loss of function. Workaround usually present. post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). labels Apr 3, 2018
@tildelowengrimm
Copy link

Head over to brave/brave-browser#871 for the next developments in this exciting saga!

@bsclifton bsclifton removed this from the Triage Backlog milestone Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@diracdeltas diracdeltas

Labels
open-in-brave-core post-v1 We don't expect to be able to resolve this before releasing v1.0 with Brave Core (instead of Muon). priority/P4 Minor loss of function. Workaround usually present. privacy wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@aekeus @diracdeltas @tildelowengrimm @alexwykoff @bsclifton