wasm: capability restriction #13911

ryanapilado · 2020-11-05T07:56:51Z

Signed-off-by: Ryan Apilado [email protected]

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Commit Message: wasm: implement capability restriction
Additional Description: Defines the configuration for capability restriction and propagates the configuration to proxy-wasm-cpp-host.
Risk Level: Medium
Testing: Unit tests (wasm_test.cc), configuration tests (config_test), and integration tests (wasm_filter_test.cc). Test that capabilities can be restricted selectively and are unrestricted by default.
Docs Changes: Add documentation for the new messages created in wasm.proto.
Release notes: N/A

antoniovicente · 2020-11-09T22:19:12Z

Thanks for the additional tests. What is missing before this PR is ready for review?

repokitteh-read-only · 2020-11-18T22:47:12Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #13911 was synchronize by ryanapilado.

see: more, trace.

Signed-off-by: Ryan Apilado <[email protected]>

PiotrSikora

cc @mathetake @kyessenov

test/extensions/filters/network/wasm/BUILD

test/extensions/filters/network/wasm/config_test.cc

test/extensions/common/wasm/wasm_test.cc

api/envoy/extensions/wasm/v3/wasm.proto

Signed-off-by: Ryan Apilado <[email protected]>

htuch · 2020-11-20T00:27:46Z

api/envoy/extensions/wasm/v3/wasm.proto

@@ -18,6 +18,12 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: Wasm]
 // [#extension: envoy.bootstrap.wasm]

+// Configuration for restricting proxy-wasm capabilities available to modules.
+message CapabilityRestrictionConfig {
+  // The list of proxy-wasm capabilities which will be exposed to the module.


Can you add a reference to where the capabilities are defined?

Added a reference and updated the docs.

Signed-off-by: Ryan Apilado <[email protected]>

mathetake

Looks good now. Thanks

bazel/repository_locations.bzl

Signed-off-by: Ryan Apilado <[email protected]>

ryanapilado · 2021-01-13T07:15:30Z

/retest

repokitteh-read-only · 2021-01-13T07:15:35Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13911 (comment) was created by @ryanapilado.

see: more, trace.

PiotrSikora · 2021-02-01T09:25:25Z

api/envoy/extensions/wasm/v3/wasm.proto

+  // and so should be left empty.
+  //
+  // The capability names are given in the
+  // `proxy-wasm ABI <https://github.com/proxy-wasm/spec/tree/master/abi-versions/vNEXT>`_.


Nit: s/proxy-wasm/Proxy-Wasm/

PiotrSikora · 2021-02-01T09:25:39Z

api/envoy/extensions/wasm/v3/wasm.proto

@@ -18,6 +18,26 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: Wasm]
 // [#extension: envoy.bootstrap.wasm]

+// Configuration for restricting proxy-wasm capabilities available to modules.
+message CapabilityRestrictionConfig {
+  // The proxy-wasm capabilities which will be allowed. Capabilities are mapped by


Nit: s/proxy-wasm/Proxy-Wasm/

PiotrSikora · 2021-02-01T09:26:00Z

api/envoy/extensions/wasm/v3/wasm.proto

+  map<string, SanitizationConfig> allowed_capabilities = 1;
+}
+
+// Configuration for sanitization of inputs to an allowed capability. NOTE: This is currently unimplemented.


Nit: move NOTE to a new line.

Misunderstood this, fixed.

PiotrSikora · 2021-02-01T09:27:20Z

source/extensions/common/wasm/wasm.cc

