1264571 - Add a test case of isolating Broadcast Channels for first party. (Tor 16300)

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Tim Huang[:timhuang]]

Attachment: tor browser patch (WIP)

We should have a test case for isolation of Broadcast Channels to first party. https://torpat.ch/16300

Comment 1

[Arthur Edelstein [:arthur]]

Please note the WIP attachment is out of date, and the current Tor Browser patch is at
https://torpat.ch/16300

Comment 2

[Tanvi Vyas[:tanvi]]

Is this a testing bug or implementation bug?

Comment 3

[Jonathan Hao (inactive) [:jhao]]

Broadcast channel is already separated by origin attributes. Here's the test:
http://searchfox.org/mozilla-central/source/browser/components/contextualidentity/test/browser/browser_broadcastchannel.js
This bug should be resolved as soon as first-party URI is in the origin attributes, but we can extend the test for first-party isolation.

Comment 4

[Tim Huang[:timhuang]]

Attachment: Add a test case of isolating broadcast channels for first party isolation.

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8797397 [details] [diff] [review]
Add a test case of isolating broadcast channels for first party isolation.

Review of attachment 8797397 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/originattributes/test/browser/browser_broadcastChannel.js
@@ +21,5 @@
> +      return data;
> +    }
> +
> +    return displayItem.innerHTML;
> +  });

This doesn't work because we are isolating, but because broadcastChannel is async. You should give time to retrieve the data.
And this is racy, right?

Comment 6

[Tim Huang[:timhuang]]

Just make clear that I understand this correctly, are you saying that this is working is not because the broadcast channel is isolated, but because the broadcast channel is async.

Comment 7

[Andrea Marchesini [:baku]]

Correct.

Comment 8

[Arthur Edelstein [:arthur]]

Comment on attachment 8797397 [details] [diff] [review]
Add a test case of isolating broadcast channels for first party isolation.

Review of attachment 8797397 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/originattributes/test/browser/browser_broadcastChannel.js
@@ +4,5 @@
> +
> +const TEST_DOMAIN = "http://example.net/";
> +const TEST_PATH = TEST_DOMAIN + "browser/browser/components/originattributes/test/browser/";
> +const TEST_PAGE = TEST_PATH + "file_broadcastChannel.html";
> +

// Maybe here you could define something like.

let waitForBroadcastMessage = function (broadcastChannel) {
  return new Promise(resolve => {
    broadCastChannel.onmessage = e => resolve(e);
  });
};

@@ +16,5 @@
> +    // which was received from the broadcast channel.
> +    if (displayItem.innerHTML.length === 0) {
> +      let data = Math.random().toString();
> +      let bc = new content.BroadcastChannel("testBroadcastChannel");
> +      bc.postMessage(data);

// Then here you could do
let messagePromise = waitForBroadcastMessage(bc);
bc.postMessage(data);
yield messagePromise;
// Probably this ensures that the message has been properly broadcast before
// you return from doTest?

Comment 9

[Tim Huang[:timhuang]]

Attachment: Bug 1264571 - Add a test case of isolating broadcast channels for first party isolation.

Review commit: https://reviewboard.mozilla.org/r/83616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83616/

Comment 10

[Arthur Edelstein [:arthur]]

Comment on attachment 8798012 [details]
Bug 1264571 - Add a test case of isolating broadcast channels for first party isolation.

https://reviewboard.mozilla.org/r/83616/#review82250

Looks good to me. Thanks, Tim!

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 8798012 [details]
Bug 1264571 - Add a test case of isolating broadcast channels for first party isolation.

https://reviewboard.mozilla.org/r/83616/#review87578

::: browser/components/originattributes/test/browser/browser_broadcastChannel.js:21
(Diff revision 1)
> +    // The way that how we make sure the message is delivered is based on an
> +    // iframe which will reply everything it receives from the broadcast channel
> +    // to the current window through the postMessage. So, we can know that the
> +    // boradcast message is sent successfully when the window receives a message
> +    // from the iframe.
> +    if (displayItem.innerHTML.length === 0) {

displayItem.innerHTML == ""

::: browser/components/originattributes/test/browser/browser_broadcastChannel.js:26
(Diff revision 1)
> +    if (displayItem.innerHTML.length === 0) {
> +      let data = Math.random().toString();
> +
> +      let receivedData = yield new Promise(resolve => {
> +        let listenFunc = event => {
> +          content.removeEventListener("message", listenFunc, false);

remove "false". It's optional.

Comment 12

[Tim Huang[:timhuang]]

Comment on attachment 8798012 [details]
Bug 1264571 - Add a test case of isolating broadcast channels for first party isolation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/83616/diff/1-2/

Comment 13

[Tim Huang[:timhuang]]

Try looks good.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48aec91f819f
Add a test case of isolating broadcast channels for first party isolation. r=arthuredelstein,baku

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/48aec91f819f

Comment 16

[vietduc]

Công ty phế liệu giá cao Việt Đức đã xem và thấy rất tốt! <a href=" https://phelieuvietduc.com/" rel="nofolow"> https://phelieuvietduc.com/ </a>