Introduce 'webrtc' as a simple on/off switch #457

zenhack · 2021-01-18T23:18:06Z

This builds on #287; I've merged in changes from master and then changed the description so that this is a simple on-off switch with * allowing anything and 'none' denying everything.

The original PR had at least one TODO in there that I haven't yet tackled, so this is definitely still in the "starting point for discussion" phase.

PTAL

(2022-02-14): For folks not wanting to wade through over a year worth of comments, a summary of things that we ended up settling on:

The directive's name changed from webrtc-src to webrtc, and we don't inherit from any other directive due to compatibility considerations.
The accepted values are 'allow' and 'block', rather than * and 'none'
In the companion pr at Add a CSP check to RTCPeerConnection.addIceCandidate(). webrtc-extensions#81, we integrate this with webrtc in the definition of "administratively prohibited."

Preview | Diff

The 'webrtc-src' directive is a proposal for handling WebRTC connections. This patch isn't exactly finished, but it should at least give a concrete proposal that we can discuss in w3c#92.

Minor conflicts wrt. new sections added; just required picking an order.

There's no longer any server/peer specific logic, it's just yes or no now.

zenhack · 2021-01-18T23:20:58Z

The CI has pointed out that I need to sign off on some licensing stuff. I'm not affiliated with any relevant organization, so probably this case applies:

Otherwise, the WG’s team contacts will request the contributors to sign the non-participant licensing commitments

how do I go about this?

index.src.html

mozfreddyb · 2021-01-21T07:29:24Z

The CI has pointed out that I need to sign off on some licensing stuff. I'm not affiliated with any relevant organization, so probably this case applies:

Otherwise, the WG’s team contacts will request the contributors to sign the non-participant licensing commitments

how do I go about this?

Paging @samuelweiler, can you help with this?

samuelweiler · 2021-01-22T21:04:01Z

The CI has pointed out that I need to sign off on some licensing stuff. I'm not affiliated with any relevant organization, so probably this case applies:

Otherwise, the WG’s team contacts will request the contributors to sign the non-participant licensing commitments

how do I go about this?

Paging @samuelweiler, can you help with this?

@zenhack Thanks for asking. It's a multi-stage process, but it's also easy.

Create a w3c account, if you don't have one already.
Then connect your W3C & GitHub accounts.
Lastly, make the appropriate IPR commitment.

zenhack · 2021-01-23T22:07:11Z

I went through the process re: IPR, invited expert application is pending, thanks.

@mozfreddyb, could you also weigh in on the telemetry question?

samuelweiler · 2021-01-25T22:55:54Z

@zenhack I was apparently somewhat wrong about the process - or at least the links; you don't need to become an IE to do this. You should now have an email with the proper form.

zenhack · 2021-01-26T01:05:50Z

Quoting Samuel Weiler (2021-01-25 17:56:26)

@zenhack I was apparently somewhat wrong about the process - or at least the links; you don't need to become an IE to do this. You should now have an email with the proper form.

Yep, got it, and filled it out. Thanks.

…

-Ian

This would be a breaking change.

@annevk

To make it more apparent that this is a stand-alone thing which doesn't inherit from anything else, per @annevk's suggestion.

...since we've backed out the finer grained cases.

zenhack · 2021-02-03T22:14:46Z

The one big TODO still left is the integration section. My current thinking is that the constructor for RTCPeerConnection should call into the procedure specified here, before doing much of anything else. So this would probably affect this section of the spec:

https://www.w3.org/TR/webrtc/#constructor

afaik there are no other entry points that would need to be blocked. Thoughts?

annevk · 2021-02-04T07:31:31Z

If you block the constructor you'll also block postMessage() of it, right?

zenhack · 2021-02-04T15:14:13Z

Hm, that hadn't specifically been my intent, is it a corollary of some detail of how object transfer works that I'm unaware of?

Note that the discussion in w3c/mediacapture-extensions#16 (comment) is about transferring MediaStreamTrack, not RTCPeerConnection, so you wouldn't be able to postMessage() the RTCPeerConnection itself anyway, only tracks obtained from it.

Note that there's some suggestion of making RTCDataChannel transferrable as well:

w3c/webrtc-extensions#6

sideshowbarker · 2021-02-18T03:59:02Z

@annevk If this is merged, will that resolve #92?

