Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: syscall: add landlock support on Linux #47049

Closed
illiliti opened this issue Jul 4, 2021 · 13 comments
Closed

proposal: syscall: add landlock support on Linux #47049

illiliti opened this issue Jul 4, 2021 · 13 comments

Comments

@illiliti
Copy link

illiliti commented Jul 4, 2021

Due to golang policy on fork/exec, I have to double-run myself, set landlock rules and exec into desired executable. This all looks like a dirty hack, especially in terms of library code where you can't simply re-exec yourself. I propose to add native landlock support.

Reference: https://landlock.io
POC implementation: https://github.com/gnoack/golandlock

Example:

package main

import (
	"log"
	"os/exec"
	"syscall"

	"golang.org/x/sys/unix"
)

func main() {
	// /tmp and /var/tmp are readable, /home is not readable
	cmd := exec.Command("/bin/ls", "/tmp", "/var/tmp", "/home")
	cmd.SysProcAttr = &syscall.SysProcAttr{
		LandlockRules: []syscall.LandlockRule{
			{
				Flags: unix.LANDLOCK_ACCESS_FS_READ_DIR,
				Files: []string{
					"/tmp",
					"/var/tmp",
				},
			},
			// this could be done implicitly
			{
				Flags: unix.LANDLOCK_ACCESS_FS_EXECUTE,
				Files: []string{
					"/bin/ls",
				},
			},
		},
	}

	if err := cmd.Run(); err != nil {
		log.Fatal(err)
	}
}
@gopherbot gopherbot added this to the Proposal milestone Jul 4, 2021
@mvdan
Copy link
Member

mvdan commented Jul 5, 2021

Presumably this would be an addition to x/sys/unix, given that the syscall package is frozen:

Deprecated: this package is locked down. Callers should use the corresponding package in the golang.org/x/sys repository instead. That is also where updates required by new systems or versions should be applied. See https://golang.org/s/go1.4-syscall for more information.

@mvdan
Copy link
Member

mvdan commented Jul 5, 2021

Ah, I see that os/exec references a syscall struct directly, so I assume you would need additions to both.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jul 5, 2021
@ianlancetaylor ianlancetaylor changed the title proposal: syscall: Add landlock support on Linux proposal: syscall: add landlock support on Linux Jul 5, 2021
@ianlancetaylor
Copy link
Contributor

Can you propose a specific API to add to the syscall package? As far as I can tell the golanglock package you mention can be implemented entirely outside of the standard library, and we could for example discuss adding it to x/sys/unix. What do we need in syscall? Thanks.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

Would it suffice to add to syscall.SysProcAttr the rulesetfd (as an *os.File I guess) and have the child call landlock_restrict_self on that fd?
Why does the whole rule language have to go into SysProcAttr?

@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 20, 2021
@rsc
Copy link
Contributor

rsc commented Nov 3, 2021

Ping @illiliti re API questions above.

@illiliti
Copy link
Author

illiliti commented Nov 3, 2021

Would it suffice to add to syscall.SysProcAttr the rulesetfd (as an *os.File I guess) and have the child call landlock_restrict_self on that fd? Why does the whole rule language have to go into SysProcAttr?

Sounds good. All other functionality that creates and manages rulesetfd will go into x/sys/unix. It should be very simple to implement.

NIT: I suppose rulesetfd should be int because File usually represents regular files and rulesetfd is not a regular file.

@rsc
Copy link
Contributor

rsc commented Nov 10, 2021

It sounds like all we need to do is add

LandlockRuleSet int

to SysProcAttr? But what about the zero value? 0 is a valid fd and if you have nothing open it's what opening the landlock fd will return. So it sounds like maybe we need

UseLandlock bool
LandlockRuleSet int

?

How often does this arise?

@illiliti
Copy link
Author

0 is a valid fd

Right.

How often does this arise?

bool is already used by Foreground and Setctty to control Ctty fd. I think it is valid to use bool here. Alternatively, we can choose slice and use its default value to avoid bool helper.

@rsc
Copy link
Contributor

rsc commented Dec 1, 2021

Does anyone object to adding to SysProcAttr:

UseLandlock bool
LandlockRuleSet int // fd of rules

?

@illiliti
Copy link
Author

illiliti commented Dec 3, 2021

Closing this because #49383 is WONTFIX. Very disappointed in Go. Feel free to reopen, but don't expect any contribution from me.

@illiliti illiliti closed this as completed Dec 3, 2021
@rsc
Copy link
Contributor

rsc commented Dec 8, 2021

Treating this as retracted, but it seems like we reached a reasonable API change. What we don't know is whether it is good enough in practice or whether anyone needs it. If someone does need it, please feel free to open a new proposal and we can continue this discussion. Thanks.

And apologies @illiliti for the requirement.

@rsc rsc moved this from Active to Declined in Proposals (old) Dec 8, 2021
@rsc
Copy link
Contributor

rsc commented Dec 8, 2021

This proposal has been declined as retracted.
— rsc for the proposal review group

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

5 participants