Skip to content

Merge CSP into main#2198

Open
fgmccabe wants to merge 83 commits into
mainfrom
csp
Open

Merge CSP into main#2198
fgmccabe wants to merge 83 commits into
mainfrom
csp

Conversation

@fgmccabe

Copy link
Copy Markdown
Collaborator

This (should) reflect changes needed to incorporate phase 4 csp.

This should probably NOT just be merged, as the change seems to include notes that might not normally be part of the spec.

flagxor and others added 30 commits November 1, 2017 10:12
Provide background on CSP threat model and use cases
Pull in latest changes from WebAssembly/spec repo
Rename 'wasm-eval' to 'wasm-unsafe-eval' Fixes #15.
Remove reference to mimetype hardening
Always allow WebAssembly.validate()
Summarize API behavior and threats relative to CSP
ngzhian and others added 26 commits April 18, 2022 11:38
At yesterday's WG meeting, we decided to make a new release, now switching to the Evergreen model. For administrative and technical reasons having to do with W3C procedure, we decided to bump the release number to 2.

From now on, the standard will iterate at version 2 from the W3C's official perspective. We use minor release numbers internally to distinguish different iterations.

(@ericprud, I hope I understood correctly that the Bikeshed "level" also needed to be bumped to 2.)
Merge from upstream wasm 3.0

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are there expected to be any tests here?

Comment thread document/web-api/index.bs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the deletions here expected? It seems it would be better to edit the "Security and Privacy Considerations" section rather than remove it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes we should keep this.

Comment thread README.md Outdated
[![CI for specs](https://github.com/WebAssembly/spec/actions/workflows/ci-spec.yml/badge.svg)](https://github.com/WebAssembly/spec/actions/workflows/ci-spec.yml)
[![CI for interpreter & tests](https://github.com/WebAssembly/spec/actions/workflows/ci-interpreter.yml/badge.svg)](https://github.com/WebAssembly/spec/actions/workflows/ci-interpreter.yml)

# Content Security Policy Proposal for WebAssembly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You'll want to undo the changes to the README.

Removed the Content Security Policy proposal section from the README.

@eqrion eqrion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd also be interested to know if there are tests for this already in WPT.

Comment thread document/web-api/index.bs

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes we should keep this.

Comment thread document/js-api/index.bs
Comment on lines +2212 to +2221
<h3 id="host-ensure-can-compile-wasm-bytes">HostEnsureCanCompileWasmBytes()</h3>

<dfn abstract-op>HostEnsureCanCompileWasmBytes()</dfn> is an implementation-defined abstract operation
that allows host environments to block certain WebAssembly functions which
compile bytes into WebAssembly code.

An implementation of HostEnsureCanCompileWasmBytes may complete normally or
abruptly. Any abrupt completions will be propagated to its callers. The default
implementation of HostEnsureCanCompileWasmBytes is to unconditionally return an
empty normal completion.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The concrete instantiation of this abstract operation requires a realm to be passed in [1]. I think we need to have that be an operand and explicitly provide it. We can probably just do Let |realm| be the [=current Realm=] in the right places.

Also CSP calls it "EnsureCSPDoesNotBlockWasmByteCompilation" not HostEnsureCanCompileWasmBytes. Not sure if that's an issue.

[1] https://w3c.github.io/webappsec-csp/#can-compile-wasm-bytes

Comment thread document/js-api/index.bs

<div algorithm>
To <dfn>compile a WebAssembly module</dfn> from source bytes |bytes|, perform the following steps:
1. Perform [$HostEnsureCanCompileWasmBytes()$].

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will happen when you call WebAssembly.validate. Is that intentional? It's also called by "asynchronously compile a WebAssembly module" in the "in parallel" section. My understanding is that's not allowed because it implies there could be racy accesses to the realm to perform the CSP check.

I think we should remove this check here and move it to the JS-API entry points that need it. So the WebAssembly.Module constructor, WebAssembly.compile, WebAssembly.instantiate, and WebAssembly.instantiateStreaming.

Comment thread document/js-api/index.bs

<div algorithm>
To <dfn>compile a WebAssembly module</dfn> from source bytes |bytes|, perform the following steps:
1. Perform [$HostEnsureCanCompileWasmBytes()$].

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Perform [$HostEnsureCanCompileWasmBytes()$].
1. Perform [=?=] [$HostEnsureCanCompileWasmBytes()$].

I believe we can use the ReturnIfAbrupt macro here. We use it elsewhere in the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants