Skip to content

Commit

Permalink
Merge pull request #5886 from dtrudg/allownet
Browse files Browse the repository at this point in the history 
Allow specified non-root users/groups to use specified --net --networks
  • Loading branch information
@dtrudg
dtrudg authored Apr 9, 2021
2 parents 6101926 + 34d5cf7 commit 0035c69
Show file tree
Hide file tree
Showing 10 changed files with 617 additions and 22 deletions.
210 changes: 208 additions & 2 deletions e2e/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package config

import (
"fmt"
"io/ioutil"
"os"
"testing"
Expand Down Expand Up @@ -400,6 +401,210 @@ func (c configTests) configGlobal(t *testing.T) {
}
}

// Tests that require combinations of directives to be set
func (c configTests) configGlobalCombination(t *testing.T) {
e2e.EnsureImage(t, c.env)

setDirective := func(t *testing.T, directives map[string]string) {
for k, v := range directives {
c.env.RunSingularity(
t,
e2e.WithProfile(e2e.RootProfile),
e2e.WithCommand("config global"),
e2e.WithArgs("--set", k, v),
e2e.ExpectExit(0),
)
}
}
resetDirective := func(t *testing.T, directives map[string]string) {
for k := range directives {
c.env.RunSingularity(
t,
e2e.WithProfile(e2e.RootProfile),
e2e.WithCommand("config global"),
e2e.WithArgs("--reset", k),
e2e.ExpectExit(0),
)
}
}

u := e2e.UserProfile.HostUser(t)
g, err := user.GetGrGID(u.GID)
if err != nil {
t.Fatalf("could not retrieve user group information: %s", err)
}

tests := []struct {
name string
argv []string
profile e2e.Profile
addRequirementsFn func(*testing.T)
cwd string
directives map[string]string
exit int
resultOp e2e.SingularityCmdResultOp
}{
{
name: "AllowNetUsersNobody",
argv: []string{"--net", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": "nobody",
},
exit: 255,
},
{
name: "AllowNetUsersUser",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
},
exit: 255,
},
{
name: "AllowNetUsersUID",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": fmt.Sprintf("%d", u.UID),
},
exit: 255,
},
{
name: "AllowNetUsersUserOK",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
"allow net networks": "bridge",
},
exit: 0,
},
{
name: "AllowNetUsersUIDOK",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": fmt.Sprintf("%d", u.UID),
"allow net networks": "bridge",
},
exit: 0,
},
{
name: "AllowNetGroupsNobody",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net groups": "nobody",
},
exit: 255,
},
{
name: "AllowNetGroupsGroup",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net groups": g.Name,
},
exit: 255,
},
{
name: "AllowNetGroupsGID",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net groups": fmt.Sprintf("%d", g.GID),
},
exit: 255,
},
{
name: "AllowNetGroupsGroupOK",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net groups": g.Name,
"allow net networks": "bridge",
},
exit: 0,
},
{
name: "AllowNetGroupsGIDOK",
argv: []string{"--net", "--network", "bridge", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net groups": fmt.Sprintf("%d", g.GID),
"allow net networks": "bridge",
},
exit: 0,
},
{
name: "AllowNetNetworksMultiMulti",
// Two networks allowed, asking for both
argv: []string{"--net", "--network", "bridge,ptp", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
"allow net networks": "bridge,ptp",
},
exit: 0,
},
{
// Two networks allowed, asking for one
name: "AllowNetNetworksMultiOne",
argv: []string{"--net", "--network", "ptp", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
"allow net networks": "bridge,ptp",
},
exit: 0,
},
{
// One network allowed, but asking for two
name: "AllowNetNetworksOneMulti",
argv: []string{"--net", "--network", "bridge,ptp", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
"allow net networks": "bridge",
},
exit: 255,
},
{
// No networks allowed, asking for two
name: "AllowNetNetworksNoneMulti",
argv: []string{"--net", "--network", "bridge,ptp", c.env.ImagePath, "true"},
profile: e2e.UserProfile,
directives: map[string]string{
"allow net users": u.Name,
},
exit: 255,
},
}

