Skip to content

Commit

Permalink
enforcement: fix use case when the same target has different hashes (#…
Browse files Browse the repository at this point in the history 
…1469)

Fix an issue when the same mount target could have different hashes
during device mount policy enforcement.
Although it's possible to mount different devices at the same mount
location, this doesn't make sense for read-only container layers.
The device mount enforcement logic has been updated to cover this
case.
This was discovered by randomized security policy unit tests.

The tests have been updated, to minimize the chance of it happening
by adding a minimal length for a random string and appropriate unit
test has been added to cover the change.

Signed-off-by: Maksim An <maksiman@microsoft.com>
  • Loading branch information
@anmaxvl
anmaxvl authored Aug 8, 2022
1 parent a244751 commit ba4bfca
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 5 deletions.
83 changes: 78 additions & 5 deletions pkg/securitypolicy/securitypolicy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

const (
// variables that influence generated test fixtures
minStringLength = 10
maxContainersInGeneratedPolicy = 32
maxLayersInGeneratedContainer = 32
maxGeneratedContainerID = 1000000
Expand All @@ -38,9 +39,17 @@ const (
var testRand *rand.Rand

func init() {
seed := rand.NewSource(time.Now().Unix())
testRand = rand.New(seed)
fmt.Fprintf(os.Stdout, "securitypolicy_test seed: %d\n", seed.Int63())
seed := time.Now().Unix()
if seedStr, ok := os.LookupEnv("SEED"); ok {

if parsedSeed, err := strconv.ParseInt(seedStr, 10, 64); err != nil {
fmt.Fprintf(os.Stderr, "failed to parse seed: %d\n", seed)
} else {
seed = parsedSeed
}
}
testRand = rand.New(rand.NewSource(seed))
fmt.Fprintf(os.Stdout, "securitypolicy_test seed: %d\n", seed)
}

// Validate that our conversion from the external SecurityPolicy representation
Expand Down Expand Up @@ -150,12 +159,16 @@ func Test_EnforceDeviceMountPolicy_Matches(t *testing.T) {
func Test_EnforceDeviceUmountPolicy_Removes_Device_Entries(t *testing.T) {
f := func(p *generatedContainers) bool {
policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString)
defer func() {
t.Logf("policy state: %+v\n", policy)
}()

target := generateMountTarget(testRand)
rootHash := selectRootHashFromContainers(p, testRand)

err := policy.EnforceDeviceMountPolicy(target, rootHash)
if err != nil {
t.Error(err)
return false
}

Expand All @@ -166,6 +179,7 @@ func Test_EnforceDeviceUmountPolicy_Removes_Device_Entries(t *testing.T) {

err = policy.EnforceDeviceUnmountPolicy(target)
if err != nil {
t.Error(err)
return false
}

Expand All @@ -182,6 +196,10 @@ func Test_EnforceDeviceUmountPolicy_Removes_Device_Entries(t *testing.T) {
func Test_EnforceOverlayMountPolicy_No_Matches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, false)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand All @@ -203,6 +221,10 @@ func Test_EnforceOverlayMountPolicy_No_Matches(t *testing.T) {
func Test_EnforceOverlayMountPolicy_Matches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand Down Expand Up @@ -232,7 +254,7 @@ func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice(t *testing.T)
}

if err := tc.policy.EnforceOverlayMountPolicy(tc.containerID, tc.layers); err == nil {
t.Fatalf("able to create overlay for the same container twice")
t.Fatal("able to create overlay for the same container twice")
}
}

Expand Down Expand Up @@ -307,13 +329,17 @@ func Test_EnforceOverlayMountPolicy_Overlay_Single_Container_Twice_With_Differen

err = sp.EnforceOverlayMountPolicy(containerIDTwo, layerPaths)
if err == nil {
t.Fatalf("able to reuse an overlay across containers")
t.Fatal("able to reuse an overlay across containers")
}
}

func Test_EnforceCommandPolicy_Matches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand All @@ -338,6 +364,9 @@ func Test_EnforceCommandPolicy_Matches(t *testing.T) {
func Test_EnforceCommandPolicy_NoMatches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()
if err != nil {
t.Error(err)
return false
Expand Down Expand Up @@ -380,6 +409,9 @@ func Test_EnforceCommandPolicy_NarrowingMatches(t *testing.T) {
p.containers = append(p.containers, testContainerOne, &testContainerTwo)

policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString)
defer func() {
t.Logf("policy state: %+v\n", policy)
}()

testContainerOneID := ""
testContainerTwoID := ""
Expand Down Expand Up @@ -463,6 +495,10 @@ func Test_EnforceCommandPolicy_NarrowingMatches(t *testing.T) {
func Test_EnforceEnvironmentVariablePolicy_Matches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand Down Expand Up @@ -522,6 +558,10 @@ func Test_EnforceEnvironmentVariablePolicy_Re2Match(t *testing.T) {
func Test_EnforceEnvironmentVariablePolicy_NotAllMatches(t *testing.T) {
f := func(p *generatedContainers) bool {
tc, err := setupContainerWithOverlay(p, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand Down Expand Up @@ -566,6 +606,9 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) {
p.containers = append(p.containers, testContainerOne, &testContainerTwo)

policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString)
defer func() {
t.Logf("policy state: %+v\n", policy)
}()

testContainerOneID := ""
testContainerTwoID := ""
Expand All @@ -578,11 +621,13 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) {

layerPaths, err := createValidOverlayForContainer(policy, container, testRand)
if err != nil {
t.Error(err)
return false
}

err = policy.EnforceOverlayMountPolicy(containerID, layerPaths)
if err != nil {
t.Error(err)
return false
}

Expand Down Expand Up @@ -622,6 +667,7 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) {
envVars := buildEnvironmentVariablesFromContainerRules(testContainerOne, testRand)
err := policy.enforceEnvironmentVariablePolicy(testContainerOneID, envVars)
if err != nil {
t.Error(err)
return false
}

Expand Down Expand Up @@ -650,6 +696,10 @@ func Test_EnforceEnvironmentVariablePolicy_NarrowingMatches(t *testing.T) {
func Test_WorkingDirectoryPolicy_Matches(t *testing.T) {
testFunc := func(gc *generatedContainers) bool {
tc, err := setupContainerWithOverlay(gc, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand All @@ -671,6 +721,10 @@ func Test_WorkingDirectoryPolicy_Matches(t *testing.T) {
func Test_WorkingDirectoryPolicy_NoMatches(t *testing.T) {
testFunc := func(gc *generatedContainers) bool {
tc, err := setupContainerWithOverlay(gc, true)
defer func() {
t.Logf("policy state: %+v\n", tc.policy)
}()

if err != nil {
t.Error(err)
return false
Expand Down Expand Up @@ -698,6 +752,9 @@ func Test_Overlay_Duplicate_Layers(t *testing.T) {
c1.Layers[numLayers-3] = c1.Layers[numLayers-2]

policy := NewStandardSecurityPolicyEnforcer([]*securityPolicyContainer{c1}, ignoredEncodedPolicyString)
defer func() {
t.Logf("policy state: %+v\n", policy)
}()

// generate mount targets
mountTargets := make([]string, numLayers)
Expand Down Expand Up @@ -755,6 +812,19 @@ func Test_Overlay_Duplicate_Layers(t *testing.T) {
}
}

func Test_EnforceDeviceMountPolicy_DifferentTargetsWithTheSameHash(t *testing.T) {
c := generateContainersContainer(testRand, 2, 2)
policy := NewStandardSecurityPolicyEnforcer([]*securityPolicyContainer{c}, ignoredEncodedPolicyString)
mountTarget := randString(testRand, 10)
if err := policy.EnforceDeviceMountPolicy(mountTarget, c.Layers[0]); err != nil {
t.Fatalf("unexpected error: %s", err)
}
// Mounting the second layer at the same mount target should fail
if err := policy.EnforceDeviceMountPolicy(mountTarget, c.Layers[1]); err == nil {
t.Fatal("expected conflicting device hashes error")
}
}

//
// Setup and "fixtures" follow...
//
Expand Down Expand Up @@ -1090,6 +1160,9 @@ func randVariableString(r *rand.Rand, maxLen int32) string {
}

func randString(r *rand.Rand, length int32) string {
if length < minStringLength {
length = minStringLength
}
charset := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
sb := strings.Builder{}
sb.Grow(int(length))
Expand Down
8 changes: 8 additions & 0 deletions pkg/securitypolicy/securitypolicyenforcer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,14 @@ func (pe *StandardSecurityPolicyEnforcer) EnforceDeviceMountPolicy(target string
for _, container := range pe.Containers {
for _, layer := range container.Layers {
if deviceHash == layer {
if existingHash := pe.Devices[target]; existingHash != "" {
return fmt.Errorf(
"conflicting device hashes for target %s: old=%s, new=%s",
target,
existingHash,
deviceHash,
)
}
pe.Devices[target] = deviceHash
return nil
}
Expand Down

0 comments on commit ba4bfca

Please sign in to comment.