bugfix: we should make MaskedPaths and ReadonlyPaths to be nil when p…

…rivileged be set

Signed-off-by: Michael Wan <zirenwan@gmail.com>

apis/swagger.yml

@@ -2263,16 +2263,6 @@ definitions:
           MaxLength: 64
           The characters of given id should be in 0123456789abcdef.
           By default, given id is unnecessary.
-      MaskedPaths:
-        description: "Masks over the provided paths inside the container."
-        type: "array"
-        items:
-          type: "string"
-      ReadonlyPaths:
-        description: "Set the provided paths as RO inside the container."
-        type: "array"
-        items:
-          type: "string"
       Snapshotter:
         description: |
             The snapshotter container choose, can be different with
@@ -2494,6 +2484,16 @@ definitions:
           InitScript:
             type: "string"
             description: "Initial script executed in container. The script will be executed before entrypoint or command"
+          MaskedPaths:
+            description: "Masks over the provided paths inside the container."
+            type: "array"
+            items:
+              type: "string"
+          ReadonlyPaths:
+            description: "Set the provided paths as RO inside the container."
+            type: "array"
+            items:
+              type: "string"
       - $ref: "#/definitions/Resources"
 
   UpdateConfig:

cri/v1alpha2/cri.go

@@ -712,8 +712,6 @@ func (c *CriManager) CreateContainer(ctx context.Context, r *runtime.CreateConta
 			NetPriority:    config.NetPriority,
 			DiskQuota:      resources.GetDiskQuota(),
 			QuotaID:        config.GetQuotaId(),
-			MaskedPaths:    config.GetLinux().GetSecurityContext().GetMaskedPaths(),
-			ReadonlyPaths:  config.GetLinux().GetSecurityContext().GetReadonlyPaths(),
 		},
 		HostConfig: &apitypes.HostConfig{
 			Binds:     generateMountBindings(config.GetMounts()),

cri/v1alpha2/cri_utils.go

@@ -759,6 +759,12 @@ func modifyHostConfig(sc *runtime.LinuxContainerSecurityContext, hostConfig *api
 	if sc.NoNewPrivs {
 		hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, "no-new-privileges")
 	}
+
+	if !hostConfig.Privileged {
+		hostConfig.MaskedPaths = sc.GetMaskedPaths()
+		hostConfig.ReadonlyPaths = sc.GetReadonlyPaths()
+	}
+
 	return nil
 }
 

cri/v1alpha2/cri_utils_test.go

@@ -702,6 +702,8 @@ func Test_modifyHostConfig(t *testing.T) {
 					SeccompProfilePath: mgr.ProfileDockerDefault,
 					ApparmorProfile:    mgr.ProfileRuntimeDefault,
 					NoNewPrivs:         true,
+					ReadonlyPaths:      []string{"/test/readyonly/path"},
+					MaskedPaths:        []string{"/test/masked/path"},
 				},
 				hostConfig: &apitypes.HostConfig{},
 			},
@@ -714,6 +716,35 @@ func Test_modifyHostConfig(t *testing.T) {
 			},
 			wantErr: nil,
 		},