for _, tt := range tests {
c.env.RunSingularity(
t,
e2e.AsSubtest(tt.name),
e2e.WithProfile(tt.profile),
e2e.WithDir(tt.cwd),
e2e.PreRun(func(t *testing.T) {
if tt.addRequirementsFn != nil {
tt.addRequirementsFn(t)
}
setDirective(t, tt.directives)
}),
e2e.PostRun(func(t *testing.T) {
resetDirective(t, tt.directives)
}),
e2e.WithCommand("exec"),
e2e.WithArgs(tt.argv...),
e2e.ExpectExit(tt.exit, tt.resultOp),
)
}
}

func (c configTests) configFile(t *testing.T) {
e2e.EnsureImage(t, c.env)

Expand Down Expand Up @@ -469,7 +674,8 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
np := testhelper.NoParallel

return testhelper.Tests{
"config file": c.configFile, // test --config file option
"config global": np(c.configGlobal), // test various global configuration
"config file": c.configFile, // test --config file option
"config global": np(c.configGlobal), // test various global configuration
"config global combination": np(c.configGlobalCombination), // test various global configuration with combination
}
}
3 changes: 2 additions & 1 deletion internal/app/singularity/cache_clean_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/sylabs/singularity/internal/pkg/cache"
"github.com/sylabs/singularity/pkg/sylog"
"github.com/sylabs/singularity/pkg/util/slice"
)

