Implemented seccomp and pass the integration test

[yihuaf]

Fix #25

So this turns out to be simpler than I thought, thanks to the existance of libseccomp. runc also uses libseccomp which does all the heavy lifting.

One concern I have is the seccomp-sys repo. We don't have a lot of choices for a rust binding for libseccomp and this one looks reasonable. However, it is LGPL, so we may want to roll our own bindings. I used this repo just for prototype. If we are NOT OK with LGPL code into our repo, then we should discuss on what is the right thing to do here.

Regardless, this is a functional prototypes now, so I would like you guys to take a look. The PR can be merged as is, given the issue mentioned above.

TODO:

• oci-spec-rs is missing a few fields for seccomp. For example, the default error code is hardcoded to EPERM at the moment.
• Our own libssecomp bindings??

[codecov-commenter]

Codecov Report

Merging #292 (c0c51b1) into main (c0a344e) will decrease coverage by 0.23%.
The diff coverage is 71.68%.

@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
- Coverage   77.41%   77.17%   -0.24%     
==========================================
  Files          42       43       +1     
  Lines        5920     6139     +219     
==========================================
+ Hits         4583     4738     +155     
- Misses       1337     1401      +64     

[flxo]

If you do decide to to your own bindings consider an option to include the build of libseccomp from your bindings build.rs because crates relying of prebuilt dependencies are just painful when cross compiling.

[yihuaf]

If you do decide to to your own bindings consider an option to include the build of libseccomp from your bindings build.rs because crates relying of prebuilt dependencies are just painful when cross compiling.

I think eventually this would be a good thing to support, especially for arch other than X86, and we will definitely keep this in mind. I am interested to understand your use cases more, especially if you are thinking about statically linking on other platform. On the other hand, I am fairly convinced that offloading to libseccomp is the right thing to do going forward, so let's sort this PR out first.

[yihuaf]

Finally.... @utam0k PTAL. Also let me know what you think about LGPL and the seccomp-sys crate.

A big chunk is the config.json I use for testing. I pulled that straight from docker, to make sure everything works.

[utam0k]

@yihuaf Sorry I didn't have time to review this PR today, so I'll review it tomorrow.

[Furisto]

@yihuaf Sorry I didn't have time to review this PR today, so I'll review it tomorrow.

Same here. Sorry for the delay.

[yihuaf]

Take your time :) It is a more complex change.

[utam0k]

Is there any documentation that shows that 0 is the correct default value?

[yihuaf]

The scmp_arg_cmp struct represent a comparison. Some of the op require one and some require two value. For example, an equal comparison requires only one value, testing if the syscall argument is equal to this value. In the case where the only one value is needed, I believe the value two or datum_b is ignored. The implementation in libseccomp sets the value to 0 anyway, but I don't think it is specifically documented in the man page.

[Furisto]

Can you add this explanation as a comment to the code?

[utam0k]

I would love to know your thoughts on the merits of this and other options? Isn't it realistic to not depend on libseccomp for example? I'm new to this field, so sorry if I'm saying something crazy.

Our own libssecomp bindings??

[yihuaf]

I would love to know your thoughts on the merits of this and other options? Isn't it realistic to not depend on libseccomp for example? I'm new to this field, so sorry if I'm saying something crazy.

Our own libssecomp bindings??

First of all, this PR contains all the necessary changes needed in Youki to use seccomp. Any further work would be to replace the library code underneath, depending on how far we want to go.

The first question is whether to use seccomp-sys repo or our own binding. The downside of seccomp-sys repo is the LGPL license and the fact that it may not be well maintained. The work to replace it doesn't look hard since it is a thin layer of FFI bindings. I believe rust even has a auto generator for generating bindings for C.

The second question is whether to use libseccomp. My short answer is we should use libseccomp unless at some point in the future it is not working out for us. libseccomp is well maintained and used by runc, so I don't worry about the project becomes unmaintained. To some extent, OCI spec is designed around the libseccomp API or designed with libseccomp in mind. We are linking against libseccomp, so its LGPL license is not an issue. So at this point, rewriting libseccomp logic does not buy us much in my opinion.

With that being said, it would be a great project to rewrite libseccomp in rust, but it should be out of the scoop of Youki.

[Furisto]

Can you add this explanation as a comment to the code?

[Furisto]

The first question is whether to use seccomp-sys repo or our own binding. The downside of seccomp-sys repo is the LGPL license and the fact that it may not be well maintained. The work to replace it doesn't look hard since it is a thin layer of FFI bindings. I believe rust even has a auto generator for generating bindings for C.

I am not familiar with LGPL in detail, so please correct me here, but could that mean that we would have to also release youki as LGPL if we statically link to it?

[yihuaf]

The first question is whether to use seccomp-sys repo or our own binding. The downside of seccomp-sys repo is the LGPL license and the fact that it may not be well maintained. The work to replace it doesn't look hard since it is a thin layer of FFI bindings. I believe rust even has a auto generator for generating bindings for C.

I am not familiar with LGPL in detail, so please correct me here, but could that mean that we would have to also release youki as LGPL if we statically link to it?

The first question is whether to use seccomp-sys repo or our own binding. The downside of seccomp-sys repo is the LGPL license and the fact that it may not be well maintained. The work to replace it doesn't look hard since it is a thin layer of FFI bindings. I believe rust even has a auto generator for generating bindings for C.

I am not familiar with LGPL in detail, so please correct me here, but could that mean that we would have to also release youki as LGPL if we statically link to it?

In short, no. You are thinking of GPL license.

If we dynamically link LGPL code, we are fine. If we statically link, we need to provide our code so people can re-link to a different version of the LGPL lib, under any license. Youki being open sourced meets the requirement. So legally we can use it without any trouble. In addition, we are not really distributing Youki as compiled binary, so we aren't limited by the license. Story will change if someone wants to use Youki code for commercial purposes.

Still I thought I should bring it up just so we are aware :) I believe we already use the systemd lib in Youki which is also LGPL code.

[yihuaf]

Turns out, writing FFI binding in rust is way easier than I imagined... I included it in this PR and removed the need for seccomp-sys crate. @Furisto Can I get another approve from you?

[utam0k]

great!!!!!