-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
wasm: capability restriction #13911
wasm: capability restriction #13911
Changes from 34 commits
f468a96
13b76e0
a62f952
4c0d583
a8e77dd
4dffd5e
15dacd1
cb0bce3
b988429
af695b1
f9cf17f
8c8b69f
e63f7ed
9edc973
caa4f68
486b68a
b5f793c
91067f6
f189898
1959a9c
f3f73d4
5b27bb2
12bd787
5798880
559f452
238bace
0b9099c
acc3e53
5c8f31b
372d335
eed3c43
452b91e
86c5c34
cc60e16
bb76300
0d0a6c2
af91df7
909b1fd
d6fa1c1
ac3f629
7101e0c
ee8a52b
38afe66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
// name. The *SanitizationConfig* which each capability maps to is currently unimplemented and ignored, | ||
// 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>`_. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
// Additionally, the following WASI capabilities from | ||
// `this list <https://github.com/WebAssembly/WASI/blob/master/phases/snapshot/docs.md#modules>`_ | ||
// are implemented and can be allowed: | ||
// *fd_write*, *fd_read*, *fd_seek*, *fd_close*, *fd_fdstat_get*, *environ_get*, *environ_sizes_get*, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. everything listed here except
And in the future, we may support get_environ since there's a feature request in the multiple SDK repositories, proxy-wasm/spec#19. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even though the host implementation of WASI is indeed a bunch of mostly non-working (but adhering to spec) stubs, modules compiled against WASI want to import them, and this PR is purely about allowing/rejecting such calls. |
||
// *args_get*, *args_sizes_get*, *proc_exit*, *clock_time_get*, *random_get*. | ||
map<string, SanitizationConfig> allowed_capabilities = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same goes for proxy-wasm-cpp-host side PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The keys in the map are always allowed capabilities, so I think the name is still accurate. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, right. Thanks |
||
} | ||
|
||
// Configuration for sanitization of inputs to an allowed capability. NOTE: This is currently unimplemented. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: move There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Misunderstood this, fixed. |
||
message SanitizationConfig { | ||
} | ||
|
||
// Configuration for a Wasm VM. | ||
// [#next-free-field: 7] | ||
message VmConfig { | ||
|
@@ -74,7 +94,7 @@ message VmConfig { | |
} | ||
|
||
// Base Configuration for Wasm Plugins e.g. filters and services. | ||
// [#next-free-field: 6] | ||
// [#next-free-field: 7] | ||
message PluginConfig { | ||
// A unique name for a filters/services in a VM for use in identifying the filter/service if | ||
// multiple filters/services are handled by the same *vm_id* and *root_id* and for | ||
|
@@ -105,6 +125,9 @@ message PluginConfig { | |
// during xDS updates the xDS configuration will be rejected and when on_start or on_configuration return false on initial | ||
// startup the proxy will not start. | ||
bool fail_open = 5; | ||
|
||
// Configuration for restricting proxy-wasm capabilities available to modules. | ||
CapabilityRestrictionConfig capability_restriction_config = 6; | ||
} | ||
|
||
// WasmService is configured as a built-in *envoy.wasm_service* :ref:`WasmService | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,10 +100,11 @@ void Wasm::initializeLifecycle(Server::ServerLifecycleNotifier& lifecycle_notifi | |
} | ||
|
||
Wasm::Wasm(absl::string_view runtime, absl::string_view vm_id, absl::string_view vm_configuration, | ||
absl::string_view vm_key, const Stats::ScopeSharedPtr& scope, | ||
Upstream::ClusterManager& cluster_manager, Event::Dispatcher& dispatcher) | ||
: WasmBase(createWasmVm(runtime), vm_id, vm_configuration, vm_key), scope_(scope), | ||
cluster_manager_(cluster_manager), dispatcher_(dispatcher), | ||
absl::string_view vm_key, proxy_wasm::AllowedCapabilitiesMap allowed_capabilities, | ||
const Stats::ScopeSharedPtr& scope, Upstream::ClusterManager& cluster_manager, | ||
Event::Dispatcher& dispatcher) | ||
: WasmBase(createWasmVm(runtime), vm_id, vm_configuration, vm_key, allowed_capabilities), | ||
scope_(scope), cluster_manager_(cluster_manager), dispatcher_(dispatcher), | ||
time_source_(dispatcher.timeSource()), | ||
wasm_stats_(WasmStats{ | ||
ALL_WASM_STATS(POOL_COUNTER_PREFIX(*scope_, absl::StrCat("wasm.", runtime, ".")), | ||
|
@@ -312,8 +313,9 @@ WasmEvent toWasmEvent(const std::shared_ptr<WasmHandleBase>& wasm) { | |
NOT_IMPLEMENTED_GCOVR_EXCL_LINE; | ||
} | ||
|
||
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, changed to capabiility_restriction_config for consistency with type name and proto. Can change to capabilities_config if too long. |
||
const PluginSharedPtr& plugin, const Stats::ScopeSharedPtr& scope, | ||
Upstream::ClusterManager& cluster_manager, | ||
Init::Manager& init_manager, Event::Dispatcher& dispatcher, | ||
Api::Api& api, Server::ServerLifecycleNotifier& lifecycle_notifier, | ||
|
@@ -380,7 +382,7 @@ static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr& | |
.value_or(code.empty() ? EMPTY_STRING : INLINE_STRING); | ||
} | ||
|
||
auto complete_cb = [cb, vm_config, plugin, scope, &cluster_manager, &dispatcher, | ||
auto complete_cb = [cb, vm_config, cr_config, plugin, scope, &cluster_manager, &dispatcher, | ||
&lifecycle_notifier, create_root_context_for_testing, | ||
wasm_extension](std::string code) -> bool { | ||
if (code.empty()) { | ||
|
@@ -391,10 +393,10 @@ static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr& | |
proxy_wasm::makeVmKey(vm_config.vm_id(), anyToBytes(vm_config.configuration()), code); | ||
auto wasm_factory = wasm_extension->wasmFactory(); | ||
proxy_wasm::WasmHandleFactory proxy_wasm_factory = | ||
[&vm_config, scope, &cluster_manager, &dispatcher, &lifecycle_notifier, | ||
[&vm_config, &cr_config, scope, &cluster_manager, &dispatcher, &lifecycle_notifier, | ||
wasm_factory](absl::string_view vm_key) -> WasmHandleBaseSharedPtr { | ||
return wasm_factory(vm_config, scope, cluster_manager, dispatcher, lifecycle_notifier, | ||
vm_key); | ||
return wasm_factory(vm_config, cr_config, scope, cluster_manager, dispatcher, | ||
lifecycle_notifier, vm_key); | ||
}; | ||
auto wasm = proxy_wasm::createWasm( | ||
vm_key, code, plugin, proxy_wasm_factory, | ||
|
@@ -469,15 +471,16 @@ static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr& | |
return true; | ||
} | ||
|
||
bool createWasm(const VmConfig& vm_config, const PluginSharedPtr& plugin, | ||
const Stats::ScopeSharedPtr& scope, Upstream::ClusterManager& cluster_manager, | ||
Init::Manager& init_manager, Event::Dispatcher& dispatcher, Api::Api& api, | ||
bool createWasm(const VmConfig& vm_config, const CapabilityRestrictionConfig& cr_config, | ||
const PluginSharedPtr& plugin, const Stats::ScopeSharedPtr& scope, | ||
Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager, | ||
Event::Dispatcher& dispatcher, Api::Api& api, | ||
Envoy::Server::ServerLifecycleNotifier& lifecycle_notifier, | ||
Config::DataSource::RemoteAsyncDataProviderPtr& remote_data_provider, | ||
CreateWasmCallback&& cb, CreateContextFn create_root_context_for_testing) { | ||
return createWasmInternal(vm_config, plugin, scope, cluster_manager, init_manager, dispatcher, | ||
api, lifecycle_notifier, remote_data_provider, std::move(cb), | ||
create_root_context_for_testing); | ||
return createWasmInternal(vm_config, cr_config, plugin, scope, cluster_manager, init_manager, | ||
dispatcher, api, lifecycle_notifier, remote_data_provider, | ||
std::move(cb), create_root_context_for_testing); | ||
} | ||
|
||
PluginHandleSharedPtr | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
s/proxy-wasm/Proxy-Wasm/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.