BroadcastChannel instances in distinct opaque origins can communicate

[Andrew Williams]

I wrote a test to determine whether BroadcastChannel instances in distinct opaque origins (tied to the same document) can communicate, and it looks like they can in Safari Tech Preview:

https://wpt.fyi/results/webmessaging/broadcastchannel/opaque-origin.html?label=experimental&label=master&aligned

BroadcastChannel messages should only be sent to instances that are same-origin, per the HTML standard.

I was curious whether this could be leveraged to bypass top-level site partitioning as well, but it doesn't appear to. I tested in the browser using the following code (run via the JS console) on two different sites, verifying that no console log messages appeared:

```
const iframe_src = (channel_name, msg) => `data:text/html,<script>
let bc2 = new BroadcastChannel("${channel_name}");
bc2.onmessage = (e) => {
  console.log(e.data);
};
bc2.postMessage("${msg}");
</script>`;

let iframe2 = document.createElement("iframe");

iframe2.src = iframe_src('test', window.location.href);

document.body.appendChild(iframe2);
```

[Radar WebKit Bug Importer]

<rdar://problem/90511155>

[Chris Dumez]

Thank you for the bug report, I will investigate.

[Chris Dumez]

Created attachment 455264 [details]
Patch

[Chris Dumez]

Created attachment 455265 [details]
Patch

[Alex Christensen]

Comment on attachment 455265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455265&action=review

> Source/WebKit/ChangeLog:14
> +        To address the issue, I introduced a new PartitionedSecurityOrigin type which is similar

Do you think PartitionedSecurityOrigin will eventually replace ClientOrigin, or do we only need it in this case?

> Source/WebCore/page/PartitionedSecurityOrigin.h:74
> +    static bool equal(const WebCore::PartitionedSecurityOrigin& a, const WebCore::PartitionedSecurityOrigin& b) { return a == b; }

This uses isSameOriginAs.  Do we want to use isSameSchemeHostPort for hash table equivalence?

> Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.h:55
> +    NetworkProcess& m_networkProcess;

Can this be a Ref<NetworkProcess> instead?

[Chris Dumez]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 455265 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455265&action=review
> 
> > Source/WebKit/ChangeLog:14
> > +        To address the issue, I introduced a new PartitionedSecurityOrigin type which is similar
> 
> Do you think PartitionedSecurityOrigin will eventually replace ClientOrigin,
> or do we only need it in this case?

I think ClientOrigin is still useful when sending across processes.

> 
> > Source/WebCore/page/PartitionedSecurityOrigin.h:74
> > +    static bool equal(const WebCore::PartitionedSecurityOrigin& a, const WebCore::PartitionedSecurityOrigin& b) { return a == b; }
> 
> This uses isSameOriginAs.  Do we want to use isSameSchemeHostPort for hash
> table equivalence?

We definitely do not. isSameOriginAs() properly deals with unique SecurityOrigins (i.e. if the pointers are equal then the origins are the same, which is helpful in the unique origin case. Also, it only calls isSameSchemeHostPort() for non-unique origins, since the scheme/host/port are empty for unique origins).
 
> > Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.h:55
> > +    NetworkProcess& m_networkProcess;
> 
> Can this be a Ref<NetworkProcess> instead?

Maybe, I'd have to make sure it doesn't create a cycle. It is actually safe here since NetworkProcess owns NetworkSession (via unique_ptr) which owns the NetworkBroadcastChannelRegistry (also via unique_ptr).

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Alex Christensen from comment #5)
> > Comment on attachment 455265 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=455265&action=review
> > 
> > > Source/WebKit/ChangeLog:14
> > > +        To address the issue, I introduced a new PartitionedSecurityOrigin type which is similar
> > 
> > Do you think PartitionedSecurityOrigin will eventually replace ClientOrigin,
> > or do we only need it in this case?
> 
> I think ClientOrigin is still useful when sending across processes.
> 
> > 
> > > Source/WebCore/page/PartitionedSecurityOrigin.h:74
> > > +    static bool equal(const WebCore::PartitionedSecurityOrigin& a, const WebCore::PartitionedSecurityOrigin& b) { return a == b; }
> > 
> > This uses isSameOriginAs.  Do we want to use isSameSchemeHostPort for hash
> > table equivalence?
> 
> We definitely do not. isSameOriginAs() properly deals with unique
> SecurityOrigins (i.e. if the pointers are equal then the origins are the
> same, which is helpful in the unique origin case. Also, it only calls
> isSameSchemeHostPort() for non-unique origins, since the scheme/host/port
> are empty for unique origins).

In other words, because SecurityOriginHash relies on isSameSchemeHostPort(), it basically treats distinct opaque security origins as equal. I hope we never store opaque/unique security origins in HashMaps..

[Chris Dumez]

Created attachment 455283 [details]
Patch

[EWS]

Committed r291589 (248684@main): <https://commits.webkit.org/248684@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455283 [details].

[Brent Fulgham]

This fix shipped with Safari 15.5 (all platforms).