annevk · 2021-02-18T07:11:58Z

I think so, even though it would be opt-in. We should try to avoid that with WebTransport.

mikewest · 2021-02-18T08:43:43Z

I was assuming Web Transport would tie into connect-src, just like web sockets?

mikewest · 2021-02-18T08:44:42Z

Also, this kind of thing still seems reasonable to me, but cc @antosart to evaluate this from Chromium's perspective.

martinthomson · 2022-02-06T20:42:42Z

¯\_(ツ)_/¯

zenhack · 2022-02-07T20:37:17Z

Ok, so I think we're just waiting on @annevk now.

annevk

Thank you @zenhack for your tireless efforts on fixing this gap in the web platform. It's unfortunate this is the best we can do due to web compatibility, but that's still a lot better than doing nothing at all.

I think it's worth updating #457 (comment) to explain the setup we ended up with.

This as well w3c/webrtc-extensions#81 look good to me.

Is there a web-platform-tests PR as well?

zenhack · 2022-02-14T21:01:52Z

Quoting Anne van Kesteren (2022-02-14 08:46:11)

I think it's worth updating [2]#457 (comment) to explain the setup we ended up with.

Done.

Is there a web-platform-tests PR as well?

There is not. I guess that's the next step then?

…

-Ian

annevk · 2022-02-15T07:44:51Z

@antosart can clarify that, but I would recommend at least writing some basic tests to ensure everyone is on the same page.

antosart · 2022-02-15T07:54:59Z

Yes, right. It would be great to have a basic web platform test before merging this one.

...as specified in: - w3c/webappsec-csp#457 - w3c/webrtc-extensions#81

zenhack · 2022-02-20T00:08:53Z

Opened web-platform-tests/wpt#32914; could use feedback.

annevk · 2022-02-25T13:36:49Z

Thanks! It seems like that would be best reviewed by @alvestrand or @jan-ivar or someone of their choosing. Like @antosart I'm not familiar enough with WebRTC testing.

youennf · 2022-04-14T15:05:35Z

WPT test in web-platform-tests/wpt#32914 seems ready to land.
Can we merge this PR?

jamesrez · 2022-04-15T16:29:12Z

Can someone merge this please? I need this today. Thanks

* Test webrtc/content-security-policy integration ...as specified in: - w3c/webappsec-csp#457 - w3c/webrtc-extensions#81 * Fix typos in comment. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> * Fix ice candidate exchange in CSP webrtc tests. we should be passing each candidate to the *other* pc. Tests now behave as expected. * CSP webrtc tests: listen for connection state change, not gathering. See #32914 (comment) * CSP webrtc tests: drop unnecessary stun server. * webrtc csp: simplify state checking "new" is the initial state. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

jan-ivar · 2022-04-18T21:18:06Z

I don't have merge access in this repo, but I just merged the wpt this depended on, so maybe someone else can do the honors?

antosart

It is good to merge for me. Could you just fix a few stylistic nits? (I would have done it myself, but I have no edit rights on your repo). Thanks!

antosart · 2022-04-19T06:41:10Z

index.bs

