search

opencontainers / runtime-spec

OCI Runtime Specification
http://www.opencontainers.org
Apache License 2.0
3.13k stars 535 forks source link

Carry #1111: specs-go/config: add Landlock LSM support #1241

Open Zheaoli opened 6 months ago

Zheaoli commented 6 months ago

Carry #1111

Zheaoli commented 6 months ago

cc @l0kod @AkihiroSuda @vbatts @gnoack @cyphar

AkihiroSuda commented 5 months ago

cc @kailun-qin (author of #1111) PTAL

AkihiroSuda commented 5 months ago

Before merging this PR, can we have a POC implementation of runc (or crun, youki) to confirm the design?

Zheaoli commented 5 months ago

https://github.com/opencontainers/runc/pull/3194 POC is here. Still need polish some code and add tests for it.

I will carry it

l0kod commented 5 months ago

Thanks for working on this!

You might want to take a look at this talk about backward and forward compatibility challenges: https://archive.fosdem.org/2023/schedule/event/rust_backward_and_forward_compatibility_for_security_features/

FYI, I plan to review these changes next week.

gnoack commented 5 months ago

Thanks for working on this!

You might want to take a look at this talk about backward and forward compatibility challenges: https://archive.fosdem.org/2023/schedule/event/rust_backward_and_forward_compatibility_for_security_features/

FYI, I plan to review these changes next week.

FWIW the most common implementation problem so far has been the use of the "refer" right, whose logic unfortunately works slightly differently than for other access rights. I tried to explain this in https://blog.gnoack.org/post/landlock-best-effort/ last year.

With Linux 6.7, Landlock also has gained support for restricting the use of bind(2) and connect(2) for TCP. (Landlock ABI version 4)

Zheaoli commented 5 months ago

FYI, I plan to review these changes next week.

Thanks for the important information, I will update this spec in this week ASAP I can

utam0k commented 5 months ago

I wonder what the behavior should be when a new process(e.g. docker exec) jumps into the existing container.

Zheaoli commented 4 months ago

I have update the PR, PTAL @kailun-qin @gnoack @l0kod

gnoack commented 4 months ago

High level remark: Looks good so far. I added a bunch of documentation nits.

As for Landlock-specific comments, I would recommend to re-think some of the type names, and I left comments in these places.

I think the bigger chance for mistakes is in the implementation itself. - Are you planning to add that in the same PR, or will that come separately?

Zheaoli commented 4 months ago

I think the bigger chance for mistakes is in the implementation itself. - Are you planning to add that in the same PR, or will that come separately?

I think it would be great to implement it in a single PR(Actually, I don't know which is better to split the PR. Depend on the ABI version or else)

AkihiroSuda commented 2 days ago

Needs rebase