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

Issue 2160: Proxy requests for CRLSets through crlsets[n].brave.com #920

Merged
merged 1 commit into from
Nov 27, 2018
Merged

Issue 2160: Proxy requests for CRLSets through crlsets[n].brave.com #920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions browser/net/brave_static_redirect_network_delegate_helper.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ int OnBeforeURLRequest_StaticRedirectWork(
GURL::Replacements replacements;
static URLPattern geo_pattern(URLPattern::SCHEME_HTTPS, kGeoLocationsPattern);
static URLPattern safeBrowsing_pattern(URLPattern::SCHEME_HTTPS, kSafeBrowsingPrefix);
static URLPattern crlSet_pattern1(URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS,
kCRLSetPrefix1);
static URLPattern crlSet_pattern2(URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS,
kCRLSetPrefix2);

if (geo_pattern.MatchesURL(ctx->request_url)) {
ctx->new_url_spec = GURL(GOOGLEAPIS_ENDPOINT GOOGLEAPIS_API_KEY).spec();
Expand All @@ -27,6 +31,20 @@ int OnBeforeURLRequest_StaticRedirectWork(
return net::OK;
}

if (crlSet_pattern1.MatchesHost(ctx->request_url)) {
replacements.SetSchemeStr("https");
replacements.SetHostStr("crlsets1.brave.com");
ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
return net::OK;
}

if (crlSet_pattern2.MatchesHost(ctx->request_url)) {
replacements.SetSchemeStr("https");
replacements.SetHostStr("crlsets2.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.

eventually i think it would be good to move all these proxy endpoints to a single endpoint like proxy.brave.com instead of having one endpoint for every google host. that way, people who use little snitch or other firewalls will only have to whitelist one host and new proxied requests will not trigger alerts.

ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
return net::OK;
}

#if !defined(NDEBUG)
GURL gurl = ctx->request_url;
static std::vector<URLPattern> allowed_patterns({
Expand Down Expand Up @@ -58,9 +76,14 @@ int OnBeforeURLRequest_StaticRedirectWork(
URLPattern(URLPattern::SCHEME_HTTPS, "https://safebrowsing.brave.com/v4/*"),
URLPattern(URLPattern::SCHEME_HTTPS, "https://ssl.gstatic.com/safebrowsing/*"),

//CRLSets
URLPattern(URLPattern::SCHEME_HTTPS, "https://crlsets1.brave.com/*"),
URLPattern(URLPattern::SCHEME_HTTPS, "https://crlsets2.brave.com/*"),

// Will be removed when https://github.com/brave/brave-browser/issues/663 is fixed
URLPattern(URLPattern::SCHEME_HTTPS, "https://www.gstatic.com/*"),
});

// Check to make sure the URL being requested matches at least one of the allowed patterns
bool is_url_allowed = std::any_of(allowed_patterns.begin(), allowed_patterns.end(),
[&gurl](URLPattern pattern) {
Expand Down
40 changes: 40 additions & 0 deletions browser/net/brave_static_redirect_network_delegate_helper_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,46 @@ TEST_F(BraveStaticRedirectNetworkDelegateHelperTest, ModifyGeoURL) {
EXPECT_EQ(ret, net::OK);
}

TEST_F(BraveStaticRedirectNetworkDelegateHelperTest, ModifyCRLSet1) {
net::TestDelegate test_delegate;
GURL url("https://dl.google.com/release2/chrome_component/AJ4r388iQSJq_4819/"
"4819_all_crl-set-5934829738003798040.data.crx3");
std::unique_ptr<net::URLRequest> request =
context()->CreateRequest(url, net::IDLE, &test_delegate,
TRAFFIC_ANNOTATION_FOR_TESTS);
std::shared_ptr<brave::BraveRequestInfo>
before_url_context(new brave::BraveRequestInfo());
brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
brave::ResponseCallback callback;
GURL expected_url("https://crlsets1.brave.com/release2/chrome_component/"
"AJ4r388iQSJq_4819/4819_all_crl-set-5934829738003798040.data.crx3");
int ret =
OnBeforeURLRequest_StaticRedirectWork(callback,
before_url_context);
EXPECT_EQ(before_url_context->new_url_spec, expected_url);
EXPECT_EQ(ret, net::OK);
}

TEST_F(BraveStaticRedirectNetworkDelegateHelperTest, ModifyCRLSet2) {
net::TestDelegate test_delegate;
GURL url("https://r2---sn-8xgp1vo-qxoe.gvt1.com/edgedl/release2/chrome_compone"
"nt/AJ4r388iQSJq_4819/4819_all_crl-set-5934829738003798040.data.crx3");
std::unique_ptr<net::URLRequest> request =
context()->CreateRequest(url, net::IDLE, &test_delegate,
TRAFFIC_ANNOTATION_FOR_TESTS);
std::shared_ptr<brave::BraveRequestInfo>
before_url_context(new brave::BraveRequestInfo());
brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
brave::ResponseCallback callback;
GURL expected_url("https://crlsets2.brave.com/edgedl/release2/chrome_compone"
"nt/AJ4r388iQSJq_4819/4819_all_crl-set-5934829738003798040.data.crx3");
int ret =
OnBeforeURLRequest_StaticRedirectWork(callback,
before_url_context);
EXPECT_EQ(before_url_context->new_url_spec, expected_url);
EXPECT_EQ(ret, net::OK);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Tests LGTM

TEST_F(BraveStaticRedirectNetworkDelegateHelperTest, ModifySafeBrowsingURLV4) {
net::TestDelegate test_delegate;
GURL url("https://safebrowsing.googleapis.com/v4/threatListUpdates:fetch?$req=ChkKCGNocm9taXVtEg02Ni");
Expand Down
2 changes: 2 additions & 0 deletions common/network_constants.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const char kEmptyImageDataURI[] = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP/
const char kJSDataURLPrefix[] = "data:application/javascript;base64,";
const char kGeoLocationsPattern[] = "https://www.googleapis.com/geolocation/v1/geolocate?key=*";
const char kSafeBrowsingPrefix[] = "https://safebrowsing.googleapis.com/";
const char kCRLSetPrefix1[] = "https://dl.google.com/release2/chrome_component/*crl-set*";
const char kCRLSetPrefix2[] = "https://*.gvt1.com/edgedl/release2/chrome_component/*crl-set*";
const char kGoogleTagManagerPattern[] = "https://www.googletagmanager.com/gtm.js";
const char kGoogleTagServicesPattern[] = "https://www.googletagservices.com/tag/js/gpt.js";
const char kForbesPattern[] = "https://www.forbes.com/*";
Expand Down
2 changes: 2 additions & 0 deletions common/network_constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extern const char kGoogleTagServicesPattern[];
extern const char kForbesPattern[];
extern const char kForbesExtraCookies[];
extern const char kSafeBrowsingPrefix[];
extern const char kCRLSetPrefix1[];
extern const char kCRLSetPrefix2[];
extern const char kTwitterPattern[];
extern const char kTwitterReferrer[];
extern const char kTwitterRedirectURL[];
Expand Down