Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimal specification of 'wasm-unsafe-eval' source directive #293

Merged
merged 29 commits into from
Oct 18, 2021
Merged
Changes from 21 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
00a7510
Start working on wasm-eval spec
eholk Feb 16, 2018
26d792b
Make WebAssembly.compile gating optional
eholk Feb 16, 2018
7363f1b
Allow unsafe-eval to allow wasm
eholk Feb 16, 2018
0a5fe12
Remove MAY, add remaining Wasm methods
eholk Mar 9, 2018
d2b0084
Describe HostEnsureCanComileWasmBytes
eholk Mar 9, 2018
538f3d1
Start working on Resource-driven Wasm hook
eholk Mar 9, 2018
8b96f47
Start working on wasm-eval spec
eholk Feb 16, 2018
8bcd2b3
Make WebAssembly.compile gating optional
fgmccabe Jul 2, 2021
1211be0
Continuing merge/rebase
fgmccabe Jul 2, 2021
047cbde
Start working on Resource-driven Wasm hook
eholk Mar 9, 2018
5096a54
Merge branch 'wasm-eval' of github.com:fgmccabe/webappsec-csp into wa…
fgmccabe Jul 8, 2021
651a3ce
Merge branch 'w3c:main' into wasm-eval
fgmccabe Jul 8, 2021
536a063
Removed extraneous index.html
fgmccabe Jul 8, 2021
88a984d
Responding to reviewer's remarks
fgmccabe Jul 9, 2021
aca2f96
Put in references to new Wasm JS API algorithms
fgmccabe Jul 9, 2021
f409c82
Removed second algorithm around HostEnsureCanCompileWasmBytesResponse
fgmccabe Jul 16, 2021
9eab24f
Some polishing ...
fgmccabe Aug 2, 2021
bcbbd70
Add item that allows instantiateStreaming and compileStreaming
fgmccabe Aug 2, 2021
ad51c7a
Fix typo
fgmccabe Aug 3, 2021
b4d31ba
Added specific language to handle WebAssembly.compileStreaming
fgmccabe Aug 26, 2021
54a1dd8
Remove reference to special handling for instantiateStreaming
fgmccabe Sep 9, 2021
7d106c4
Some typos & reflecting reviewers feedback
fgmccabe Sep 13, 2021
8cb258c
Revert editorial change
fgmccabe Sep 14, 2021
fc1ea9d
Adjust language referencing byte sequences and bytes
fgmccabe Sep 15, 2021
fa252aa
Merge branch 'main' into wasm-eval
antosart Sep 16, 2021
4638f68
Modified conditions governing WebAssembly to include unsafe-eval
fgmccabe Sep 30, 2021
7233eb3
Changed violation resource to wasm-eval
fgmccabe Oct 5, 2021
278873b
Added wasm-eval to list of violation resources
fgmccabe Oct 6, 2021
5b3c2a4
Remove references to |bytes|
antosart Oct 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 88 additions & 7 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,21 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
type: grammar
text: ASCII whitespace; url: ascii-whitespace
text: INFRA; url: #

spec: WebAssembly-js-api; urlPrefix: https://webassembly.github.io/spec/js-api/
type: method
text: new WebAssembly.Module(); url: #dom-module-module
text: WebAssembly.compile(); url: #dom-webassembly-compile
text: WebAssembly.instantiate(); url: #dom-webassembly-instantiate
text: HostEnsureCanCompileWasmBytes(); url:#dom-host-ensure-can-compile-wasm-bytes

spec: WebAssembly-web-api-api; urlPrefix: https://webassembly.github.io/spec/web-api/
type: method
text: WebAssembly.compileStreaming(); url: #dom-webassembly-compilestreaming
text: WebAssembly.instantiateStreaming(); url: #dom-webassembly-instantiatestreaming
type: exception
text: WebAssembly.CompileError; url: #exceptiondef-compileerror

