Commit 3016a020 authored by Maks Orlovich's avatar Maks Orlovich Committed by Commit Bot

WebSocket: use proper context for mojom::kWebSocketOptionBlockThirdPartyCookies check

The passed in site_for_cookies_ (which considers the entire frame hierarchy)
is what should be used, not origin, which is for local frame only.

(Android Webview is the only embedder in Chrome tree that uses this knob)

Bug: 1052454

Change-Id: I9d25321648d6312df9c3113a3c681fbbb6e71f60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062937
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: default avatarNate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742792}
parent cb737c0d
......@@ -858,6 +858,66 @@ public class CookieManagerTest {
webSocketCookieHelper(false /* shouldUseThirdPartyUrl */, cookieKey, cookieValue));
}
// Tests websockets inside third party frame --- the socket is first party to the frame,
// but the frame itself is third-party to the main document.
private String webSocketThirdPartyFrameCookieHelper(String cookieKey, String cookieValue)
throws Throwable {
TestWebServer webServer = TestWebServer.startSsl();
try {
// |cookieUrl| sets a cookie on response.
String cookieUrl = toThirdPartyUrl(
makeCookieWebSocketUrl(webServer, "/cookie_1", cookieKey, cookieValue));
// This html file includes a script establishing a WebSocket connection to |cookieUrl|,
// with wrappers to talk to parent frame.
String childFrameUrl = toThirdPartyUrl(makeFrameableWebSocketScriptUrl(
webServer, "/frame_with_websocket.html", cookieUrl));
// Wrap that in an iframe on the default domain to make it be third-party, and load it.
String url = makeIframeUrl(webServer, "/parent.html", childFrameUrl);
mActivityTestRule.loadUrlSync(
mAwContents, mContentsClient.getOnPageFinishedHelper(), url);
// Make sure websocket has completed.
JavaScriptUtils.runJavascriptWithAsyncResult(
mAwContents.getWebContents(), "callIframe()");
return mCookieManager.getCookie(cookieUrl);
} finally {
webServer.shutdown();
}
}
@Test
@MediumTest
@Feature({"AndroidWebView", "Privacy"})
public void testThirdPartyIframeCookieForWebSocketHandshake_thirdParty_disabled()
throws Throwable {
allowFirstPartyCookies();
blockThirdPartyCookies(mAwContents);
String cookieKey = "test3PFrame";
String cookieValue = "value3PFrame";
Assert.assertNull("Should not set cookie in 3P frame when 3P cookies are disabled",
webSocketThirdPartyFrameCookieHelper(cookieKey, cookieValue));
}
@Test
@MediumTest
@Feature({"AndroidWebView", "Privacy"})
public void testThirdPartyIframeCookieForWebSocketHandshake_thirdParty_enabled()
throws Throwable {
allowFirstPartyCookies();
allowThirdPartyCookies(mAwContents);
String cookieKey = "test3PFrame";
String cookieValue = "value3PFrame";
Assert.assertEquals(cookieKey + "=" + cookieValue,
webSocketThirdPartyFrameCookieHelper(cookieKey, cookieValue));
}
/**
* Creates a response on the TestWebServer which attempts to set a cookie when fetched.
* @param webServer the webServer on which to create the response
......@@ -921,6 +981,26 @@ public class CookieManagerTest {
return webServer.setResponse(path, responseStr, null);
}
/**
* Creates a response on the TestWebServer which contains a script establishing a WebSocket
* connection in response to a postMessage, and replies when established.
* @param webServer the webServer on which to create the response
* @param path the path component of the url (e.g "/my_thing_with_script.html")
* @param url the url to pass to websocket.
* @return the url which gets the response
*/
private String makeFrameableWebSocketScriptUrl(
TestWebServer webServer, String path, String url) {
String responseStr = "<html><head><title>Content!</title></head>"
+ "<body><script>\n"
+ "window.onmessage = function(ev) {"
+ " let ws = new WebSocket('" + url.replaceAll("^http", "ws") + "');\n"
+ " ws.onopen = () => ev.source.postMessage(true, '*');\n"
+ "}\n"
+ "</script></body></html>";
return webServer.setResponse(path, responseStr, null);
}
@Test
@MediumTest
@Feature({"AndroidWebView", "Privacy"})
......
......@@ -382,6 +382,7 @@ WebSocket::WebSocket(
child_id_(child_id),
frame_id_(frame_id),
origin_(std::move(origin)),
site_for_cookies_(site_for_cookies),
has_raw_headers_access_(has_raw_headers_access),
writable_watcher_(FROM_HERE,
mojo::SimpleWatcher::ArmingPolicy::MANUAL,
......@@ -466,10 +467,6 @@ void WebSocket::StartClosingHandshake(uint16_t code,
}
bool WebSocket::AllowCookies(const GURL& url) const {
// TODO(https://crbug.com/1052454): This should be using passed-in
// site_for_cookies instead.
const net::SiteForCookies site_for_cookies =
net::SiteForCookies::FromOrigin(origin_);
net::StaticCookiePolicy::Type policy =
net::StaticCookiePolicy::ALLOW_ALL_COOKIES;
if (options_ & mojom::kWebSocketOptionBlockAllCookies) {
......@@ -480,7 +477,7 @@ bool WebSocket::AllowCookies(const GURL& url) const {
return true;
}
return net::StaticCookiePolicy(policy).CanAccessCookies(
url, site_for_cookies) == net::OK;
url, site_for_cookies_) == net::OK;
}
int WebSocket::OnBeforeStartTransaction(net::CompletionOnceCallback callback,
......
......@@ -175,6 +175,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) WebSocket : public mojom::WebSocket {
// The web origin to use for the WebSocket.
const url::Origin origin_;
// For 3rd-party cookie permission checking.
net::SiteForCookies site_for_cookies_;
// handshake_succeeded_ is used by WebSocketManager to manage counters for
// per-renderer WebSocket throttling.
bool handshake_succeeded_ = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment