Skip to content

+ Security and Privacy Considerations on Core and JS-API#1015

Merged
binji merged 4 commits intoWebAssembly:masterfrom
ericprud:master
May 14, 2019
Merged

+ Security and Privacy Considerations on Core and JS-API#1015
binji merged 4 commits intoWebAssembly:masterfrom
ericprud:master

Conversation

@ericprud
Copy link
Copy Markdown

@ericprud ericprud commented May 9, 2019

These sections will make it easier to transition to CR, and they are sections that folks habitually look for in specs.

ISSUES:

  1. added as a regular section in JS-API but added as an appendix in CORE. probably fine.
  2. broken refs to import and export in the JS-API Security section; should link into CORE

@rossberg
Copy link
Copy Markdown
Member

rossberg commented May 9, 2019

Hm, I think this falls squarely into the category of design considerations and rationale. There is a lot that could be said and explained in that regard, not just about security, which is just one among many topics. Therefore, the CG decided early on that the spec is not the right place to include such discussion. So far, the design docs have been the place to document such considerations. The core spec's introduction only includes a very high-level summary of the design goals, which mentions sandboxing.

@binji
Copy link
Copy Markdown
Member

binji commented May 9, 2019

This is a common section in w3 technical reports, see for example Pointer events, Accelerometer, Web Audio, etc.

Are you suggesting we have a link to the design docs in the spec document? Or that we omit this section entirely? Or perhaps reword the contents?

@rossberg
Copy link
Copy Markdown
Member

It might be more adequate to move it to the Web API spec, because it seems specifically relevant to the web environment. For example, I can’t tell how privacy is even a meaningful consideration for an ISA on its own.

But on second thought I agree that it also might be helpful to briefly explain the basic security model enabled by Wasm and that it's mostly up to the embedder. This is closely related to the section on "Scope" in intro/introduction.rst, so introducing a new subsection right after that would be the most natural place (it seems a little short for its own appendix anyway). May I suggest the following wording:

.. index:: ! security, host, embedder,  module, function, import
.. _security:

Security Considerations
~~~~~~~~~~~~~~~~~~~~~~~

WebAssembly provides no ambient access to the computing environment in which code is executed.
Any interaction with the environment, such as I/O, access to resources, or operating system calls, can only be performed by invoking :ref:`functions <function>` provided by the :ref:`embedder <embedder>` and imported into a WebAssembly :ref:`module <module>`.
An embedder can establish security policies suitable for a respective environment by controlling or limiting which functional capabilities it makes available for import.
Such considerations are an embedder’s responsibility and the subject of :ref:`API definitions <scope>` for a specific environment.

Because WebAssembly is designed to be translated into machine code running directly on the host's hardware, it is potentially vulnerable to side channel attacks on the hardware level.
In environments where this is a concern, an embedder may have to put suitable mitigations into place to isolate WebAssembly computations.

(Intentionally leaves out Privacy as well as singling out specific embeddings or proposals.)

@Ms2ger
Copy link
Copy Markdown
Collaborator

Ms2ger commented May 10, 2019

It's not just common in W3C specs; it's formally required to enter the CR stage: https://www.w3.org/pubrules/doc/rules/?profile=CR#document-body

@rossberg
Copy link
Copy Markdown
Member

@Ms2ger, well, that specifies a "should", notably not a "must". :) The points and questions listed in the guidelines are very specific to the Web context and mostly inapplicable to the Wasm core spec.

So one can argue that the Web API spec is the right place to satisfy this criterion at large. But I agree that a generic statement about security makes sense in the core spec.

@binji
Copy link
Copy Markdown
Member

binji commented May 10, 2019

This security section looks good to me, @ericprud what do you think? I agree that many of the w3 checklists are difficult to apply to the core spec: I like to tell people "wasm can't really do anything." :)

@ericprud
Copy link
Copy Markdown
Author

ericprud commented May 12, 2019

I updated the PR with @rossberg's suggestions:

@@ -1,10 +1,15 @@
+.. index:: ! security, host, embedder,  module, function, import
-.. _security-considerations:
+.. _security:
 
 Security and Privacy Considerations
 -----------------------
 
-On its own, WebAssembly provides no access to the native computing environment; all I/O is performed by marshaling data through functions exposed by the :ref:`import <syntax-import>` and :ref:`export <syntax-export>` operators.
-An implementation could provide access to native libraries by e.g. pre-defining standard functions.
-Such a system would have to define its security and privacy behavior.
+WebAssembly provides no ambient access to the computing environment in which code is executed.
+Any interaction with the environment, such as I/O, access to resources, or operating system calls, can only be performed by invoking :ref:`functions <function>` provided by the :ref:`embedder <embedder>` and imported into a WebAssembly :ref:`module <module>`.
+An embedder can establish security policies suitable for a respective environment by controlling or limiting which functional capabilities it makes available for import.
+Such considerations are an embedder’s responsibility and the subject of :ref:`API definitions <scope>` for a specific environment.
+
+Because WebAssembly is designed to be translated into machine code running directly on the host's hardware, it is potentially vulnerable to side channel attacks on the hardware level.
+In environments where this is a concern, an embedder may have to put suitable mitigations into place to isolate WebAssembly computations.
+
-As of the data of publication, the only host binding is the :ref:`Javascript Interface js-api`, which states how WebAssembly interacts with the web environment.
+As of the data of publication, the only embedder is the :ref:`Javascript Interface js-api`, which states how WebAssembly interacts with the web environment.
-Future versions will provide threads which can be used to create a high-resolution clock, which can be used for timing-based attacks (e.g. spectre).

I kept the "Privacy" part of the heading because:

  1. it's a handy convention and a lot of people look for it during and after the transition to REC.
  2. the description applies equally to privacy and security.

I kept a reference to js-api because I think that's what I think almost all readers will be looking for. If I understood how to do links into js-api, I'd add a link to that security section.

I moved the file up one level (hopefully correctly) so it would appear just before Conformance.

@rossberg, are you convinced by my arguments for "and Privacy" and the ref from core to js-api?

@rossberg
Copy link
Copy Markdown
Member

I kept the "Privacy" part of the heading because:

  • it's a handy convention and a lot of people look for it during and after the transition to REC.
  • the description applies equally to privacy and security.

I don't see it saying or implying anything about privacy, nor can it. This document defines a virtual ISA, nothing else. An ISA is orthogonal to questions of privacy -- in the same way that no CPU manual could meaningfully talk about privacy.

I kept a reference to js-api because I think that's what I think almost all readers will be looking for. If I understood how to do links into js-api, I'd add a link to that security section.

I understand, but please remove it anyway. So far, the core spec -- very consciously -- avoids any reference to a specific embedder spec anywhere. For one, to underline that all embedders are equal and none has specific privileges, and perhaps also to avoid circular dependencies between documents. (FWIW, that ref won't work, it's just gonna be an undefined xref in Sphinx.)

I moved the file up one level (hopefully correctly) so it would appear just before Conformance.

Ah, we shouldn't create any rst files besides index.rst at the toplevel. Can you integrate the text into the existing intro/introduction.rst, such that it appears as a section after Scope? I think that's the most logical place for it.

@binji
Copy link
Copy Markdown
Member

binji commented May 13, 2019

I've added an agenda item to discuss this tomorrow.

Comment thread document/core/index.rst Outdated
Comment thread document/js-api/index.bs Outdated
Copy link
Copy Markdown
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

Thanks!