+  <h3 id="webrtc-integration">Integration with WebRTC</h3>
+
+  <p>The [=administratively-prohibited=] algorithm calls [[#should-block-rtc-connection]]
+  when invoked, and prohibits all candidates if it returns "`Blocked`."</p>


nit:

"`Blocked`".

(swap " and .)

antosart · 2022-04-19T06:43:41Z

index.bs

+                  [[#create-violation-for-global]] on |global|, |policy|, and
+                  |directive|'s <a for="directive">name</a>.
+
+              3.  Set |violation|'s <a for="violation">resource</a> to null.


nit:

`null`

(with backquotes)

antosart · 2022-04-19T06:46:13Z

index.bs

+  </div>
+
+  <h5 algorithm id="webrtc-pre-connect">
+    `webrtc` Preconnect Check


nit: Pre-connect for consistency.

antosart · 2022-04-19T06:47:07Z

index.bs

+
+  1.  If this directive's [=directive/value=] contains a single item which is an
+      <a>ASCII case-insensitive</a> match for the string "<a grammar>`'allow'`</a>",
+      return "Allowed"


nit:

"`Allowed`".

(with backquotes, and dot at the end).

annevk · 2022-04-22T10:43:16Z

@antosart weirdly enough I do have push access so I made the requested changes + one bonus.

antosart · 2022-04-22T11:16:11Z

Ok, then let's merge it.

@antosart

SHA: 7e0f637 Reason: push, by @antosart Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…2914) * Test webrtc/content-security-policy integration ...as specified in: - w3c/webappsec-csp#457 - w3c/webrtc-extensions#81 * Fix typos in comment. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> * Fix ice candidate exchange in CSP webrtc tests. we should be passing each candidate to the *other* pc. Tests now behave as expected. * CSP webrtc tests: listen for connection state change, not gathering. See web-platform-tests#32914 (comment) * CSP webrtc tests: drop unnecessary stun server. * webrtc csp: simplify state checking "new" is the initial state. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

…gration, a=testonly Automatic update from web-platform-tests Test webrtc/content-security-policy integration (#32914) * Test webrtc/content-security-policy integration ...as specified in: - w3c/webappsec-csp#457 - w3c/webrtc-extensions#81 * Fix typos in comment. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> * Fix ice candidate exchange in CSP webrtc tests. we should be passing each candidate to the *other* pc. Tests now behave as expected. * CSP webrtc tests: listen for connection state change, not gathering. See web-platform-tests/wpt#32914 (comment) * CSP webrtc tests: drop unnecessary stun server. * webrtc csp: simplify state checking "new" is the initial state. Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com> -- wpt-commits: 0abba58602758eb8be11c38788c6f51fed2529e4 wpt-pr: 32914

Merged to the webappsec-csp repo in April: w3c/webappsec-csp#457

mikewest and others added 4 commits January 17, 2018 13:38

Introduce 'webrtc-src'.

b509df0

The 'webrtc-src' directive is a proposal for handling WebRTC connections. This patch isn't exactly finished, but it should at least give a concrete proposal that we can discuss in w3c#92.

Merge remote-tracking branch 'origin/master' into webrtc-src

2c3220b

Minor conflicts wrt. new sections added; just required picking an order.

Make pre-connect checks used by webrtc a simple on/off switch

71fb900

Simplify overview description of webrtc-src

0ac40aa

There's no longer any server/peer specific logic, it's just yes or no now.

zenhack mentioned this pull request Jan 18, 2021

WebRTC can be used for exfiltration #92

Closed

annevk reviewed Jan 19, 2021

View reviewed changes

index.src.html Outdated Show resolved Hide resolved

zenhack mentioned this pull request Jan 21, 2021

Investigate the possibility to interact with data channel in workers w3c/webrtc-extensions#64

Closed

samuelweiler mentioned this pull request Jan 22, 2021

More detailed instructions for non-participants w3c/ash-nazg#198

Closed

zenhack mentioned this pull request Jan 28, 2021

Investigate the possibility to transfer MediaStreamTrack w3c/mediacapture-extensions#16

Closed

zenhack added 5 commits February 3, 2021 16:46

Don't check default-src or connect-src for webrtc.

166733f

This would be a breaking change.

Grammar: Add a missing word

cb0a72d

s/webrtc-src/webrtc/

97cb5d8

To make it more apparent that this is a stand-alone thing which doesn't inherit from anything else, per @annevk's suggestion.

Update webrtc example to use 'none'

5751132

...since we've backed out the finer grained cases.

Update build output

73d1cd1

Base automatically changed from master to main February 16, 2021 23:21

mikewest closed this Feb 18, 2021

mikewest reopened this Feb 18, 2021

jan-ivar approved these changes Feb 7, 2022

View reviewed changes

annevk approved these changes Feb 14, 2022

View reviewed changes

alvestrand mentioned this pull request Feb 15, 2022

What should the default be for the "webrtc" directive in workers? #538

Open

lgrahl mentioned this pull request Feb 15, 2022

Add support for WebRTC Data Channel in Workers w3c/webrtc-pc#230

Closed

zenhack added a commit to zenhack/wpt that referenced this pull request Feb 20, 2022

Test webrtc/content-security-policy integration

7040121

...as specified in: - w3c/webappsec-csp#457 - w3c/webrtc-extensions#81

zenhack mentioned this pull request Feb 20, 2022

Test webrtc/content-security-policy integration web-platform-tests/wpt#32914

Merged

zenhack mentioned this pull request Mar 3, 2022

Ian: Push web standards to allow blocking WebRTC sandstormports/community-project#12

Open

antosart approved these changes Apr 19, 2022

View reviewed changes

nits

9405e95

antosart merged commit 7e0f637 into w3c:main Apr 22, 2022

github-actions bot added a commit that referenced this pull request Apr 22, 2022

Introduce 'webrtc' as a simple on/off switch (#457)

c8abece

SHA: 7e0f637 Reason: push, by @antosart Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Seirdy added a commit to Seirdy/grapheneos.org that referenced this pull request Aug 6, 2022

CSP: block WebRTC

ced8439

Merged to the webappsec-csp repo in April: w3c/webappsec-csp#457

Seirdy mentioned this pull request Aug 6, 2022

CSP: block WebRTC GrapheneOS/grapheneos.org#479

Closed

Seirdy added a commit to Seirdy/grapheneos.org that referenced this pull request Aug 26, 2022

CSP: block WebRTC

01ad9e8

Merged to the webappsec-csp repo in April: w3c/webappsec-csp#457

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce 'webrtc' as a simple on/off switch #457

Introduce 'webrtc' as a simple on/off switch #457

zenhack commented Jan 18, 2021 •

edited

Loading

zenhack commented Jan 18, 2021

mozfreddyb commented Jan 21, 2021

samuelweiler commented Jan 22, 2021

zenhack commented Jan 23, 2021

samuelweiler commented Jan 25, 2021

zenhack commented Jan 26, 2021 via email

zenhack commented Feb 3, 2021

annevk commented Feb 4, 2021

zenhack commented Feb 4, 2021

sideshowbarker commented Feb 18, 2021

annevk commented Feb 18, 2021

mikewest commented Feb 18, 2021

mikewest commented Feb 18, 2021

martinthomson commented Feb 6, 2022

zenhack commented Feb 7, 2022

annevk left a comment

zenhack commented Feb 14, 2022 via email

annevk commented Feb 15, 2022

antosart commented Feb 15, 2022

zenhack commented Feb 20, 2022

annevk commented Feb 25, 2022

youennf commented Apr 14, 2022 •

edited

Loading

jamesrez commented Apr 15, 2022

jan-ivar commented Apr 18, 2022

antosart left a comment

antosart Apr 19, 2022

antosart Apr 19, 2022

antosart Apr 19, 2022

antosart Apr 19, 2022

annevk commented Apr 22, 2022

antosart commented Apr 22, 2022

Introduce 'webrtc' as a simple on/off switch #457

Introduce 'webrtc' as a simple on/off switch #457

Conversation

zenhack commented Jan 18, 2021 • edited Loading

zenhack commented Jan 18, 2021

mozfreddyb commented Jan 21, 2021

samuelweiler commented Jan 22, 2021

zenhack commented Jan 23, 2021

samuelweiler commented Jan 25, 2021

zenhack commented Jan 26, 2021 via email

zenhack commented Feb 3, 2021

annevk commented Feb 4, 2021

zenhack commented Feb 4, 2021

sideshowbarker commented Feb 18, 2021

annevk commented Feb 18, 2021

mikewest commented Feb 18, 2021

mikewest commented Feb 18, 2021

martinthomson commented Feb 6, 2022

zenhack commented Feb 7, 2022

annevk left a comment

Choose a reason for hiding this comment

zenhack commented Feb 14, 2022 via email

annevk commented Feb 15, 2022

antosart commented Feb 15, 2022

zenhack commented Feb 20, 2022

annevk commented Feb 25, 2022

youennf commented Apr 14, 2022 • edited Loading

jamesrez commented Apr 15, 2022

jan-ivar commented Apr 18, 2022

antosart left a comment

Choose a reason for hiding this comment

antosart Apr 19, 2022

Choose a reason for hiding this comment

antosart Apr 19, 2022

Choose a reason for hiding this comment

antosart Apr 19, 2022

Choose a reason for hiding this comment

antosart Apr 19, 2022

Choose a reason for hiding this comment

annevk commented Apr 22, 2022

antosart commented Apr 22, 2022

zenhack commented Jan 18, 2021 •

edited

Loading

youennf commented Apr 14, 2022 •

edited

Loading