Merge pull request from GHSA-c9cp-9c75-9v8c

dmcgowan · web-flow · commit 551516a18d0a · 2022-03-23T10:50:56.000-07:00
Fix the Inheritable capability defaults.
diff --git a/oci/spec.go b/oci/spec.go
@@ -148,10 +148,9 @@ func populateDefaultUnixSpec(ctx context.Context, s *Spec, id string) error {
 				GID: 0,
 			},
 			Capabilities: &specs.LinuxCapabilities{
-				Bounding:    defaultUnixCaps(),
-				Permitted:   defaultUnixCaps(),
-				Inheritable: defaultUnixCaps(),
-				Effective:   defaultUnixCaps(),
+				Bounding:  defaultUnixCaps(),
+				Permitted: defaultUnixCaps(),
+				Effective: defaultUnixCaps(),
 			},
 			Rlimits: []specs.POSIXRlimit{
 				{
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -873,7 +873,6 @@ func WithCapabilities(caps []string) SpecOpts {
 		s.Process.Capabilities.Bounding = caps
 		s.Process.Capabilities.Effective = caps
 		s.Process.Capabilities.Permitted = caps
-		s.Process.Capabilities.Inheritable = caps
 
 		return nil
 	}
@@ -908,7 +907,6 @@ func WithAddedCapabilities(caps []string) SpecOpts {
 				&s.Process.Capabilities.Bounding,
 				&s.Process.Capabilities.Effective,
 				&s.Process.Capabilities.Permitted,
-				&s.Process.Capabilities.Inheritable,
 			} {
 				if !capsContain(*cl, c) {
 					*cl = append(*cl, c)
@@ -928,7 +926,6 @@ func WithDroppedCapabilities(caps []string) SpecOpts {
 				&s.Process.Capabilities.Bounding,
 				&s.Process.Capabilities.Effective,
 				&s.Process.Capabilities.Permitted,
-				&s.Process.Capabilities.Inheritable,
 			} {
 				removeCap(cl, c)
 			}
@@ -943,7 +940,7 @@ func WithDroppedCapabilities(caps []string) SpecOpts {
 func WithAmbientCapabilities(caps []string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 		setCapabilities(s)
-
+		s.Process.Capabilities.Inheritable = caps
 		s.Process.Capabilities.Ambient = caps
 		return nil
 	}
diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go
@@ -39,7 +39,6 @@ func TestAddCaps(t *testing.T) {
 		s.Process.Capabilities.Bounding,
 		s.Process.Capabilities.Effective,
 		s.Process.Capabilities.Permitted,
-		s.Process.Capabilities.Inheritable,
 	} {
 		if !capsContain(cl, "CAP_CHOWN") {
 			t.Errorf("cap list %d does not contain added cap", i)
@@ -63,7 +62,6 @@ func TestDropCaps(t *testing.T) {
 		s.Process.Capabilities.Bounding,
 		s.Process.Capabilities.Effective,
 		s.Process.Capabilities.Permitted,
-		s.Process.Capabilities.Inheritable,
 	} {
 		if capsContain(cl, "CAP_CHOWN") {
 			t.Errorf("cap list %d contains dropped cap", i)
@@ -82,7 +80,6 @@ func TestDropCaps(t *testing.T) {
 		s.Process.Capabilities.Bounding,
 		s.Process.Capabilities.Effective,
 		s.Process.Capabilities.Permitted,
-		s.Process.Capabilities.Inheritable,
 	} {
 		if capsContain(cl, "CAP_FOWNER") {
 			t.Errorf("cap list %d contains dropped cap", i)
@@ -103,7 +100,6 @@ func TestDropCaps(t *testing.T) {
 		s.Process.Capabilities.Bounding,
 		s.Process.Capabilities.Effective,
 		s.Process.Capabilities.Permitted,
-		s.Process.Capabilities.Inheritable,
 	} {
 		if len(cl) != 0 {
 			t.Errorf("cap list %d is not empty", i)
diff --git a/oci/spec_test.go b/oci/spec_test.go
@@ -45,7 +45,6 @@ func TestGenerateSpec(t *testing.T) {
 		for _, cl := range [][]string{
 			s.Process.Capabilities.Bounding,
 			s.Process.Capabilities.Permitted,
-			s.Process.Capabilities.Inheritable,
 			s.Process.Capabilities.Effective,
 		} {
 			for i := 0; i < len(defaults); i++ {
@@ -193,8 +192,8 @@ func TestWithCapabilities(t *testing.T) {
 	if len(s.Process.Capabilities.Permitted) != 1 || s.Process.Capabilities.Permitted[0] != "CAP_SYS_ADMIN" {
 		t.Error("Unexpected capabilities set")
 	}
-	if len(s.Process.Capabilities.Inheritable) != 1 || s.Process.Capabilities.Inheritable[0] != "CAP_SYS_ADMIN" {
-		t.Error("Unexpected capabilities set")
+	if len(s.Process.Capabilities.Inheritable) != 0 {
+		t.Errorf("Unexpected capabilities set: length is non zero (%d)", len(s.Process.Capabilities.Inheritable))
 	}
 }
 
diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go
@@ -255,15 +255,14 @@ func TestContainerCapabilities(t *testing.T) {
 		for _, include := range test.includes {
 			assert.Contains(t, spec.Process.Capabilities.Bounding, include)
 			assert.Contains(t, spec.Process.Capabilities.Effective, include)
-			assert.Contains(t, spec.Process.Capabilities.Inheritable, include)
 			assert.Contains(t, spec.Process.Capabilities.Permitted, include)
 		}
 		for _, exclude := range test.excludes {
 			assert.NotContains(t, spec.Process.Capabilities.Bounding, exclude)
 			assert.NotContains(t, spec.Process.Capabilities.Effective, exclude)
-			assert.NotContains(t, spec.Process.Capabilities.Inheritable, exclude)
 			assert.NotContains(t, spec.Process.Capabilities.Permitted, exclude)
 		}
+		assert.Empty(t, spec.Process.Capabilities.Inheritable)
 		assert.Empty(t, spec.Process.Capabilities.Ambient)
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,6 @@ func TestGenerateSpec(t *testing.T) {`
`45`	`45`	`for _, cl := range [][]string{`
`46`	`46`	`s.Process.Capabilities.Bounding,`
`47`	`47`	`s.Process.Capabilities.Permitted,`
`48`		`- s.Process.Capabilities.Inheritable,`
`49`	`48`	`s.Process.Capabilities.Effective,`
`50`	`49`	`} {`
`51`	`50`	`for i := 0; i < len(defaults); i++ {`
`@@ -193,8 +192,8 @@ func TestWithCapabilities(t *testing.T) {`
`193`	`192`	`if len(s.Process.Capabilities.Permitted) != 1 \|\| s.Process.Capabilities.Permitted[0] != "CAP_SYS_ADMIN" {`
`194`	`193`	`t.Error("Unexpected capabilities set")`
`195`	`194`	`}`
`196`		`- if len(s.Process.Capabilities.Inheritable) != 1 \|\| s.Process.Capabilities.Inheritable[0] != "CAP_SYS_ADMIN" {`
`197`		`- t.Error("Unexpected capabilities set")`
	`195`	`+ if len(s.Process.Capabilities.Inheritable) != 0 {`
	`196`	`+ t.Errorf("Unexpected capabilities set: length is non zero (%d)", len(s.Process.Capabilities.Inheritable))`
`198`	`197`	`}`
`199`	`198`	`}`
`200`	`199`
Original file line number	Diff line number	Diff line change
`@@ -255,15 +255,14 @@ func TestContainerCapabilities(t *testing.T) {`
`255`	`255`	`for _, include := range test.includes {`
`256`	`256`	`assert.Contains(t, spec.Process.Capabilities.Bounding, include)`
`257`	`257`	`assert.Contains(t, spec.Process.Capabilities.Effective, include)`
`258`		`- assert.Contains(t, spec.Process.Capabilities.Inheritable, include)`
`259`	`258`	`assert.Contains(t, spec.Process.Capabilities.Permitted, include)`
`260`	`259`	`}`
`261`	`260`	`for _, exclude := range test.excludes {`
`262`	`261`	`assert.NotContains(t, spec.Process.Capabilities.Bounding, exclude)`
`263`	`262`	`assert.NotContains(t, spec.Process.Capabilities.Effective, exclude)`
`264`		`- assert.NotContains(t, spec.Process.Capabilities.Inheritable, exclude)`
`265`	`263`	`assert.NotContains(t, spec.Process.Capabilities.Permitted, exclude)`
`266`	`264`	`}`
	`265`	`+ assert.Empty(t, spec.Process.Capabilities.Inheritable)`
`267`	`266`	`assert.Empty(t, spec.Process.Capabilities.Ambient)`
`268`	`267`	`}`
`269`	`268`	`}`