Skip to content

Commit

Permalink
Merge pull request #9940 from /issues/17795
Browse files Browse the repository at this point in the history
Restore network cookies access in 3p-allowed frames.
  • Loading branch information
goodov authored Sep 1, 2021
2 parents ae8f4a2 + 01bd05b commit 95228a4
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 59 deletions.
12 changes: 4 additions & 8 deletions browser/ephemeral_storage/ephemeral_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -648,8 +648,7 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
EXPECT_EQ("", values_after.iframe_2.cookies);
}

IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
NetworkCookiesAreNotSentIn3p) {
IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest, NetworkCookiesAreSentIn3p) {
WebContents* site_a_tab = LoadURLInNewTab(a_site_ephemeral_storage_url_);
SetValuesInFrames(site_a_tab, "a.com", "from=a.com");

Expand All @@ -658,14 +657,11 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
// Non 3p request should have cookies in headers.
EXPECT_TRUE(http_request_monitor_.HasHttpRequestWithCookie(
a_site_ephemeral_storage_url_, "from=a.com"));
// 3p requests should NOT have cookies in headers, even from the ephemeral
// 3p requests should have cookies in headers from the ephemeral
// storage.
// https://chromium-review.googlesource.com/c/chromium/src/+/2367394
// "<...> when the NetworkDelegate forces PrivacyMode, it will disable sending
// auth credentials".
EXPECT_FALSE(http_request_monitor_.HasHttpRequestWithCookie(
EXPECT_TRUE(http_request_monitor_.HasHttpRequestWithCookie(
b_site_ephemeral_storage_url_, "from=a.com"));
EXPECT_FALSE(http_request_monitor_.HasHttpRequestWithCookie(
EXPECT_TRUE(http_request_monitor_.HasHttpRequestWithCookie(
b_site_ephemeral_storage_url_.Resolve("/simple.html"), "from=a.com"));

// Cookie values should be available via JS API.
Expand Down
51 changes: 51 additions & 0 deletions browser/net/brave_network_delegate_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "base/path_service.h"
#include "base/strings/strcat.h"
#include "base/strings/stringprintf.h"
#include "brave/common/brave_paths.h"
#include "brave/common/pref_names.h"
Expand All @@ -26,10 +27,13 @@
#include "content/public/test/browser_test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/http_request.h"
#include "url/gurl.h"

using net::test_server::EmbeddedTestServer;

namespace {

bool NavigateRenderFrameToURL(content::RenderFrameHost* frame,
std::string iframe_id,
const GURL& url) {
Expand All @@ -46,6 +50,15 @@ bool NavigateRenderFrameToURL(content::RenderFrameHost* frame,
return result;
}

GURL GetHttpRequestURL(const net::test_server::HttpRequest& http_request) {
return GURL(
base::StrCat({http_request.base_url.scheme_piece(), "://",
http_request.headers.at(net::HttpRequestHeaders::kHost),
http_request.relative_url}));
}

} // namespace

class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
public:
BraveNetworkDelegateBrowserTest()
Expand All @@ -62,6 +75,9 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {

https_server_.ServeFilesFromDirectory(test_data_dir);
https_server_.AddDefaultHandlers(GetChromeTestDataDir());
https_server_.RegisterRequestMonitor(base::BindRepeating(
&BraveNetworkDelegateBrowserTest::MonitorHTTPRequest,
base::Unretained(this)));
content::SetupCrossSiteRedirector(&https_server_);
ASSERT_TRUE(https_server_.Start());

Expand Down Expand Up @@ -180,6 +196,17 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
false);
}

void MonitorHTTPRequest(const net::test_server::HttpRequest& request) {
auto cookie_it = request.headers.find(net::HttpRequestHeaders::kCookie);
if (cookie_it != request.headers.end()) {
seen_cookies_[GetHttpRequestURL(request)] = cookie_it->second;
}
}

const base::flat_map<GURL, std::string>& seen_cookies() const {
return seen_cookies_;
}

protected:
GURL url_;
GURL nested_iframe_script_url_;
Expand All @@ -201,6 +228,7 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
GURL ipfs_cid1_url_;
GURL ipfs_cid2_frame_url_;
net::test_server::EmbeddedTestServer https_server_;
base::flat_map<GURL, std::string> seen_cookies_;

private:
ContentSettingsPattern top_level_page_pattern_;
Expand Down Expand Up @@ -662,6 +690,29 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ExpectCookiesOnHost(GURL("https://example.wordpress.com"), "frame=true");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ThirdPartyYesNetworkCookieWpComInWordpressCom) {
NavigateToPageWithFrame(wordpress_top_url_);
ExpectCookiesOnHost(GURL("https://example.wp.com"), "");

NavigateFrameTo(wp_frame_url_);
ExpectCookiesOnHost(GURL("https://example.wp.com"), "frame=true");

// No network cookie should be sent on first request.
EXPECT_FALSE(base::Contains(seen_cookies(), wp_frame_url_));

// Navigate from WordPress elsewhere.
NavigateToPageWithFrame(cookie_iframe_url_);

// Navigate to WordPress and to a friendly 3p frame to ensure network cookies
// are sent from the frame.
NavigateToPageWithFrame(wordpress_top_url_);
NavigateFrameTo(wp_top_url_);

ASSERT_TRUE(base::Contains(seen_cookies(), wp_top_url_));
EXPECT_EQ(seen_cookies().at(wp_top_url_), "frame=true");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
BlockThirdPartyCookiesIPFSLocalhost) {
DefaultBlockThirdPartyCookies();
Expand Down
12 changes: 12 additions & 0 deletions chromium_src/services/network/cookie_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ bool CookieSettings::IsEphemeralCookieAccessible(
return IsCookieAccessible(cookie, url, site_for_cookies, top_frame_origin);
}

bool CookieSettings::IsEphemeralPrivacyModeEnabled(
const GURL& url,
const GURL& site_for_cookies,
const absl::optional<url::Origin>& top_frame_origin,
net::SamePartyContext::Type same_party_cookie_context_type) const {
if (IsEphemeralCookieAccessAllowed(url, site_for_cookies, top_frame_origin))
return false;

return IsPrivacyModeEnabled(url, site_for_cookies, top_frame_origin,
same_party_cookie_context_type);
}

bool CookieSettings::AnnotateAndMoveUserBlockedEphemeralCookies(
const GURL& url,
const GURL& site_for_cookies,
Expand Down
8 changes: 8 additions & 0 deletions chromium_src/services/network/cookie_settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
const absl::optional<url::Origin>& top_frame_origin) const; \
bool IsCookieAccessible

#define IsPrivacyModeEnabled \
IsEphemeralPrivacyModeEnabled( \
const GURL& url, const GURL& site_for_cookies, \
const absl::optional<url::Origin>& top_frame_origin, \
net::SamePartyContext::Type same_party_context_type) const; \
bool IsPrivacyModeEnabled

#define AnnotateAndMoveUserBlockedCookies \
AnnotateAndMoveUserBlockedEphemeralCookies( \
const GURL& url, const GURL& site_for_cookies, \
Expand All @@ -24,6 +31,7 @@
#include "../../../../services/network/cookie_settings.h"

#undef AnnotateAndMoveUserBlockedCookies
#undef IsPrivacyModeEnabled
#undef IsCookieAccessible

#endif // BRAVE_CHROMIUM_SRC_SERVICES_NETWORK_COOKIE_SETTINGS_H_
42 changes: 5 additions & 37 deletions chromium_src/services/network/network_service_network_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,12 @@
#include "services/network/cookie_settings.h"

#define IsCookieAccessible IsEphemeralCookieAccessible
#define OnAnnotateAndMoveUserBlockedCookies \
OnAnnotateAndMoveUserBlockedCookies_ChromiumImpl
#define OnCanSetCookie OnCanSetCookie_ChromiumImpl
#define IsPrivacyModeEnabled IsEphemeralPrivacyModeEnabled
#define AnnotateAndMoveUserBlockedCookies \
AnnotateAndMoveUserBlockedEphemeralCookies

#include "../../../../services/network/network_service_network_delegate.cc"

#undef OnCanSetCookie
#undef OnAnnotateAndMoveUserBlockedCookies
#undef AnnotateAndMoveUserBlockedCookies
#undef IsPrivacyModeEnabled
#undef IsCookieAccessible

namespace network {

bool NetworkServiceNetworkDelegate::OnAnnotateAndMoveUserBlockedCookies(
const net::URLRequest& request,
net::CookieAccessResultList& maybe_included_cookies,
net::CookieAccessResultList& excluded_cookies,
bool allowed_from_caller) {
// Enable ephemeral storage support for the call.
auto scoped_ephemeral_storage_awareness =
network_context_->cookie_manager()
->cookie_settings()
.CreateScopedEphemeralStorageAwareness();
return OnAnnotateAndMoveUserBlockedCookies_ChromiumImpl(
request, maybe_included_cookies, excluded_cookies, allowed_from_caller);
}

bool NetworkServiceNetworkDelegate::OnCanSetCookie(
const net::URLRequest& request,
const net::CanonicalCookie& cookie,
net::CookieOptions* options,
bool allowed_from_caller) {
// Enable ephemeral storage support for the call.
auto scoped_ephemeral_storage_awareness =
network_context_->cookie_manager()
->cookie_settings()
.CreateScopedEphemeralStorageAwareness();
return OnCanSetCookie_ChromiumImpl(request, cookie, options,
allowed_from_caller);
}

} // namespace network
14 changes: 0 additions & 14 deletions chromium_src/services/network/network_service_network_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,6 @@
#ifndef BRAVE_CHROMIUM_SRC_SERVICES_NETWORK_NETWORK_SERVICE_NETWORK_DELEGATE_H_
#define BRAVE_CHROMIUM_SRC_SERVICES_NETWORK_NETWORK_SERVICE_NETWORK_DELEGATE_H_

#define FinishedCanSendReportingReports \
NotUsed() {} \
bool OnAnnotateAndMoveUserBlockedCookies_ChromiumImpl( \
const net::URLRequest& request, \
net::CookieAccessResultList& maybe_included_cookies, \
net::CookieAccessResultList& excluded_cookies, \
bool allowed_from_caller); \
bool OnCanSetCookie_ChromiumImpl( \
const net::URLRequest& request, const net::CanonicalCookie& cookie, \
net::CookieOptions* options, bool allowed_from_caller); \
void FinishedCanSendReportingReports

#include "../../../../services/network/network_service_network_delegate.h"

#undef FinishedCanSendReportingReports

#endif // BRAVE_CHROMIUM_SRC_SERVICES_NETWORK_NETWORK_SERVICE_NETWORK_DELEGATE_H_

0 comments on commit 95228a4

Please sign in to comment.