@binji binji merged commit 5baa425 into WebAssembly:master May 14, 2019
binji added a commit to WebAssembly/threads that referenced this pull request Jan 17, 2020
* [spec] Fix definition of littleendian (#968)

* Fix some test function names that got out of sync with the recent instruction renaming (#964)

* [interpreter] Fix broken link in README (#970)

* [test] More tests for missing operand missing -Part2 (#950)

* [test] More tests for missing operands -Part3 (#971)

* [test] Move tests in typecheck.wast to appropriate files (#973)

* [jsapi] update limits.any.js (#975)

Caches the generated Wasm buffer instead of re-generating
it for each test.

Refs WebAssembly/spec#972

* [jsapi] Fix typo in limits.any.js (#978)

mininum -> minimum

* [test] Add binary test for local count of zero (#980)

* [jsapi] Update wasm-module-builder.js

This speeds up the WasmModuleBuilder enormously. It follows this V8-side
CL: https://crrev.com/c/1508352
It also adopts a few other minor changes to the wasm-module-builder.js
file from the v8 side.

* [jsapi] Fix tests after wasm-module-builder.js update

Updating the wasm-module-builder.js broke the jsapi tests. I was under
the impression that travis would run them, but it seems like this is
not the case. Rolling the updated version to v8 failed then:
https://crrev.com/c/1516684

This fixes two things:
1) {toBuffer} returns a Uint8Array as before, not an ArrayBuffer,
2) {addExplicitSection} expects a byte sequence, hence pass the
   truncated buffer instead of the {Binary} object.

* Fix store handling in 'instantiate a WebAssembly module'. (#981)

The store variable was not defined before it was used, and the result was not
saved.

* Editorial: Stop breaking lists to insert a note.

* Editorial: Extract a "read the imports" algorithm.

* Editorial: Extract a "create an instance object" algorithm.

* Editorial: Remove an unnecessary local variable in "create an instance object".

* Editorial: Extract an "instantiate the core of a WebAssembly module" algorithm.

* Editorial: Extract an "asynchronously instantiate a WebAssembly module" algorithm.

* Fix `if` instruction type in index (#987)

Fixes #986.

* Normative: Read the imports synchronously in WebAssembly.instantiate(Module).

* Update index.bs

* Update index.bs

* Update index.bs

* Update index.bs

* Update index.bs

* Update index.bs

* Update index.bs

* Reject the promise if reading the imports fails.

I missed this in #988.

* [test] Check for "unreachable" consistently (#992)

The full error message generated by the interpreter is "unreachable executed", but all of the other tests check for just "unreachable".

* [interpreter] Fix in JS conversion

* [test] Add test when start function traps (#994)

The start function is invoked after the store has been modified. So if a
start function traps, all data and element segments will have been
copied into the memory and table respectively. This means that functions
can be invoked on this instance, even though the instance is not
available to the embedder directly.

* [interpreter] Check argument types on invoke

* Update instructions.rst (#995)

use `local.get` name instead of `get_local` which isn't used elsewhere.

* [test] Add dedicated test for memory.size (#997)

* [spec] Add paragraph break for more emphasis (#1001)

* [test] Alignment and offset with overlong leb128 (#998)

* [spec] More precise Unicode terminology (#1002)

* [spec] Pre/post-conditions and some renamings in embedding interface (#1003)

* [spec] Work around Sphinx/Latex issue (#1004)

* [spec] Fix off-by-one error in table grow size limit (#1005)

* [interpreter] Group digits with '_' when printing numbers (#1006)

* [spec] Fix typo (#1011)

* Fix bikeshed build (#1014)

A recent change caused the generated html to have a sequence that looked
like `{{abkasdfadsf}}`, which bikeshed misinterprets as a WebIDL markup
shorthand. Since we never use that shorthand in the core spec, we can
disable it.

Since bikeshed thought this was a IDL reference, it inserted an
additional `<code>` tag, so the `mathjax2katex.py` script was not able
to extract the math blocks properly. The `FindMatching` function was
raising an `IndexError`, but it was never caught and instead terminated
the worker thread. The main thread would then wait forever for a worker
thread that will never finish. The fix here is to catch all exceptions
and print an error, instead of just catching `KeyboardInterrupt` and
`AssertionError`.

* [spec] Typo in type table (#1013)

* [spec] Constrain name section ordering (#1012)

* [test] More tests for non-minimal LEB128 (#1007)

* [test] More tests for overlong LEB128 (#1016)

* [spec] Fix ToC of Appendix in w3c version (#1017)

It seems Sphinx and ReST allow you to use any underline pattern for
sections, but assign their number ordinally as they're encountered.

So when PR #1003 intrdouced a new section in A.1 Embedding using "...",
it becomes `<h4>`, and changes the meaning of the other sections,
creating a strange ToC.

* + Security and Privacy Considerations on Core and JS-API (#1015)

* ~ reflect (rossberg's suggestions)[WebAssembly/spec#1015 (comment)]

* ~ reflect [WebAssembly/spec#1015 (comment)]

* ~ reflect binji's fixes

[https://github.com/WebAssembly/spec/pull/1015/files/122a9483ef1b1e7e7138a51a8711604d45ec1f4b#r283588141]
[https://github.com/WebAssembly/spec/pull/1015/files/122a9483ef1b1e7e7138a51a8711604d45ec1f4b#r283588352]

* [spec] Replace URLs with bikeshed biblio refs (#1018)

This will automatically add these entries to the normative references
section which is auto-generated at the bottom of the document.

* [test] More LEB128 tests (#1019)

* [spec][js-api] Fix some links (#1020)

Replace `init_store` with `store_init`.

Also, reword the "Security and Privacy Considerations" section, and fix
the references are using ReST syntax (which doesn't work in bikeshed).

* [spec] Tweak wording (#966)

* [spec] Address feedback on section 4 (#1022)

* [interpreter] Fix edge cases for f32_convert_i64 (#1021)

* Editorial: Remove links from Number, Object when checking types

This follows the convention followed by the JavaScript spec and HTML.

Fixes #1026

* [test/interpreter] Rounding edge cases for float literals (#1025)

* Remove extraneous copyright from bikeshed document (#1030)

* [test] Fix unintended errors in negative tests (#1034)

* [interpreter] Fix broken link (#1035)

* [test] More inconsistent lengths (#1029)

for type, import, export, table, memory, global, element/data segment,
and br_table.

* [js-api] Remove incorrect note about object caching. (#1036)

Fixes #985.

* [js-api] Use IDL conventions around ambient values. (#1037)

* [interpreter] Fix typo in comment (#1045)

* [spec] Fix header level (#1047)

* [interpreter] Downgrade to Ocaml 4.02 (#1044)

* [spec] Upgrade to IEEE 754-2019 (#1050)

* [spec] Terminology nits (#1053)

* [interpreter] Make format roundtrips perfect (#1057)

* [interpreter] Update BS support (#1058)

* [interpreter] Tweak  target

* [interpreter] Simplify wast.js build

* [interpreter] Add `assert_exhaustion` to README (#1061)

* [spec] Fix the katex build on Travis. (#1062)

It was broken due to the accidental submodule update in #1034.

* Rewrite definition of error types.

This at least gets somewhat close to defining what's actually intended.

Fixes #810.

* [js-api] Editorial: Clarify the host function algorithms a bit. (#1063)

* [js-api] Editorial: Update reference to 'resolve'. (#1065)

* [spec] Fix float text format (#1069)

* [test] Don't require the element index in "uninitialized element" trap strings. (#1076)

One test in elem.wast expects the trap message "uninitialized element 7",
which requires wasm runtimes to be able to print the element index. This
is obviously nice for humans, but can be inconvenient to implement in wasm
implementations which use signals to implement traps, as it requires special
code to preserve the element index.

Other tests eg. in imports.wast don't include the number in the expected
message.

It seems better to allow implementations to decide for themselves
whether to print the index number, rather than having it be an outright
requirement of the spec test.

* Fix error in build.py when test compilation fails

convert_wast_to_js uses multiprocessing to parallelize compilation of
wast test cases. When an error happens, the logging code expects the
result to be a process exit code and a CompletedProcess instance.

This isn't possible, so this commit updates the result to be a
CompletedProcess and modifies the code to manually check the exit code.

* [spec] Fix typo (determinsitically) (#1079)

* Add OOPSLA paper

* [build] Try to reduce output spam from Latex built

* Editorial: Update editor information in JS/Web API specifications.

I will be taking over Dan's duties as editor.

* Add tests for multiple start sections (#1092)

* [spec] fix valid tableinst and meminst premises in appendix (#1093)

* Dont expect indices in validation messages either. (#1089)

Similar to #1076, don't include index numbers in expected error messages
from validation. This allows implementations to avoid creating
dynamically formatted strings for validation error messages. Admittedly
this isn't a huge burden, but it does seem like something that shouldn't
be required to pass the spec test.

* Miscellaneous tests (#1090)

* Test that INT_MIN/0 gets a divide-by-zero trap.

* Test Unicode characters that differ with NFC, NFD, NFKC, and NFKD.

There are three characters whose normalization forms under NFC, NFD, NFKC,
and NFKD are all different. Test them.

* Add a testcase for unreachable in an if-then with no else.

* Fix some assert_malformed module tests to ONLY include malformed syntax

* [test] fix a typo (#1098)

* [spec] Fix typo (#1099)

* [interpreter] Cosmetic tweak to return reduction (#1101)

according to
https://webassembly.github.io/spec/core/exec/instructions.html#exec-return
the `Return` instruction pops the top of the stack, until the next
frame. The equivalent in the reference interpreter should be to set
the value stack to `[]`. This is what is happening for `Break` as well.
So for consistency, also do it here.

This has no visible effect: If the first instruction is `Returning`, the
stack is ignored.

* [test] Long argument lists (#1105)

* Fix cross-reference for list/is empty.

* [wasm-js-api-1] Align with Web IDL specification (#1083)

* Remove a year from the ECMAScript Language title in normative reference

The URL points to latest spec draft so it should be just ECMAScript and not ES2018. This also make it consistent with [WebAssembly JS API Normative Reference](https://www.w3.org/TR/2019/PR-wasm-js-api-1-20191001/#normative)

* [js-api] Correct 'append' cross-references.

* [interpreter] Unify assert_result* assertions (#1104)

Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org>
Co-authored-by: Andrew Scheidecker <andrew@scheidecker.net>
Co-authored-by: Junior Rojas <jrojasdavalos@gmail.com>
Co-authored-by: Wanming Lin <wanming.lin@intel.com>
Co-authored-by: Sven Sauleau <github@sauleau.com>
Co-authored-by: Clemens (Hammacher) Backes <post@clemens-backes.de>
Co-authored-by: Ms2ger <Ms2ger@gmail.com>
Co-authored-by: Daniel Ehrenberg <littledan@chromium.org>
Co-authored-by: Søren Sjørup <zorren@gmail.com>
Co-authored-by: Galaxtone <galaxtone.plays.minecraft@gmail.com>
Co-authored-by: Eric Prud'hommeaux <eric+github@w3.org>
Co-authored-by: Erik McClure <erikm@blackspherestudios.com>
Co-authored-by: Rikard Hjort <rikard.hjort@gmail.com>
Co-authored-by: NDTSTN <indietasten@gmail.com>
Co-authored-by: Dan Gohman <sunfish@mozilla.com>
Co-authored-by: Ryan Hunt <rhunt@eqrion.net>
Co-authored-by: Xuan Huang  <huxpro@gmail.com>
Co-authored-by: Petr Penzin <41971413+penzn@users.noreply.github.com>
Co-authored-by: Jeremy Ruten <jeremy.ruten@gmail.com>
Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>
Co-authored-by: Rian Hunter <rianhunter@users.noreply.github.com>
Co-authored-by: autokagami <saschanaz+autokagami@outlook.com>
Co-authored-by: Sergey Rubanov <chi187@gmail.com>
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.

4 participants