-static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr& plugin,
-                               const Stats::ScopeSharedPtr& scope,
+static bool createWasmInternal(const VmConfig& vm_config,
+                               const CapabilityRestrictionConfig& cr_config,


Nit: s/cr_config/capabilities_config, otherwie variable isn't self-describing.

Fixed, changed to capabiility_restriction_config for consistency with type name and proto. Can change to capabilities_config if too long.

PiotrSikora · 2021-02-01T09:29:10Z

test/extensions/common/wasm/test_data/test_restriction_cpp.cc

+WASM_EXPORT(uint32_t, proxy_on_vm_start, (uint32_t context_id, uint32_t configuration_size)) {
+  (void)(context_id);
+  (void)(configuration_size);
+  std::string level_message = "after on_vm_start, before proxy_log";


Nit: s/level_message/log_message/

Also, this message is written as part of proxy_log, not before proxy_log.

PiotrSikora · 2021-02-01T09:34:29Z

test/extensions/common/wasm/wasm_test.cc

+  auto context = std::make_unique<TestContext>(wasm.get());
+  EXPECT_TRUE(wasm->initialize(code, false));
+
+  // on_vm_start will trigger proxy_log, but expect no call because both are restricted


Should proxy_on_vm_start callback be even called considering that is not allowed? This is special case, since if we don't call proxy_on_vm_start then there is a high chance that plugin won't work correctly, but other callbacks should be skipped if they are not in the allowed capabilities.

Yeah, the comment is incorrect, proxy_on_vm_start will never run in this case. Fixed. I also added a different callback to the wasm file (proxy_on_context_create) and adjusted the tests to test for it.

Signed-off-by: Ryan Apilado <[email protected]>

ryanapilado · 2021-02-03T06:32:58Z

/retest

repokitteh-read-only · 2021-02-03T06:33:01Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13911 (comment) was created by @ryanapilado.

see: more, trace.

ryanapilado · 2021-02-03T16:35:54Z

/retest

repokitteh-read-only · 2021-02-03T16:35:58Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13911 (comment) was created by @ryanapilado.

see: more, trace.

Signed-off-by: Ryan Apilado <[email protected]>

PiotrSikora

Thanks!

@lizan could you take a pass?

lizan · 2021-02-09T00:31:11Z

@envoyproxy/dependency-shepherds PTAL

moderation · 2021-02-09T01:49:12Z

/lgtm deps

ryanapilado mentioned this pull request Nov 5, 2020

Implement a capability restriction system. proxy-wasm/proxy-wasm-cpp-host#89

Merged

ryanapilado force-pushed the capability-restriction-tests branch 2 times, most recently from b6baa97 to adc6b26 Compare November 5, 2020 16:13

mattklein123 assigned PiotrSikora Nov 18, 2020

ryanapilado force-pushed the capability-restriction-tests branch from adc6b26 to 03d1ffb Compare November 18, 2020 22:47

repokitteh-read-only bot added the api label Nov 18, 2020

ryanapilado changed the title ~~[WIP] add tests for proxy-wasm capability restriction~~ [WIP] proxy-wasm abi restriction Nov 18, 2020

ryanapilado changed the title ~~[WIP] proxy-wasm abi restriction~~ [WIP] proxy-wasm: abi restriction Nov 19, 2020

implement proxy-wasm ABI restriction

f468a96

Signed-off-by: Ryan Apilado <[email protected]>

ryanapilado force-pushed the capability-restriction-tests branch from 03d1ffb to f468a96 Compare November 19, 2020 02:26

ryanapilado changed the title ~~[WIP] proxy-wasm: abi restriction~~ [WIP] wasm: abi restriction Nov 19, 2020

ryanapilado added 4 commits November 19, 2020 02:50

change std::unordered_set to absl::flat_hash_set

13b76e0

Signed-off-by: Ryan Apilado <[email protected]>

BUILD file format

a62f952

Signed-off-by: Ryan Apilado <[email protected]>

BUILD file format

4c0d583

Signed-off-by: Ryan Apilado <[email protected]>

format

a8e77dd

Signed-off-by: Ryan Apilado <[email protected]>

PiotrSikora reviewed Nov 19, 2020

View reviewed changes

ryanapilado added 7 commits November 19, 2020 16:49

rename abi function to capability

4dffd5e

Signed-off-by: Ryan Apilado <[email protected]>

revert unnecessary change

15dacd1

Signed-off-by: Ryan Apilado <[email protected]>

format

cb0bce3

Signed-off-by: Ryan Apilado <[email protected]>

add WASI test

b988429

Signed-off-by: Ryan Apilado <[email protected]>

fix empty value and names

af695b1

Signed-off-by: Ryan Apilado <[email protected]>

formatting

f9cf17f

Signed-off-by: Ryan Apilado <[email protected]>

test all VMs

8c8b69f

Signed-off-by: Ryan Apilado <[email protected]>

htuch reviewed Nov 20, 2020

View reviewed changes

ryanapilado added 4 commits December 3, 2020 04:44

change allowed_capabilities to a map

e63f7ed

Signed-off-by: Ryan Apilado <[email protected]>

update documentation

9edc973

Signed-off-by: Ryan Apilado <[email protected]>

fix format

caa4f68

Signed-off-by: Ryan Apilado <[email protected]>

spelling

486b68a

Signed-off-by: Ryan Apilado <[email protected]>

mathetake reviewed Jan 13, 2021

View reviewed changes

bazel/repository_locations.bzl Outdated Show resolved Hide resolved

update proxy-wasm-cpp-host to latest version

bb76300

Signed-off-by: Ryan Apilado <[email protected]>

Base automatically changed from master to main January 15, 2021 23:01

PiotrSikora mentioned this pull request Jan 25, 2021

Add knobs for allowed modifications in ext_authz and ext_proc #14789

Closed

PiotrSikora reviewed Feb 1, 2021

View reviewed changes

ryanapilado and others added 7 commits February 3, 2021 02:28

s/proxy-wasm/Proxy-Wasm

0d0a6c2

Signed-off-by: Ryan Apilado <[email protected]>

NOTE on newline

af91df7

Signed-off-by: Ryan Apilado <[email protected]>

s/cr_config/capability_restriction_config

909b1fd

Signed-off-by: Ryan Apilado <[email protected]>

fix test names and comments

d6fa1c1

Signed-off-by: Ryan Apilado <[email protected]>

add new callback test

ac3f629

Signed-off-by: Ryan Apilado <[email protected]>

Merge branch 'main' into capability-restriction-tests

7101e0c

config -> config_

ee8a52b

Signed-off-by: Ryan Apilado <[email protected]>

fix NOTE newline

38afe66

Signed-off-by: Ryan Apilado <[email protected]>

PiotrSikora requested a review from mathetake February 5, 2021 19:47

mathetake approved these changes Feb 8, 2021

View reviewed changes

PiotrSikora approved these changes Feb 8, 2021

View reviewed changes

lizan approved these changes Feb 9, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Feb 9, 2021

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Feb 9, 2021

lizan merged commit 47ad8ee into envoyproxy:main Feb 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm: capability restriction #13911

wasm: capability restriction #13911

ryanapilado commented Nov 5, 2020 •

edited

Loading

antoniovicente commented Nov 9, 2020

repokitteh-read-only bot commented Nov 18, 2020

PiotrSikora left a comment

htuch Nov 20, 2020

ryanapilado Dec 18, 2020

mathetake left a comment

ryanapilado commented Jan 13, 2021

repokitteh-read-only bot commented Jan 13, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

ryanapilado Feb 5, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

PiotrSikora Feb 1, 2021

ryanapilado Feb 3, 2021

ryanapilado commented Feb 3, 2021

repokitteh-read-only bot commented Feb 3, 2021

ryanapilado commented Feb 3, 2021

repokitteh-read-only bot commented Feb 3, 2021

PiotrSikora left a comment

lizan commented Feb 9, 2021

moderation commented Feb 9, 2021

wasm: capability restriction #13911

wasm: capability restriction #13911

Conversation

ryanapilado commented Nov 5, 2020 • edited Loading

antoniovicente commented Nov 9, 2020

repokitteh-read-only bot commented Nov 18, 2020

PiotrSikora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathetake left a comment

Choose a reason for hiding this comment

ryanapilado commented Jan 13, 2021

repokitteh-read-only bot commented Jan 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanapilado commented Feb 3, 2021

repokitteh-read-only bot commented Feb 3, 2021

ryanapilado commented Feb 3, 2021

repokitteh-read-only bot commented Feb 3, 2021

PiotrSikora left a comment

Choose a reason for hiding this comment

lizan commented Feb 9, 2021

moderation commented Feb 9, 2021

ryanapilado commented Nov 5, 2020 •

edited

Loading