runtime-spec icon indicating copy to clipboard operation
runtime-spec copied to clipboard

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

Open Zheaoli opened this issue 1 year ago • 32 comments

Carry #1111

Zheaoli avatar Jan 02 '24 15:01 Zheaoli

cc @l0kod @AkihiroSuda @vbatts @gnoack @cyphar

Zheaoli avatar Jan 02 '24 15:01 Zheaoli

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

AkihiroSuda avatar Jan 09 '24 08:01 AkihiroSuda

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

AkihiroSuda avatar Jan 09 '24 08:01 AkihiroSuda

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

I will carry it

Zheaoli avatar Jan 09 '24 09:01 Zheaoli

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.

l0kod avatar Jan 10 '24 16:01 l0kod

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)

gnoack avatar Jan 10 '24 19:01 gnoack

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

Zheaoli avatar Jan 10 '24 21:01 Zheaoli

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

utam0k avatar Jan 11 '24 11:01 utam0k

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

Zheaoli avatar Mar 01 '24 03:03 Zheaoli

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?

gnoack avatar Mar 01 '24 08:03 gnoack

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)

Zheaoli avatar Mar 01 '24 11:03 Zheaoli

Needs rebase

AkihiroSuda avatar Jul 01 '24 05:07 AkihiroSuda