Skip to content

Commit

Permalink
rego: fix slightly incorrect sandbox and hugepage mounts enforcement
Browse files Browse the repository at this point in the history 
Sandbox and hugepage mounts come via CRI config in the form:
`sandbox://<absolute-path>`, however the existing enforcement and tests
expect it to be `sandbox://<relative-path>` which causes a problem during
mount enforcement, when the sandbox prefix is replaced with an additional
path separator in the end.

Additionally update policy tests.

Signed-off-by: Maksim An <maksiman@microsoft.com>
  • Loading branch information
@anmaxvl
anmaxvl committed Jan 27, 2023
1 parent 6cd5572 commit 29c1096
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 41 deletions.
12 changes: 6 additions & 6 deletions internal/tools/securitypolicy/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ strategy = "re2"
rule = "PREFIX_.+=.+"

[[container.mount]]
host_path = "sandbox://host/path/one"
host_path = "sandbox:///host/path/one"
container_path = "/container/path/one"
readonly = false

[[container.mount]]
host_path = "sandbox://host/path/two"
host_path = "sandbox:///host/path/two"
container_path = "/container/path/two"
readonly = true

Expand Down Expand Up @@ -130,7 +130,7 @@ represented in JSON.
"length": 2,
"elements": {
"0": {
"source": "sandbox://host/path/one",
"source": "sandbox:///host/path/one",
"destination": "/container/path/one",
"type": "bind",
"options": {
Expand All @@ -143,7 +143,7 @@ represented in JSON.
}
},
"1": {
"source": "sandbox://host/path/two",
"source": "sandbox:///host/path/two",
"destination": "/container/path/two",
"type": "bind",
"options": {
Expand Down Expand Up @@ -223,7 +223,7 @@ containers := [
"command": ["rustc","--help"],
"env_rules": [{"pattern": "PATH=/usr/local/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "strategy": "string", "required": false},{"pattern": "RUSTUP_HOME=/usr/local/rustup", "strategy": "string", "required": false},{"pattern": "CARGO_HOME=/usr/local/cargo", "strategy": "string", "required": false},{"pattern": "RUST_VERSION=1.52.1", "strategy": "string", "required": false},{"pattern": "TERM=xterm", "strategy": "string", "required": false},{"pattern": "PREFIX_.+=.+", "strategy": "re2", "required": false}],
"layers": ["fe84c9d5bfddd07a2624d00333cf13c1a9c941f3a261f13ead44fc6a93bc0e7a","4dedae42847c704da891a28c25d32201a1ae440bce2aecccfa8e6f03b97a6a6c","41d64cdeb347bf236b4c13b7403b633ff11f1cf94dbc7cf881a44d6da88c5156","eb36921e1f82af46dfe248ef8f1b3afb6a5230a64181d960d10237a08cd73c79","e769d7487cc314d3ee748a4440805317c19262c7acd2fdbdb0d47d2e4613a15c","1b80f120dbd88e4355d6241b519c3e25290215c469516b49dece9cf07175a766"],
"mounts": [{"destination": "/container/path/one", "options": ["rbind","rshared","rw"], "source": "sandbox://host/path/one", "type": "bind"},{"destination": "/container/path/two", "options": ["rbind","rshared","ro"], "source": "sandbox://host/path/two", "type": "bind"}],
"mounts": [{"destination": "/container/path/one", "options": ["rbind","rshared","rw"], "source": "sandbox:///host/path/one", "type": "bind"},{"destination": "/container/path/two", "options": ["rbind","rshared","ro"], "source": "sandbox://host/path/two", "type": "bind"}],
"exec_processes": [{"command": ["top"], "signals": []}],
"signals": [],
"allow_elevated": true,
Expand Down Expand Up @@ -287,7 +287,7 @@ containers := [
"command": ["rustc","--help"],
"env_rules": [{"pattern": "PATH=/usr/local/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "strategy": "string", "required": false},{"pattern": "RUSTUP_HOME=/usr/local/rustup", "strategy": "string", "required": false},{"pattern": "CARGO_HOME=/usr/local/cargo", "strategy": "string", "required": false},{"pattern": "RUST_VERSION=1.52.1", "strategy": "string", "required": false},{"pattern": "TERM=xterm", "strategy": "string", "required": false},{"pattern": "PREFIX_.+=.+", "strategy": "re2", "required": false}],
"layers": ["fe84c9d5bfddd07a2624d00333cf13c1a9c941f3a261f13ead44fc6a93bc0e7a","4dedae42847c704da891a28c25d32201a1ae440bce2aecccfa8e6f03b97a6a6c","41d64cdeb347bf236b4c13b7403b633ff11f1cf94dbc7cf881a44d6da88c5156","eb36921e1f82af46dfe248ef8f1b3afb6a5230a64181d960d10237a08cd73c79","e769d7487cc314d3ee748a4440805317c19262c7acd2fdbdb0d47d2e4613a15c","1b80f120dbd88e4355d6241b519c3e25290215c469516b49dece9cf07175a766"],
"mounts": [{"destination": "/container/path/one", "options": ["rbind","rshared","rw"], "source": "sandbox://host/path/one", "type": "bind"},{"destination": "/container/path/two", "options": ["rbind","rshared","ro"], "source": "sandbox://host/path/two", "type": "bind"}],
"mounts": [{"destination": "/container/path/one", "options": ["rbind","rshared","rw"], "source": "sandbox:///host/path/one", "type": "bind"},{"destination": "/container/path/two", "options": ["rbind","rshared","ro"], "source": "sandbox://host/path/two", "type": "bind"}],
"exec_processes": [{"command": ["top"], "signals": []}],
"signals": [],
"allow_elevated": true,
Expand Down
3 changes: 2 additions & 1 deletion pkg/securitypolicy/securitypolicy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"math/rand"
"os"
"path/filepath"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -1105,7 +1106,7 @@ func generateMounts(r *rand.Rand) []mountInternal {
sourcePrefix = guestpath.HugePagesMountPrefix
}

source := sourcePrefix + randVariableString(r, maxGeneratedMountSourceLength)
source := filepath.Join(sourcePrefix, randVariableString(r, maxGeneratedMountSourceLength))
destination := randVariableString(r, maxGeneratedMountDestinationLength)

mounts[i] = mountInternal{
Expand Down
15 changes: 2 additions & 13 deletions pkg/securitypolicy/securitypolicyenforcer_rego.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"os"
"strconv"
"strings"
"syscall"
Expand Down Expand Up @@ -345,16 +344,6 @@ func (policy *regoEnforcer) EnforceOverlayUnmountPolicy(target string) error {
return err
}

// Rego does not have a way to determine the OS path separator
// so we append it via these methods.
func sandboxMountsDir(sandboxID string) string {
return fmt.Sprintf("%s%c", spec.SandboxMountsDir(sandboxID), os.PathSeparator)
}

func hugePagesMountsDir(sandboxID string) string {
return fmt.Sprintf("%s%c", spec.HugePagesMountsDir(sandboxID), os.PathSeparator)
}

func getEnvsToKeep(envList []string, results rpi.RegoQueryResult) ([]string, error) {
value, err := results.Value("env_list")
if err != nil {
Expand Down Expand Up @@ -395,8 +384,8 @@ func (policy *regoEnforcer) EnforceCreateContainerPolicy(
"argList": argList,
"envList": envList,
"workingDir": workingDir,
"sandboxDir": sandboxMountsDir(sandboxID),
"hugePagesDir": hugePagesMountsDir(sandboxID),
"sandboxDir": spec.SandboxMountsDir(sandboxID),
"hugePagesDir": spec.HugePagesMountsDir(sandboxID),
"mounts": appendMountData([]interface{}{}, mounts),
}

Expand Down
44 changes: 23 additions & 21 deletions test/cri-containerd/policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ func securityPolicyFromContainers(
policyString, err := securitypolicy.MarshalPolicy(policyType, false, pc,
[]securitypolicy.ExternalProcessConfig{
{
Command: []string{"ls", "-l", "/dev/mapper"},
WorkingDir: "/",
Command: []string{"ls", "-l", "/dev/mapper"},
WorkingDir: "/",
AllowStdioAccess: true,
},
{
Command: []string{"bash"},
WorkingDir: "/",
Command: []string{"bash"},
WorkingDir: "/",
AllowStdioAccess: true,
},
},
[]securitypolicy.FragmentConfig{},
Expand All @@ -57,10 +59,10 @@ func securityPolicyFromContainers(
return base64.StdEncoding.EncodeToString([]byte(policyString)), nil
}

func sandboxSecurityPolicy(t *testing.T, policyType string, allowEnvironmentVariableDropping bool) string {
func sandboxSecurityPolicy(t *testing.T, policyType string, unencryptedOk bool) string {
t.Helper()
defaultContainers := helpers.DefaultContainerConfigs()
policyString, err := securityPolicyFromContainers(policyType, true, defaultContainers, allowEnvironmentVariableDropping)
policyString, err := securityPolicyFromContainers(policyType, unencryptedOk, defaultContainers, true)
if err != nil {
t.Fatalf("failed to create security policy string: %s", err)
}
Expand Down Expand Up @@ -135,7 +137,7 @@ func Test_RunPodSandbox_WithPolicy_Allowed(t *testing.T) {

for _, pc := range policyTestMatrix {
t.Run(t.Name()+fmt.Sprintf("_Enforcer_%s_Input_%s", pc.enforcer, pc.input), func(t *testing.T) {
sandboxPolicy := sandboxSecurityPolicy(t, pc.input, false)
sandboxPolicy := sandboxSecurityPolicy(t, pc.input, true)
sandboxRequest := sandboxRequestWithPolicy(t, sandboxPolicy)
sandboxRequest.Config.Annotations[annotations.SecurityPolicyEnforcer] = pc.enforcer

Expand Down Expand Up @@ -364,7 +366,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -375,7 +377,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
},
},
Expand All @@ -386,7 +388,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
Readonly: true,
Expand All @@ -398,7 +400,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Readonly: true,
},
Expand All @@ -410,7 +412,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path/regexp",
HostPath: "sandbox:///sandbox/path/regexp",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -421,7 +423,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_Allowed(t *testing.T) {
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path/r.+",
HostPath: "sandbox:///sandbox/path/r.+",
ContainerPath: "/container/path",
},
},
Expand Down Expand Up @@ -479,7 +481,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
},
},
Expand All @@ -491,7 +493,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/invalid/path",
HostPath: "sandbox:///sandbox/invalid/path",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -506,7 +508,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path/invalid",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -521,7 +523,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
Readonly: true,
Expand All @@ -537,7 +539,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -548,7 +550,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path",
HostPath: "sandbox:///sandbox/path",
ContainerPath: "/container/path",
Readonly: true,
},
Expand All @@ -561,7 +563,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
sideEffect: func(req *runtime.CreateContainerRequest) error {
req.Config.Mounts = append(
req.Config.Mounts, &runtime.Mount{
HostPath: "sandbox://sandbox/path/regex/no/match",
HostPath: "sandbox:///sandbox/path/regex/no/match",
ContainerPath: "/container/path",
Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
Expand All @@ -572,7 +574,7 @@ func Test_RunContainer_WithPolicy_And_MountConstraints_NotAllowed(t *testing.T)
securitypolicy.WithMountConstraints(
[]securitypolicy.MountConfig{
{
HostPath: "sandbox://sandbox/path/R.+",
HostPath: "sandbox:///sandbox/path/R.+",
ContainerPath: "/container/path",
},
},
Expand Down

0 comments on commit 29c1096

Please sign in to comment.