</pre>
<pre class="biblio">
{
Expand Down Expand Up @@ -665,6 +680,7 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
<dfn>keyword-source</dfn> = "<dfn>'self'</dfn>" / "<dfn>'unsafe-inline'</dfn>" / "<dfn>'unsafe-eval'</dfn>"
/ "<dfn>'strict-dynamic'</dfn>" / "<dfn>'unsafe-hashes'</dfn>" /
/ "<dfn>'report-sample'</dfn>" / "<dfn>'unsafe-allow-redirects'</dfn>"
/ "<dfn>'wasm-unsafe-eval'</dfn>"

ISSUE: Bikeshed `unsafe-allow-redirects`.

Expand Down Expand Up @@ -878,10 +894,11 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
The `Content-Security-Policy-Report-Only` HTTP Response Header Field
</h3>

The <dfn export id="header-content-security-policy-report-only" http-header>`Content-Security-Policy-Report-Only`</dfn>
HTTP response header field allows web developers to experiment with policies by monitoring (but
not enforcing) their effects. The header's value is represented by the following ABNF
[[!RFC5234]]:
The <dfn export id="header-content-security-policy-report-only"
http-header>`Content-Security-Policy-Report-Only`</dfn> HTTP response header
field allows web developers to experiment with policies by monitoring (but not
enforcing) their effects. The header's value is represented by the following
ABNF [[!RFC5234]]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it doesn't seem like anything actually changed here so it would probably be better to undo this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this because the existing line length was over 100 characters. Ok with not doing that though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if editorial changes happened in their own commit. Makes using blame easier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this


<pre>
Content-Security-Policy-Report-Only = 1#<a grammar>serialized-policy</a>
Expand Down Expand Up @@ -1418,8 +1435,62 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
ISSUE(tc39/ecma262#938): {{HostEnsureCanCompileStrings()}} does not include the string which is
going to be compiled as a parameter. We'll also need to update HTML to pipe that value through
to CSP.
</section>

<h3 id="wasm-integration">Integration with WebAssembly</h3>

WebAssembly defines the {{HostEnsureCanCompileWasmBytes()}} abstract operation
which allows the host environment to block the compilation of WebAssembly
sources into executable code. This document defines an implementation of this
abstract operation which examines the relevant <a for="global object">CSP
list</a> to determine whether such compilation ought to be blocked.

<h4 id="can-compile-wasm-bytes" algorithm dfn>
EnsureCSPDoesNotBlockWasmByteCompilation(|callerRealm|, |calleeRealm|, |bytes|)
</h4>

Given two <a>realms</a> (|callerRealm| and |calleeRealm|), and (|bytes|),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should specify the type of |bytes| (I guess its a byte sequence)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual algorithm does not mention the byte sequence. Thinking about that more, I believe that it cannot in typical realistic scenarios. (The policy is about whether wasm is allowed, not this particular wasm module).
So, should we just remove the reference to |bytes|/|byte sequence|?

this algorithm returns normally if compilation is allowed, and throws a
{{WebAssembly.CompileError}} if not:

1. Let |globals| be a list containing |callerRealm|'s [=Realm/global object=] and |calleeRealm|'s
[=Realm/global object=].

2. For each |global| in |globals|:

1. Let |result| be "`Allowed`".

2. For each |policy| in |global|'s [=global object/CSP list=]:

1. Let |source-list| be `null`.

2. If |policy| contains a [=directive=] whose [=directive/name=] is "`script-src`", then
set |source-list| to that [=directive=]'s [=directive/value=].

Otherwise if |policy| contains a [=directive=] whose [=directive/name=] is
"`default-src`", then set |source-list| to that directive's [=directive/value=].

3. If |source-list| is non-`null`, and does not contain a [=source expression=] which is
an [=ASCII case-insensitive=] match for the string "<a grammar>`'wasm-unsafe-eval'`</a>",
antosart marked this conversation as resolved.
Show resolved Hide resolved
then:

1. Let |violation| be the result of executing [[#create-violation-for-global]] on
|global|, |policy|, and "`script-src`".

2. Set |violation|'s [=violation/resource=] to "`inline`".
antosart marked this conversation as resolved.
Show resolved Hide resolved

3. If |source-list| [=list/contains=] the expression
"<a grammar>`'report-sample'`</a>", then set |violation|'s [=violation/sample=] to
a string consisting of the first 40 bytes of |bytes| encoded using [=base64 encoding=].

4. Execute [[#report-violation]] on |violation|.

5. If |policy|'s [=policy/disposition=] is "`enforce`", then set |result| to
"`Blocked`".

3. If |result| is "`Blocked`", throw a {{WebAssembly.CompileError}} exception.

</section>

<!-- Big Text: Reporting -->
<section>
<h2 id="reporting">
Expand Down Expand Up @@ -2657,7 +2728,7 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
as in most situations there is no particular reason to have separate lists of
permissions for inline event handlers and <{script}> elements.

The `script-src` directive governs five things:
The `script-src` directive governs seven things:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't they six?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. Ack.


1. Script <a for="/">requests</a> MUST pass through [[#should-block-request]].

Expand All @@ -2684,7 +2755,17 @@ spec: INFRA; urlPrefix: https://infra.spec.whatwg.org/
and <a>`script-src-elem`</a> are not used when performing this check, instead
`script-src` (or it's fallback directive) is always used.

5. Navigation to `javascript:` URLs MUST pass through [[#script-src-inline]].
5. The following WebAssembly execution sinks are gated on the "`wasm-unsafe-eval`"
source expression:

* {{new WebAssembly.Module()}}
antosart marked this conversation as resolved.
Show resolved Hide resolved
* {{WebAssembly.compile()}}
* {{WebAssembly.compileStreaming()}}
* {{WebAssembly.instantiate()}}
* {{WebAssembly.instantiateStreaming()}}

6. Navigation to `javascript:` URLs MUST pass through [[#should-block-inline]]. Such navigations
will only execute script if every policy allows inline script, as per #3 above.

<h5 algorithm id="script-src-pre-request">
`script-src` Pre-request check
Expand Down