+		{
+			name: "ReadonlypathAndMaskedPathTest",
+			args: args{
+				sc: &runtime.LinuxContainerSecurityContext{
+					Privileged:     false,
+					ReadonlyRootfs: true,
+					Capabilities: &runtime.Capability{
+						AddCapabilities:  []string{"fooAdd1", "fooAdd2"},
+						DropCapabilities: []string{"fooDrop1", "fooDrop2"},
+					},
+					SeccompProfilePath: mgr.ProfileDockerDefault,
+					ApparmorProfile:    mgr.ProfileRuntimeDefault,
+					NoNewPrivs:         true,
+					ReadonlyPaths:      []string{"/test/readyonly/path"},
+					MaskedPaths:        []string{"/test/masked/path"},
+				},
+				hostConfig: &apitypes.HostConfig{},
+			},
+			wantHostConfig: &apitypes.HostConfig{
+				Privileged:     false,
+				ReadonlyRootfs: true,
+				CapAdd:         []string{"fooAdd1", "fooAdd2"},
+				CapDrop:        []string{"fooDrop1", "fooDrop2"},
+				SecurityOpt:    []string{"no-new-privileges"},
+				ReadonlyPaths:  []string{"/test/readyonly/path"},
+				MaskedPaths:    []string{"/test/masked/path"},
+			},
+			wantErr: nil,
+		},
 		{
 			name: "Capabilities Nil Test",
 			args: args{

daemon/mgr/container.go

@@ -416,21 +416,25 @@ func (mgr *ContainerManager) Create(ctx context.Context, name string, config *ty
 	// set default log driver and validate for logger driver
 	config.HostConfig.LogConfig = mgr.getDefaultLogConfigIfMissing(config.HostConfig.LogConfig)
 
+	// set ReadonlyPaths and MaskedPaths to nil if privileged was set.
+	if config.HostConfig.Privileged {
+		config.HostConfig.ReadonlyPaths = nil
+		config.HostConfig.MaskedPaths = nil
+	}
+
 	container := &Container{
 		State: &types.ContainerState{
 			Status:     types.StatusCreated,
 			StartedAt:  time.Time{}.UTC().Format(utils.TimeLayout),
 			FinishedAt: time.Time{}.UTC().Format(utils.TimeLayout),
 		},
-		ID:            id,
-		Image:         imgID.String(),
-		Name:          name,
-		Config:        &config.ContainerConfig,
-		Created:       time.Now().UTC().Format(utils.TimeLayout),
-		HostConfig:    config.HostConfig,
-		SnapshotID:    snapID,
-		ReadonlyPaths: config.ReadonlyPaths,
-		MaskedPaths:   config.MaskedPaths,
+		ID:         id,
+		Image:      imgID.String(),
+		Name:       name,
+		Config:     &config.ContainerConfig,
+		Created:    time.Now().UTC().Format(utils.TimeLayout),
+		HostConfig: config.HostConfig,
+		SnapshotID: snapID,
 	}
 
 	if _, err := mgr.initContainerIO(container); err != nil {

daemon/mgr/container_types.go

@@ -269,12 +269,6 @@ type Container struct {
 
 	// SnapshotID specify id of the snapshot that container using.
 	SnapshotID string
-
-	// Masks over the provided paths inside the container.
-	MaskedPaths []string `json:"MaskedPaths,omitempty"`
-
-	// Set the provided paths as RO inside the container.
-	ReadonlyPaths []string `json:"ReadonlyPaths，omitempty"`
 }
 
 // Key returns container's id.

daemon/mgr/spec_linux.go

@@ -69,21 +69,18 @@ func populatePlatform(ctx context.Context, c *Container, specWrapper *SpecWrappe
 	// setup something depend on privileged authority
 	if !c.HostConfig.Privileged {
 		s.Linux.MountLabel = c.MountLabel
+
+		// if MaskedPaths or ReadonlyPaths are set, we will use them, otherwise using the default values.
+		if len(c.HostConfig.MaskedPaths) > 0 {
+			s.Linux.MaskedPaths = c.HostConfig.MaskedPaths
+		}
+		if len(c.HostConfig.ReadonlyPaths) > 0 {
+			s.Linux.ReadonlyPaths = c.HostConfig.ReadonlyPaths
+		}
 	} else {
-		s.Linux.ReadonlyPaths = nil
+		// MaskedPaths and ReadonlyPaths have default values, we should reset them when privileged be set
 		s.Linux.MaskedPaths = nil
-	}
-
-	// Apply masked paths if specified.
-	if c.MaskedPaths != nil {
-		s.Linux.MaskedPaths = make([]string, len(c.MaskedPaths))
-		copy(s.Linux.MaskedPaths, c.MaskedPaths)
-	}
-
-	// Apply readonly paths if specified.
-	if c.ReadonlyPaths != nil {
-		s.Linux.ReadonlyPaths = make([]string, len(c.ReadonlyPaths))
-		copy(s.Linux.ReadonlyPaths, c.ReadonlyPaths)
+		s.Linux.ReadonlyPaths = nil
 	}
 
 	// start to setup linux seccomp