-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add a "dropNetworking" function and unit tests to the runner package. #3582
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// +build !linux | ||
|
||
package main | ||
|
||
import "os/exec" | ||
|
||
// The implementation of this currently only works on Linux. | ||
// This is a placeholder for compilation/testing. | ||
func dropNetworking(cmd *exec.Cmd) { | ||
panic("only implemented on linux") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package main | ||
|
||
import ( | ||
"os/exec" | ||
"syscall" | ||
) | ||
|
||
// dropNetworking modifies the supplied exec.Cmd to execute in a net set of namespaces that do not | ||
// have network access | ||
func dropNetworking(cmd *exec.Cmd) { | ||
// These flags control the behavior of the new process. | ||
// Documentation for these is available here: https://man7.org/linux/man-pages/man2/clone.2.html | ||
// We mostly want to just create a new network namespace, unattached to any networking devices. | ||
// The other flags are necessary for that to work. | ||
|
||
if cmd.SysProcAttr == nil { | ||
// We build this up piecemeal in case it was already set, to avoid overwriting anything. | ||
cmd.SysProcAttr = &syscall.SysProcAttr{} | ||
} | ||
cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWNS | | ||
syscall.CLONE_NEWPID | // NEWPID creates a new process namespace | ||
syscall.CLONE_NEWNET | // NEWNET creates a new network namespace (this is the one we really care about) | ||
syscall.CLONE_NEWUSER // NEWUSER creates a new user namespace | ||
|
||
// We need to map the existing user IDs into the new namespace. | ||
// Just map everything. | ||
cmd.SysProcAttr.UidMappings = []syscall.SysProcIDMap{ | ||
{ | ||
ContainerID: 0, | ||
HostID: 0, | ||
// Map all users | ||
Size: 4294967295, | ||
}, | ||
} | ||
|
||
// This is needed to allow programs to call setgroups when in a new Gid namespace. | ||
// Things like apt-get install require this to work. | ||
cmd.SysProcAttr.GidMappingsEnableSetgroups = true | ||
// We need to map the existing group IDs into the new namespace. | ||
// Just map everything. | ||
cmd.SysProcAttr.GidMappings = []syscall.SysProcIDMap{ | ||
{ | ||
ContainerID: 0, | ||
HostID: 0, | ||
|
||
// Map all groups | ||
Size: 4294967295, | ||
}, | ||
} | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
// +build linux | ||
|
||
package main | ||
|
||
import ( | ||
"os/exec" | ||
"testing" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
) | ||
|
||
// This isn't a great unit test, but it's the best I can think of. | ||
dlorenc marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is such a nitpick but i think if you start docstrings with anything other than the name of the function it's confusing for tools |
||
// It attempts to verify there is no network access by making a network | ||
// request. If the test were to run in an offline environment, or an already | ||
// sandboxed environment, the test could pass even if the dropNetworking | ||
// function did nothing. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i guess the other thing you could do is assert on the exact values you set on the cmd object but that doesn't feel super useful |
||
func TestDropNetworking(t *testing.T) { | ||
dlorenc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cmd := exec.Command("curl", "google.com") | ||
dropNetworking(cmd) | ||
b, err := cmd.CombinedOutput() | ||
if err == nil { | ||
t.Errorf("Expected an error making a network connection. Got %s", string(b)) | ||
} | ||
|
||
// Other things (env, etc.) should all be the same | ||
cmds := []string{"env", "whoami", "pwd", "uname"} | ||
for _, cmd := range cmds { | ||
withNetworking := exec.Command(cmd) | ||
withoutNetworking := exec.Command(cmd) | ||
dropNetworking(withoutNetworking) | ||
|
||
b1, err1 := withNetworking.CombinedOutput() | ||
b2, err2 := withoutNetworking.CombinedOutput() | ||
if err1 != err2 { | ||
t.Errorf("Expected no errors, got %v %v", err1, err2) | ||
} | ||
if diff := cmp.Diff(string(b1), string(b2)); diff != "" { | ||
t.Error(diff) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function is beautiful 🤩