-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "Revert "Add a "dropNetworking" function and unit tests to the…
… runner package."" This fixes the size calculation to also work on 32-bit systems. This can be quickly checked with "GOOS=linux GOARCH=arm go build ./..." The two commits can be seen separately here: master...dlorenc:rollbacknet This reverts commit dccd0ed.
- Loading branch information
1 parent
dccd0ed
commit 5cd1b51
Showing
3 changed files
with
117 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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") | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package main | ||
|
||
import ( | ||
"math" | ||
"os/exec" | ||
"syscall" | ||
) | ||
|
||
// We need the max value of an unsigned 32 bit integer (4294967295), but we also need this number | ||
// to fit into an "int". One some systems this is 32 bits, so the max uint32 can't fit into here. | ||
// maxIntForArch is the higher of those two values. | ||
func maxIntForArch() int { | ||
// We have to do over two lines as a variable. The go compiler optimizes | ||
// away types for constants, so int(uint32(math.MaxUint32)) is the same as int(math.MaxUint32), | ||
// which overflows. | ||
maxUint := uint32(math.MaxUint32) | ||
if int(maxUint) > math.MaxInt32 { | ||
return int(maxUint) | ||
} | ||
return math.MaxInt32 | ||
} | ||
|
||
// 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) { | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
vdemeester
Member
|
||
// 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: maxIntForArch(), | ||
}, | ||
} | ||
|
||
// 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: maxIntForArch(), | ||
}, | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
// 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. | ||
func TestDropNetworking(t *testing.T) { | ||
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) | ||
} | ||
} | ||
} |
@dlorenc so… this definitely does not work on OpenShift. I didn't get into which part doesn't, but a process inside OpenShift, not being root (which is the case of most taskrun pods, which will also be the case of the entrypoint, and which is the case of the test runned in our CI) cannot set all thoses. The error doesn't speak too much
/usr/bin/env: operation not permitted