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.

Sign up for GitHub

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

Jump to bottom

Fix 8034: Add Crowd Deny requests to audit whitelist #8035

Merged
merged 1 commit into from
Feb 6, 2020
Merged

Fix 8034: Add Crowd Deny requests to audit whitelist #8035

merged 1 commit into from
Feb 6, 2020

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Feb 3, 2020 •
edited
Loading

Fix #8034
PR for proxy brave/brave-core#4482

Submitter Checklist:

Test Plan:

  1. network-audit should pass.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions.

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@jumde jumde self-assigned this Feb 3, 2020
@jumde jumde requested a review from diracdeltas February 3, 2020 22:06
@brave-builds brave-builds force-pushed the crowd_deny_proxy branch 3 times, most recently from 723f7ba to d06cffc Compare February 5, 2020 06:19
@jumde jumde added the QA/No label Feb 5, 2020
@brave-builds brave-builds force-pushed the crowd_deny_proxy branch 2 times, most recently from adb6a50 to bc508d4 Compare February 5, 2020 16:09
diracdeltas
diracdeltas reviewed Feb 5, 2020
View reviewed changes
lib/whitelistedUrlPatterns.js
@@ -2,6 +2,8 @@
module.exports = [
'http://[A-Za-z0-9-\.]+\.gvt1\.com/edgedl/release2/chrome_component/.+', // allowed because it 307's to crlsets.brave.com
'https://[A-Za-z0-9-\.]+\.gvt1\.com/edgedl/release2/chrome_component/.+', // allowed because it 307's to crlsets.brave.com
'http://[A-Za-z0-9-\.]+\.gvt1\.com/edgedl/release2/.+', // allowed because it 307's to redirector.brave.com
'https://[A-Za-z0-9-\.]+\.gvt1\.com/edgedl/release2/.+', // allowed because it 307's to redirector.brave.com
Copy link
Member

Choose a reason for hiding this comment

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

if i'm not mistaken, this is a superset of the patterns above it, so those patterns can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

diracdeltas
diracdeltas reviewed Feb 5, 2020
View reviewed changes
lib/whitelistedUrlPrefixes.js Outdated
@@ -30,4 +30,5 @@ module.exports = [
'https://dns.google/dns-query', // needed for DoH on Mac build machines
'https://chrome.cloudflare-dns.com/dns-query', // needed for DoH on Mac build machines
'https://tor.bravesoftware.com/', // for fetching tor client updater component
'https://redirector.brave.com',
Copy link
Member

Choose a reason for hiding this comment

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

missing slash at the end

diracdeltas
diracdeltas previously approved these changes Feb 5, 2020
View reviewed changes
@jumde jumde merged commit a64bfee into master Feb 6, 2020
@jumde jumde deleted the crowd_deny_proxy branch February 6, 2020 05:41
jumde added a commit that referenced this pull request Feb 6, 2020
@jumde
Merge pull request #8035 from brave/crowd_deny_proxy
c0fac76 
Fix 8034: Add Crowd Deny requests to audit whitelist
@jumde jumde mentioned this pull request Feb 6, 2020
Closed
jumde added a commit that referenced this pull request Feb 6, 2020
@jumde
Merge pull request #8035 from brave/crowd_deny_proxy
d3c5855 
Fix 8034: Add Crowd Deny requests to audit whitelist
@jumde jumde mentioned this pull request Feb 6, 2020
Closed
jumde added a commit that referenced this pull request Feb 6, 2020
@jumde
Merge pull request #8035 from brave/crowd_deny_proxy
e88af00 
Fix 8034: Add Crowd Deny requests to audit whitelist
@jumde jumde mentioned this pull request Feb 6, 2020
Closed
@jumde jumde added this to the 1.6.x - Nightly milestone Feb 6, 2020
jumde added a commit that referenced this pull request Feb 6, 2020
@jumde
Merge pull request #8035 from brave/crowd_deny_proxy
5d998d2 
Fix 8034: Add Crowd Deny requests to audit whitelist
@jumde jumde mentioned this pull request Feb 6, 2020
Merged
jumde added a commit that referenced this pull request Feb 6, 2020
@jumde
Merge pull request #8035 from brave/crowd_deny_proxy
2c84837 
Fix 8034: Add Crowd Deny requests to audit whitelist
This was referenced Feb 6, 2020
Merged
Merged
brave-builds pushed a commit that referenced this pull request Feb 6, 2020
@jumde @brave-browser-releases
Merge pull request #8035 from brave/crowd_deny_proxy
9472840 
Fix 8034: Add Crowd Deny requests to audit whitelist
@bbondy bbondy modified the milestones: 1.6.x - Beta, 1.7.x - Dev Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diracdeltas diracdeltas diracdeltas approved these changes

Assignees

@jumde jumde

Labels
QA/No
Projects
None yet
Milestone
1.7.x - Release
Development

Successfully merging this pull request may close these issues.

Add Crowd Deny requests to audit whitelist
3 participants
@jumde @diracdeltas @bbondy