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

Fixes #12223: Mitigate HSTS Fingerprinting #13649

Merged
merged 1 commit into from
Mar 29, 2018
Merged

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Mar 29, 2018

Fixes: #12223

Test Plan:

  • Open Brave. Disable HTTPS Everywhere in preferences.
  • Open safebrowsing-proxy.brave.com (ping @jumde if the domain is not working)
  • Verify that the content from githubusercontent is loaded with a 301 redirect
  • Navigate to example.com
  • Clear Browsing Data Cached images and Files
  • Navigate to safebrowsing-proxy.brave.com and verify that the ninja image is loaded with a 301 redirect.
  • Load https://avatars2.githubusercontent.com/u/1903815?s=40&v=4 in the URL bar
  • Clear Browsing Data Cached images and Files
  • Open safebrowsing-proxy.brave.com and confirm that the HTTPS upgrade for githubusercontent happens with a 307 redirect instead of 301.

Alternative test plan if proxy site is not working

repeat steps above, but replace safebrowsing-proxy.com with https://jsfiddle.net/pqwdgr5x/5/

@jumde jumde requested a review from diracdeltas March 29, 2018 06:09
@codecov-io
Copy link

codecov-io commented Mar 29, 2018

Codecov Report

Merging #13649 into master will decrease coverage by 0.03%.
The diff coverage is 36%.

@@            Coverage Diff             @@
##           master   #13649      +/-   ##
==========================================
- Coverage   56.67%   56.64%   -0.04%     
==========================================
  Files         285      285              
  Lines       28881    28907      +26     
  Branches     4774     4777       +3     
==========================================
+ Hits        16369    16375       +6     
- Misses      12512    12532      +20
Flag Coverage Δ
#unittest 56.64% <36%> (-0.04%) ⬇️
Impacted Files Coverage Δ
app/sessionStore.js 88.4% <100%> (+0.18%) ⬆️
app/filtering.js 17.61% <15.78%> (-0.33%) ⬇️

Copy link
Member

@diracdeltas diracdeltas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test plan works now!

@diracdeltas diracdeltas added this to the 0.23.x (Developer Channel) milestone Mar 29, 2018
@jumde jumde merged commit dbb7297 into master Mar 29, 2018
@jumde
Copy link
Contributor Author

jumde commented Mar 29, 2018

0.23.x - 3136329
master - dbb7297

@bsclifton bsclifton deleted the hsts_fingerprinting branch March 29, 2018 18:20
@@ -823,9 +823,15 @@ module.exports.runPreMigrations = (data) => {
}

if (data.lastAppVersion) {
let runHSTSCleanup = false
try { runHSTSCleanup = compareVersions(data.lastAppVersion, '0.22.00') < 1 } catch (e) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be changed if this PR is going out in 0.23.x instead of 0.22.x

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: since this is going out in 0.22.x release 2, this needs to be updated once we know the version number of 0.22.x release 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: looks like the final version to compare against is 0.22.13

@bsclifton bsclifton modified the milestones: 0.23.x (Developer Channel), 0.22.x Release 2 (Beta Channel) Apr 2, 2018
@bsclifton bsclifton mentioned this pull request Apr 2, 2018
2 tasks
@bsclifton
Copy link
Member

bsclifton commented Apr 3, 2018

0.22.x-single-webview c90a275
0.22.x 8aee363

@bsclifton bsclifton modified the milestones: 0.22.x Release 2 (Beta Channel), 0.22.x Release 3 Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants