Making verifying binary optional in wasi related code path

[yihuaf]

In the OCI container path, we do a few things to verify that the binary path pass in as part of the process block in the OCI spec exist and has the right permission. The path is usually an absolute path to a binary in the container rootfs. However, wasi and wasm have syntax that calls to a specific module of a wasm file. So the path becomes /some/path/x.wasm#module. This is not a valid path and will fail our check.

Specifically see the issue from runwasi. containerd/runwasi#194

There are two options I am contemplating:

• Provide a optional field in the libcontainer interface to make the verification optional.
• Move the verification logic into executor instead.

I like the simplicity of moving the verification logic into executor, but there is one down side. The executor will not be executed until youki start and youki create will no longer fail if the binary doesn't exist. This may also break some existing integration test and even break OCI compliance.

I have not decided on which path to take, but want this issue for tracking and discussion.

[YJDoc2]

I feel we should support the wasm use case in some way, but I am inclined against making any changes that will break OCI compatibility or make the integration tests fail. Is there a way to check if given entrypoint is wasm or not, and then conditionally do the check?

[yihuaf]

I am inclined against making any changes that will break OCI compatibility

Agreed. (I will do some test and reading the spec to make sure this. If it doesn't break assumptions in the spec, we can continue to explore if this is a good idea. Otherwise, this is a deal breaker.)

if given entrypoint is wasm or not, and then conditionally do the check?

Perhaps, but I also want to be careful introducing wasm specific information into the libcontainer internals. So far, we have been successfully abstracting wasm related information to outside of the libcontainer. My biggest concern is that wasm is a fast moving targets and things in this space moves quite fast and unstable. On the other hand, libcontainer do need to support OCI which is reasonably stable and mature by now.

Perhaps this means to need to introduce a configuration in the interface to skip the binary verification. At least, in this way, we surface the control knob up to the interface, instead of conceal the checking inside the implementation of libcontainer.

---

Another idea is to change the executor interface to split off a validate step, which runs as part of the youki create path. I am in the process of changing the executor interface back to using a struct with method, so we can easily make this changes. I think it is not too crazy of an idea to require the executor to have a validate step to validate if the input spec is good.

Sigh... designing a good interface is hard... but at least now we have a customer we can look to for usecases and hints :)

[jsturtevant]

We are also exploring using OCI Artifacts for WASM. In this case the module was loaded from containerd's content store instead of within the container. The container file system in this case is just the files required for the module to run but the module itself is loaded by the wasm runtime. I got around this by disabling verification temporarily.

[jsturtevant]

of the two options, I like the "Provide a optional field in the libcontainer interface to make the verification optional" via the builder interface becuase of the downsides of running it at create time. Maybe something like:

ContainerBuilder::new(self.id.clone(), syscall.as_ref())
            .with_executor(vec![Box::new(WasmtimeExecutor {
                stdin,
                stdout,
                stderr,
                engine,
            })])?
            .with_root_path(self.rootdir.clone())?
            .as_init(&self.bundle)
            .with_binary_verification(false)
            .with_systemd(false)
            .build()?;

[utam0k]

🤦‍♂I forgot about it.

I like the simplicity of moving the verification logic into executor, but there is one down side. The executor will not be executed until youki start and youki create will no longer fail if the binary doesn't exist. This may also break some existing integration test and even break OCI compliance.

I love @jsturtevant 's idea.

[yihuaf]

@jsturtevant Yes, looks like introducing the nob in the builder interface is the better approach here. I will get this implemented soon.

[jsturtevant]

The builder approach might have the drawback that if more than one executor is running it would apply to all of them?

https://github.com/containerd/runwasi/pull/156/files#r1283810527

[Mossaka]

If we implement binary check logic in can_handle of the DefaultExecutor, does it achieve the same effect as calling verify_binary in the container_init_process function call?

[yihuaf]

If we implement binary check logic in can_handle of the DefaultExecutor, does it achieve the same effect as calling verify_binary in the container_init_process function call?

This is what I was hinting at to abstract the validation logic into the executor. If we head down this path, I do think we need to move where the can_handle (may be renamed to something else like validate to preserve the behavior.

[yihuaf]

The builder approach might have the drawback that if more than one executor is running it would apply to all of them?

https://github.com/containerd/runwasi/pull/156/files#r1283810527

This should be OK. I am changing the API to use a single executor and remove the executor list. The new scheme will favor composing multiple executor into a single executor. This should provider executor authors with greater control on how executors are implemented.