var (
Expand Down Expand Up @@ -42,7 +43,7 @@ func CleanSingularityCache(imgCache *cache.Handle, dryRun bool, cacheCleanTypes

// If specified caches, and we don't have 'all' specified then clean the specified
// ones only.
if len(cacheCleanTypes) > 0 && !stringInSlice("all", cacheCleanTypes) {
if len(cacheCleanTypes) > 0 && !slice.ContainsString(cacheCleanTypes, "all") {
cachesToClean = cacheCleanTypes
}

Expand Down
16 changes: 4 additions & 12 deletions internal/app/singularity/cache_list_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/sylabs/singularity/internal/pkg/cache"
"github.com/sylabs/singularity/internal/pkg/util/fs"
"github.com/sylabs/singularity/pkg/util/slice"
)

// listTypeCache will list a cache type with given name (cacheType). The options are 'library', and 'oci'.
Expand Down Expand Up @@ -74,15 +75,15 @@ func ListSingularityCache(imgCache *cache.Handle, cacheListTypes []string, cache
blobsShown := false

// If types requested includes "all" then we don't want to filter anything
if stringInSlice("all", cacheListTypes) {
if slice.ContainsString(cacheListTypes, "all") {
cacheListTypes = []string{}
}

for _, cacheType := range cache.OciCacheTypes {
// the type blob is special: 1. there's a
// separate counter for it; 2. the cache entries
// are actually one level deeper
if len(cacheListTypes) > 0 && !stringInSlice(cacheType, cacheListTypes) {
if len(cacheListTypes) > 0 && !slice.ContainsString(cacheListTypes, cacheType) {
continue
}
cacheDir, err := imgCache.GetOciCacheDir(cacheType)
Expand All @@ -101,7 +102,7 @@ func ListSingularityCache(imgCache *cache.Handle, cacheListTypes []string, cache
blobsShown = true
}
for _, cacheType := range cache.FileCacheTypes {
if len(cacheListTypes) > 0 && !stringInSlice(cacheType, cacheListTypes) {
if len(cacheListTypes) > 0 && !slice.ContainsString(cacheListTypes, cacheType) {
continue
}
cacheDir, err := imgCache.GetFileCacheDir(cacheType)
Expand Down Expand Up @@ -141,12 +142,3 @@ func ListSingularityCache(imgCache *cache.Handle, cacheListTypes []string, cache

return nil
}

func stringInSlice(a string, list []string) bool {
for _, b := range list {
if b == a {
return true
}
}
return false
}
9 changes: 7 additions & 2 deletions internal/pkg/runtime/engine/singularity/cleanup_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,18 @@ func (e *EngineOperations) CleanupContainer(ctx context.Context, fatal error, st
}

if networkSetup != nil {
if e.EngineConfig.GetFakeroot() {
net := e.EngineConfig.GetNetwork()
privileged := false
// If a CNI configuration was allowed as non-root (or fakeroot)
if net != "none" && os.Geteuid() != 0 {
priv.Escalate()
privileged = true
}
sylog.Debugf("Cleaning up CNI network config %s", net)
if err := networkSetup.DelNetworks(ctx); err != nil {
sylog.Errorf("could not delete networks: %v", err)
}
if e.EngineConfig.GetFakeroot() {
if privileged {
priv.Drop()
}
}
Expand Down
38 changes: 33 additions & 5 deletions internal/pkg/runtime/engine/singularity/container_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/sylabs/singularity/pkg/util/loop"
"github.com/sylabs/singularity/pkg/util/namespaces"
"github.com/sylabs/singularity/pkg/util/singularityconf"
"github.com/sylabs/singularity/pkg/util/slice"
"golang.org/x/crypto/ssh/terminal"
"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -2245,11 +2246,35 @@ func (c *container) prepareNetworkSetup(system *mount.System, pid int) (func(con
fakeroot := c.engine.EngineConfig.GetFakeroot()
net := c.engine.EngineConfig.GetNetwork()
euid := os.Geteuid()
allowedNetUnpriv := false
if euid != 0 {
// Is the user permitted in the list of unpriv users / groups permitted to use CNI?
allowedNetUser, err := user.UIDInList(euid, c.engine.EngineConfig.File.AllowNetUsers)
if err != nil {
return nil, err
}
allowedNetGroup, err := user.UIDInAnyGroup(euid, c.engine.EngineConfig.File.AllowNetGroups)
if err != nil {
return nil, err
}
// Is/are the requested network(s) in the list of networks allowed for unpriv CNI?
allowedNetNetwork := false
for _, n := range strings.Split(net, ",") {
allowedNetNetwork = slice.ContainsString(c.engine.EngineConfig.File.AllowNetNetworks, n)
// If any one requested network is not allowed, disallow the whole config
if !allowedNetNetwork {
sylog.Errorf("Network %s is not permitted for unprivileged users.", n)
break
}
}
// User is in the user / groups allowed, and requesting an allowed network?
allowedNetUnpriv = (allowedNetUser || allowedNetGroup) && allowedNetNetwork
}

if !c.netNS || net == noneNet {
return nil, nil
} else if (c.userNS || euid != 0) && !fakeroot {
return nil, fmt.Errorf("network requires root or --fakeroot, users need to specify --network=%s with --net", noneNet)
} else if (c.userNS || euid != 0) && !fakeroot && !allowedNetUnpriv {
return nil, fmt.Errorf("network requires root or --fakeroot, non-root users can only use --network=%s unless permitted by the administrator", noneNet)
}

// we hold a reference to container network namespace
Expand All @@ -2264,6 +2289,7 @@ func (c *container) prepareNetworkSetup(system *mount.System, pid int) (func(con
}
networks := strings.Split(c.engine.EngineConfig.GetNetwork(), ",")

// In fakeroot mode only permit the `fakeroot` CNI config
if fakeroot && euid != 0 && net != fakerootNet {
// set as debug message to avoid annoying warning
sylog.Debugf("only '%s' network is allowed for regular user, you requested '%s'", fakerootNet, net)
Expand Down Expand Up @@ -2293,10 +2319,12 @@ func (c *container) prepareNetworkSetup(system *mount.System, pid int) (func(con
}

return func(ctx context.Context) error {
if fakeroot {
if fakeroot || allowedNetUnpriv {
// prevent port hijacking between user processes
if err := networkSetup.SetPortProtection(fakerootNet, 0); err != nil {
return err
for _, n := range strings.Split(net, ",") {
if err := networkSetup.SetPortProtection(n, 0); err != nil {
return err
}
}
if euid != 0 {
priv.Escalate()
Expand Down
Loading

0 comments on commit 0035c69

Please sign in to comment.