search

wapc / wapc-go

Golang-based WebAssembly Host Runtime for waPC-compliant modules
Apache License 2.0
91 stars 25 forks source link

feat(hostcallDuringLoad): Allow updating the context with InvokeContext #55

Closed ritesh089 closed 1 year ago

ritesh089 commented 1 year ago

wapc go 's wazero runtime allows web assembly modules to make host calls to request the host runtime to execute some tasks that web assembly is not capable of . wapc 's host call is limited to the guest module's explicitly exported function that gets called during the invoke. This PR allows the host to update with InvocationContext prior to wasm instantiation. This allows the wasm to perform hostcalls during the loading phase.

Value Added by this PR: This PR is important because the wasm can now fetch environment variables during the instantiation phase while utilizing the benefits provided by wapc.

If there is a need to perform hostcalls from main, prior to instantiation of the wasm, a line of code is to be added : m.ctx = wazero.AddInvokeContext(m.ctx)

pkedy commented 1 year ago

Hi @ritesh089 - thanks for this PR and the clear explanation of the added value.

First, I'm considering the security impact of this change. External access during initialization is considered insecure in other Wasm plugin projects I've worked on. The Wasm module could be untrusted code. That said, waPC works a little differently, and no concern immediately comes to mind, but I want to think about it some more.

Second, if we assume this is safe, it should be consistent across all engines (e.g., Wasmtime, Wasmer). That said, the approach would differ. With Wazero, I think it would make sense to change WithStartFunctions(functionStart, functionInit) to WithStartFunctions() and call those functions manually with a context initialized with a context like in Invoke:

    ic := invokeContext{operation: operation, guestReq: payload}
    ctx = newInvokeContext(ctx, &ic)

I wanted to run my thinking by you. Thoughts?

/cc: @codefromthecrypt Do you have thoughts on the security aspect of this feature?

ritesh089 commented 1 year ago

hi @pkedy , Since Wazero already supports calling host functions during main directly, I am assuming it is safe. Wapc go uses the invoke context which will be nil when called from _start because of which the call fails. Changing to WithStartFunctions() will still call it from main and you will still need the above method to use the invocation context. This PR would still be useful in that case as well. So it supports both the options. But I think calling _start explicitly requires additional steps with not much change in behavior.

I am not clear about the security impact of this change. I am trying to understand how we can identify that a wasm module is a trusted source in its _start function.

pkedy commented 1 year ago

Changing to WithStartFunctions() will still call it from main and you will still need the above method to use the invocation context.

@ritesh089 I wasn't sure if I followed the "will still call it from main" part of this comment, but you are correct about the invocation context.

Check out https://github.com/wapc/wapc-go/compare/feature/host_calls_during_instantiate?expand=1 and see if we are on the same page. The other engines would likely need to be tweaked as well. I welcome contributions, so feel free to snag my changes for this PR if it was what you were thinking above.

ritesh089 commented 1 year ago

Thanks @pkedy , Yes these changes work . Will update my PR accordingly .

ritesh089 commented 1 year ago

@pkedy , have updated the PR. Can you review and let me know if I need to make any additional changes?