landlock-lsm / go-landlock

A Go library for the Linux Landlock sandboxing feature
MIT License
115 stars 8 forks source link

landlock/syscall: use syscall.AllThreadsSyscall{,6} directly #17

Closed tklauser closed 3 years ago

tklauser commented 3 years ago

Go 1.16 added the AllThreadsSyscall{,6} functions to the syscall package. The kernel.org/pub/linux/libs/security/libcap/psx package merely wraps these for Go 1.16 and newer (and implements them using cgo for older Go versions).

This package's go.mod specifies version 1.16. Thus, AllThreadsSyscall{,6} can be used directly from package syscall.

gnoack commented 3 years ago

Thank you for your contribution.

I'm afraid I'm currently not convinced by this argument. From the AllThreadsSyscall documentation:

AllThreadsSyscall is unaware of any threads that are launched explicitly by cgo linked code, so the function always returns ENOTSUP in binaries that use cgo.

And from https://pkg.go.dev/cmd/cgo:

The cgo tool is enabled by default for native builds on systems where it is expected to work. It is disabled by default when cross-compiling.

My understanding is that

  1. syscall.AllThreadsSyscall will not work in the case of programs that use cgo, because it then doesn't know any more which OS threads exist for the current process.
  2. Cgo is the default on native builds on platforms like x86_64 Linux.

I did play with syscall.AllThreadsSyscall instead of psx at some point, and found that it quickly fell apart in my own setup, just by linking some common core libraries like the Go http package (iirc) which apparently make the compiler use cgo if available.

Please also see the discussion and further links on the closed issue #5.

Possible alternative solutions I see are:

l0kod commented 3 years ago
  • Alternatively, it might be nice if Landlock supported process-wide enforcement like seccomp does with its "tsync" (thread sync) flag. @l0kod FYI

Yep, I need to add this option.

tklauser commented 3 years ago

Thanks for the elaborate answer.

My understanding is that

  1. syscall.AllThreadsSyscall will not work in the case of programs that use cgo, because it then doesn't know any more which OS threads exist for the current process.
  2. Cgo is the default on native builds on platforms like x86_64 Linux.

Indeed, I did not consider the case of using cgo at all :( I should have read the docs more carefully.

  • I'd love to see a better solution to that problem that is part of the Go runtime, which I believe you have been contributing to? :)

I'd love to see that as well. But it's currently a bit out of reach for me to implement.

  • Or, I'm misunderstanding something above, which is also well possible. Let me know. :)

Not at all, it's a misunderstanding on my side. Thanks for taking the time to elaborate.

Closing this PR.