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

Issue 3920: Proxy requests for resources from storage.googleapis.com through the crlset redirector #2114

Merged
merged 1 commit into from
Apr 1, 2019

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Mar 28, 2019

PR builder passed here: https://staging.ci.brave.com/job/brave-browser-build-pr/job/PR-3929/

Description

fix: brave/brave-browser#3920

CRLSets on windows started requesting resources from storage.googleapis.com. We updated the redirector to proxy requests for these resources: brave/redirector@372e442

This PR updates the network delegate to proxy requests for storage.googleapis.com through the redirector.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Verify npm run test-security passes
  2. Navigate to brave://components and verify that CRLSet component version is non-zero
  3. revoked.badssl.com - shows a certificate error.
  • Win/Linux - Using Fidler, make sure you’re not connecting to storage.googleapis.com on startup (leave the browser running for a while and make sure no other Google API calls are made)
  • macOS - Same thing but Little Snitch.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@jumde jumde self-assigned this Mar 28, 2019
@jumde jumde force-pushed the storage_proxy_crlsets branch 6 times, most recently from 1f66594 to 418e9f1 Compare March 29, 2019 18:32
@@ -8,4 +8,4 @@ index 3d9a560e26e43fb2da1332dff339c1039cfb1a3e..4050aa4ee4122124f154ceeff1e3ac83
-BUILD=3683
-PATCH=75
+BUILD=65
+PATCH=28
+PATCH=0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To-Do: Remove. Changed to get around CI build issues, will be fixed in master soon.

@jumde jumde changed the title WIP: Issue 3920: Proxy requests for resources from storage.googleapis.com through the crlset redirector Issue 3920: Proxy requests for resources from storage.googleapis.com through the crlset redirector Mar 30, 2019
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

LGTM! 😄 👍

@jumde jumde merged commit 6290c65 into master Apr 1, 2019
@jumde jumde added this to the 0.65.x - Nightly milestone Apr 1, 2019
@mihaiplesa mihaiplesa deleted the storage_proxy_crlsets branch May 13, 2019 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to crlsets connects to storage.googleapis on windows
2 participants